[patch] avoid possible null pointer dereference

[patch] avoid possible null pointer dereference

[Ulrich Spörlein]

[-- Attachment #1: Type: text/plain, Size: 247 bytes --]

And the last one,

termp_xx_pre() will call term_word() with NULL when the switch case
falls through to the default case. There are several ways to avoid the
segfault, not sure this is the best one.

This is Coverity Scan CID 976115.

Cheers,
Uli

[-- Attachment #2: mdoc_term.diff --]
[-- Type: text/x-diff, Size: 430 bytes --]

Index: mdoc_term.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_term.c,v
retrieving revision 1.245
diff -u -p -r1.245 mdoc_term.c
--- mdoc_term.c	17 Nov 2012 00:26:33 -0000	1.245
+++ mdoc_term.c	17 May 2013 01:32:03 -0000
@@ -1756,7 +1756,7 @@ termp_xx_pre(DECL_ARGS)
 		pp = "UNIX";
 		break;
 	default:
-		break;
+		return(0);
 	}
 
 	term_word(p, pp);

Re: [patch] avoid possible null pointer dereference

[Ingo Schwarze]

Hi Ulrich,

Ulrich Spörlein wrote on Fri, May 17, 2013 at 03:37:39AM +0200:

> termp_xx_pre() will call term_word() with NULL when the switch case
> falls through to the default case.

Actually, that cannot happen because termp_xx_pre() will not be called
for macros it is not intended to handle.

Still, it's arguably a cosmetical issue.  The code looks like the default
case would be legitimate, which it is not.

> There are several ways to avoid the segfault, not sure this is the
> best one.

No, it isn't, because that merely substitutes one uncontrolled failure
mode by another one that's arguably even harder to debug, should it
ever occur (after incorrect code changes elsewhere).

Instead, i committed a change to bsd.lv and openbsd.org using abort(3).

> This is Coverity Scan CID 976115.

Thanks,
  Ingo

> Index: mdoc_term.c
> ===================================================================
> RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_term.c,v
> retrieving revision 1.245
> diff -u -p -r1.245 mdoc_term.c
> --- mdoc_term.c	17 Nov 2012 00:26:33 -0000	1.245
> +++ mdoc_term.c	17 May 2013 01:32:03 -0000
> @@ -1756,7 +1756,7 @@ termp_xx_pre(DECL_ARGS)
>  		pp = "UNIX";
>  		break;
>  	default:
> -		break;
> +		return(0);
>  	}
>  
>  	term_word(p, pp);
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: [patch] avoid possible null pointer dereference

[Ulrich Spörlein]

On Sat, 2013-05-18 at 19:54:35 +0200, Ingo Schwarze wrote:
> Hi Ulrich,
> 
> Ulrich Spörlein wrote on Fri, May 17, 2013 at 03:37:39AM +0200:
> 
> > termp_xx_pre() will call term_word() with NULL when the switch case
> > falls through to the default case.
> 
> Actually, that cannot happen because termp_xx_pre() will not be called
> for macros it is not intended to handle.
> 
> Still, it's arguably a cosmetical issue.  The code looks like the default
> case would be legitimate, which it is not.
> 
> > There are several ways to avoid the segfault, not sure this is the
> > best one.
> 
> No, it isn't, because that merely substitutes one uncontrolled failure
> mode by another one that's arguably even harder to debug, should it
> ever occur (after incorrect code changes elsewhere).
> 
> Instead, i committed a change to bsd.lv and openbsd.org using abort(3).

Thanks! I figured that it was not the best fix, but I don't know the API
"contracts" within mdocml.

There are a couple more issues that Coverity Scan thinks exists, but I
need to understand those better first.

Cheers,
Uli
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv