Date: Tue, 16 Jul 2019 22:31:24 +0300 From: Eygene Ryabinkin <rea@freebsd.org> To: freebsd-current@FreeBSD.org, bapt@FreeBSD.org Subject: [CFT][patch] mandoc: don't segfault on empty tbl(1) continuation blocks Message-ID: <20190716193124.yrrntrtah22aky5n@phoenix.codelabs.ru>
next in thread | raw e-mail | index | archive | help
--gz7utc52kvukndjs
Content-Type: multipart/mixed; boundary="cds6ufi5oa7zoiin"
Content-Disposition: inline
--cds6ufi5oa7zoiin
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Good day.
Attached is the patch that makes built-in tbl(1) processor in mandoc
to avoid dumping core when it renders the table with empty "T{ T}"
block and horizontally-ruled table.
The simplest way to reproduce the issue is to either
- run 'man notmuch-config' with mail/notmuch installed;
- run 'mandoc tests/empty-table-cdata.1' against the attached
test-only manpage.
With the patch applied, one can utilize 'make check': regression
test was added. Perhaps an invocation of
{{{
mtree -deU -f /usr/src/etc/mtree/BSD.tests.dist -p /usr/tests
}}}
will be needed to run 'make check' without remaking/installing
the world.
The patch is for the fresh -CURRENT. Be interested in any results
of its application and usage.
Thanks!
P.S.: please, CC me: I am not subscribed to the list.
--=20
Eygene Ryabinkin ,,,^..^,,,
[ Life's unfair - but root password helps! | codelabs.ru ]
[ 82FE 06BC D497 C0DE 49EC 4FF0 16AF 9EAE 8152 ECFB | freebsd.org ]
--cds6ufi5oa7zoiin
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="mandoc-fix-empty-cdata-crash.patch"
Content-Transfer-Encoding: quoted-printable
mandoc: fix built-in tbl(1) processing of empty continuation blocks
Empty "T{ T}" (continuation) blocks produce NULL-valued string
for their data block: getdata() allocates structure with string
set to NULL and tbl_cdata() will just return when it sees
the end ("T}") of the block without any further manipulations
with dat->string.
This is completely legal; moreover, tbl.h specifies that for
'struct tbl_dat' the 'string' member is NULL when entry type
is not TBL_DATA_DATA. This is not so all the time, but one
shouldn't rely on this.
The segfault in question was plain NULL pointer dereference
triggered from tbl_term.c::tbl_hrule().
Added check for dpn->pos not being TBL_DATA_DATA. Also added
regression test to find such problems in the future.
The real-world case when manpage was provoking core dump
is notmuch-config.1 for mail/notmuch port: it is auto-generated
=66rom reStructuredText, so has empty blocks at the places where
it would be enough just to specify the empty value.
Index: usr.bin/mandoc/Makefile
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- usr.bin/mandoc/Makefile (revision 349971)
+++ usr.bin/mandoc/Makefile (working copy)
@@ -101,4 +101,7 @@
CFLAGS.gcc+=3D -Wno-format
LIBADD=3D openbsd z
=20
+HAS_TESTS=3D
+SUBDIR.${MK_TESTS}+=3D tests
+
.include <bsd.prog.mk>
Index: usr.bin/mandoc/tests/Makefile
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- usr.bin/mandoc/tests/Makefile (nonexistent)
+++ usr.bin/mandoc/tests/Makefile (working copy)
@@ -0,0 +1,11 @@
+# $FreeBSD$
+
+PACKAGE=3D tests
+
+${PACKAGE}FILES+=3D empty-table-cdata.1
+
+ATF_TESTS_SH+=3D regression-tests
+
+BINDIR=3D ${TESTSDIR}
+
+.include <bsd.test.mk>
Property changes on: usr.bin/mandoc/tests/Makefile
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+FreeBSD=3D%H
\ No newline at end of property
Added: svn:mime-type
## -0,0 +1 ##
+text/plain
\ No newline at end of property
Index: usr.bin/mandoc/tests/Makefile.depend
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- usr.bin/mandoc/tests/Makefile.depend (nonexistent)
+++ usr.bin/mandoc/tests/Makefile.depend (working copy)
@@ -0,0 +1,11 @@
+# $FreeBSD$
+# Autogenerated - do NOT edit!
+
+DIRDEPS =3D \
+
+
+.include <dirdeps.mk>
+
+.if ${DEP_RELDIR} =3D=3D ${_DEP_RELDIR}
+# local dependencies - needed for -jN in clean tree
+.endif
Property changes on: usr.bin/mandoc/tests/Makefile.depend
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+FreeBSD=3D%H
\ No newline at end of property
Added: svn:mime-type
## -0,0 +1 ##
+text/plain
\ No newline at end of property
Index: usr.bin/mandoc/tests/empty-table-cdata.1
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- usr.bin/mandoc/tests/empty-table-cdata.1 (nonexistent)
+++ usr.bin/mandoc/tests/empty-table-cdata.1 (working copy)
@@ -0,0 +1,21 @@
+.\" $FreeBSD$
+.
+.TH EMPTY-TABLE-CDATA 1 1970-01-01
+.SH Empty table cdata test for tbl processor
+.
+.PP
+The following table should not make mandoc to dump core:
+.
+.TS
+|l|l|.
+_
+A test
+_
+table T{
+T}
+_
+.TE
+.
+.SH Author
+.PP
+Eygene Ryabinkin, <rea@FreeBSD.org>.
Property changes on: usr.bin/mandoc/tests/empty-table-cdata.1
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+FreeBSD=3D%H
\ No newline at end of property
Added: svn:mime-type
## -0,0 +1 ##
+text/plain
\ No newline at end of property
Index: usr.bin/mandoc/tests/regression-tests.sh
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- usr.bin/mandoc/tests/regression-tests.sh (nonexistent)
+++ usr.bin/mandoc/tests/regression-tests.sh (working copy)
@@ -0,0 +1,20 @@
+# $FreeBSD$
+
+
+SRCDIR=3D$(atf_get_srcdir)
+
+
+atf_test_case empty_table_cdata
+empty_table_cdata_head() {
+ atf_set "descr" "Normal processing of empty T{ T} blocks in tables"
+}
+empty_table_cdata_body() {
+ local mandoc=3D$(atf_config_get usr.bin.mandoc.test_mandoc /usr/bin/mando=
c)
+
+ atf_check -s exit: -o not-empty $mandoc "$SRCDIR"/empty-table-cdata.1
+}
+
+
+atf_init_test_cases() {
+ atf_add_test_case empty_table_cdata
+}
Property changes on: usr.bin/mandoc/tests/regression-tests.sh
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:executable
## -0,0 +1 ##
+*
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+FreeBSD=3D%H
\ No newline at end of property
Added: svn:mime-type
## -0,0 +1 ##
+text/plain
\ No newline at end of property
Index: etc/mtree/BSD.tests.dist
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- etc/mtree/BSD.tests.dist (revision 349971)
+++ etc/mtree/BSD.tests.dist (working copy)
@@ -1004,6 +1004,8 @@
..
m4
..
+ mandoc
+ ..
mkimg
..
ncal
Index: contrib/mandoc/tbl_term.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- contrib/mandoc/tbl_term.c (revision 349971)
+++ contrib/mandoc/tbl_term.c (working copy)
@@ -626,7 +626,8 @@
=20
lw =3D cpp =3D=3D NULL || cpn =3D=3D NULL ||
(cpn->pos !=3D TBL_CELL_DOWN &&
- (dpn =3D=3D NULL || strcmp(dpn->string, "\\^") !=3D 0))
+ (dpn =3D=3D NULL || dpn->pos !=3D TBL_DATA_DATA ||
+ strcmp(dpn->string, "\\^") !=3D 0))
? hw : 0;
tbl_direct_border(tp, BHORIZ * lw,
col->width + col->spacing / 2);
@@ -670,7 +671,8 @@
=20
rw =3D cpp =3D=3D NULL || cpn =3D=3D NULL ||
(cpn->pos !=3D TBL_CELL_DOWN &&
- (dpn =3D=3D NULL || strcmp(dpn->string, "\\^") !=3D 0))
+ (dpn =3D=3D NULL || dpn->pos !=3D TBL_DATA_DATA ||
+ strcmp(dpn->string, "\\^") !=3D 0))
? hw : 0;
=20
/* The line crossing at the end of this column. */
--cds6ufi5oa7zoiin
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="empty-table-cdata.1"
.\" $FreeBSD$
.
.TH EMPTY-TABLE-CDATA 1 1970-01-01
.SH Empty table cdata test for tbl processor
.
.PP
The following table should not make mandoc to dump core:
.
.TS
|l|l|.
_
A test
_
table T{
T}
_
.TE
.
.SH Author
.PP
Eygene Ryabinkin, <rea@FreeBSD.org>.
--cds6ufi5oa7zoiin--
--gz7utc52kvukndjs
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
iNUEABEKAH0WIQSC/ga81JfA3knsT/AWr56ugVLs+wUCXS4mB18UgAAAAAAuAChp
c3N1ZXItZnByQG5vdGF0aW9ucy5vcGVucGdwLmZpZnRoaG9yc2VtYW4ubmV0ODJG
RTA2QkNENDk3QzBERTQ5RUM0RkYwMTZBRjlFQUU4MTUyRUNGQgAKCRAWr56ugVLs
+3yMAP9Qi6AhAa+Te9ckPanrkwn1yQlkNJ7Ijzpk2uqLr6x5qQD/Wv9q8un/WYxm
eaxYMUayUFoVumCdva9hBw9yPrTa5V4=
=lWaK
-----END PGP SIGNATURE-----
--gz7utc52kvukndjs--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190716193124.yrrntrtah22aky5n>