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.
-- 
Eygene Ryabinkin                                        ,,,^..^,,,
[ Life's unfair - but root password helps!           | codelabs.ru ]
[ 82FE 06BC D497 C0DE 49EC  4FF0 16AF 9EAE 8152 ECFB | freebsd.org ]
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
from reStructuredText, so has empty blocks at the places where
it would be enough just to specify the empty value.

Index: usr.bin/mandoc/Makefile
===================================================================
--- usr.bin/mandoc/Makefile	(revision 349971)
+++ usr.bin/mandoc/Makefile	(working copy)
@@ -101,4 +101,7 @@
 CFLAGS.gcc+=	-Wno-format
 LIBADD=	openbsd z
 
+HAS_TESTS=
+SUBDIR.${MK_TESTS}+= tests
+
 .include <bsd.prog.mk>
Index: usr.bin/mandoc/tests/Makefile
===================================================================
--- usr.bin/mandoc/tests/Makefile	(nonexistent)
+++ usr.bin/mandoc/tests/Makefile	(working copy)
@@ -0,0 +1,11 @@
+# $FreeBSD$
+
+PACKAGE=	tests
+
+${PACKAGE}FILES+=	empty-table-cdata.1
+
+ATF_TESTS_SH+=		regression-tests
+
+BINDIR=	${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=%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
===================================================================
--- 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 = \
+
+
+.include <dirdeps.mk>
+
+.if ${DEP_RELDIR} == ${_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=%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
===================================================================
--- 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, <r...@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=%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
===================================================================
--- usr.bin/mandoc/tests/regression-tests.sh	(nonexistent)
+++ usr.bin/mandoc/tests/regression-tests.sh	(working copy)
@@ -0,0 +1,20 @@
+# $FreeBSD$
+
+
+SRCDIR=$(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=$(atf_config_get usr.bin.mandoc.test_mandoc /usr/bin/mandoc)
+
+	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=%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
===================================================================
--- 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
===================================================================
--- contrib/mandoc/tbl_term.c	(revision 349971)
+++ contrib/mandoc/tbl_term.c	(working copy)
@@ -626,7 +626,8 @@
 
 		lw = cpp == NULL || cpn == NULL ||
 		    (cpn->pos != TBL_CELL_DOWN &&
-		     (dpn == NULL || strcmp(dpn->string, "\\^") != 0))
+		     (dpn == NULL || dpn->pos != TBL_DATA_DATA ||
+		      strcmp(dpn->string, "\\^") != 0))
 		    ? hw : 0;
 		tbl_direct_border(tp, BHORIZ * lw,
 		    col->width + col->spacing / 2);
@@ -670,7 +671,8 @@
 
 		rw = cpp == NULL || cpn == NULL ||
 		    (cpn->pos != TBL_CELL_DOWN &&
-		     (dpn == NULL || strcmp(dpn->string, "\\^") != 0))
+		     (dpn == NULL || dpn->pos != TBL_DATA_DATA ||
+		      strcmp(dpn->string, "\\^") != 0))
 		    ? hw : 0;
 
 		/* The line crossing at the end of this column. */
.\" $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, <r...@freebsd.org>.

Attachment: signature.asc
Description: PGP signature

Reply via email to