Re: port-hppa/56118: sporadic app crashes in HPPA -current

[Tom Lane <tgl%sss.pgh.pa.us@localhost>]

The following reply was made to PR port-hppa/56118; it has been noted by GNATS.

From: Tom Lane <tgl%sss.pgh.pa.us@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: port-hppa/56118: sporadic app crashes in HPPA -current
Date: Sun, 05 Jun 2022 22:34:39 -0400

 I wrote:
 > I poked into this a little bit.  I think what is happening is that
 > makemandb assumes that it can do this function pointer comparison:
 >         } else if (mdocs[n->tok] == pmdoc_Xr) {
 > without regard for whether "mdocs[n->tok]" is a valid pointer;
 
 Actually ... this code fragment is utterly broken, and seemingly
 has been for awhile.  mdocs[] is a constant array of function
 pointers:
 
 static  const proff_nf mdocs[MDOC_MAX - MDOC_Dd] = {
         NULL, /* Dd */
         NULL, /* Dt */
 
 and it is supposed to be indexed by token type minus MDOC_Dd,
 not just token type.  (If you're hoping that MDOC_Dd is zero,
 it ain't.)  So we're not even indexing the array correctly,
 nor do we have any guards for an out-of-range token type,
 which is how come we're managing to pass garbage to
 __canonicalize_funcptr_for_compare.  The one other usage of
 mdocs[] in proff_node() gets these indexing considerations right.
 
 So a minimal fix would look like
 
 Index: makemandb.c
 ===================================================================
 RCS file: /cvsroot/src/usr.sbin/makemandb/makemandb.c,v
 retrieving revision 1.62
 diff -u -r1.62 makemandb.c
 --- makemandb.c	6 Apr 2022 03:23:38 -0000	1.62
 +++ makemandb.c	6 Jun 2022 02:14:22 -0000
 @@ -1078,14 +1078,16 @@
  
  	if (n->type == ROFFT_TEXT) {
  		mdoc_parse_section(n->sec, n->string, rec);
 -	} else if (mdocs[n->tok] == pmdoc_Xr) {
 +	} else if (n->tok >= MDOC_Dd && n->tok < MDOC_MAX &&
 +		   mdocs[n->tok - MDOC_Dd] == pmdoc_Xr) {
  		/*
  		 * When encountering other inline macros,
  		 * call pmdoc_macro_handler.
  		 */
  		pmdoc_macro_handler(n, rec, MDOC_Xr);
  		xr_found = 1;
 -	} else if (mdocs[n->tok] == pmdoc_Pp) {
 +	} else if (n->tok >= MDOC_Dd && n->tok < MDOC_MAX &&
 +		   mdocs[n->tok - MDOC_Dd] == pmdoc_Pp) {
  		pmdoc_macro_handler(n, rec, MDOC_Pp);
  	}
  
 I applied this version and I find that "makemandb -Q" completes now
 on my HPPA box, which it did not before.  However, I don't have a
 whole lot of faith in that being a 100% fix, because I don't think
 it's entirely guaranteed that the program's GOT is swapped into the
 TLB buffers when we reach this code.  Seeing that pmdoc_Xr and
 pmdoc_Pp appear only once in mdocs[], this code could be simplified
 without change of semantics to
 
 Index: makemandb.c
 ===================================================================
 RCS file: /cvsroot/src/usr.sbin/makemandb/makemandb.c,v
 retrieving revision 1.62
 diff -u -r1.62 makemandb.c
 --- makemandb.c	6 Apr 2022 03:23:38 -0000	1.62
 +++ makemandb.c	6 Jun 2022 02:17:21 -0000
 @@ -1078,14 +1078,14 @@
  
  	if (n->type == ROFFT_TEXT) {
  		mdoc_parse_section(n->sec, n->string, rec);
 -	} else if (mdocs[n->tok] == pmdoc_Xr) {
 +	} else if (n->tok == MDOC_Xr) {
  		/*
  		 * When encountering other inline macros,
  		 * call pmdoc_macro_handler.
  		 */
  		pmdoc_macro_handler(n, rec, MDOC_Xr);
  		xr_found = 1;
 -	} else if (mdocs[n->tok] == pmdoc_Pp) {
 +	} else if (n->tok == MDOC_Pp) {
  		pmdoc_macro_handler(n, rec, MDOC_Pp);
  	}
  
 and on the whole I'd recommend that coding.
 
 I wonder, however, why nobody has noticed that this code doesn't
 work as intended on any platform.  I don't know enough about roff
 to devise a test case for it, but maybe one is needed.
 
 Meanwhile, I still think that the HPPA kernel support for PROBE[I]
 is broken.
 
 			regards, tom lane