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>