[PATCH 10/15] xkb: Fix possible NULL pointer dereference

Pauli Nieminen ext-pauli.nieminen at nokia.com
Wed Jul 28 04:14:26 PDT 2010


On 28/07/10 01:15 +0200, ext Peter Hutterer wrote:
> On Tue, Jul 27, 2010 at 03:09:51PM +0300, Pauli Nieminen wrote:
> > If search for device failed sli is NULL. In that case we have to protect
> > dereference to prevent server crash.
> > 
> > Signed-off-by: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> > ---
> >  xkb/ddxLoad.c |    6 +++++-
> >  xkb/xkbLEDs.c |   10 ++++++----
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
> > index 5e6ab87..2fdf71e 100644
> > --- a/xkb/ddxLoad.c
> > +++ b/xkb/ddxLoad.c
> > @@ -342,7 +342,11 @@ char		fileName[PATH_MAX];
> >  unsigned	missing;
> >  
> >      *xkbRtrn = NULL;
> > -    if ((keybd==NULL)||(keybd->key==NULL)||(keybd->key->xkbInfo==NULL))
> > +    if (!keybd) {
> > +        LogMessage(X_ERROR, "XKB: %s no device given as parameter.\n", __func__);
> > +        return 0;
> > +    }
> > +    if ((keybd->key==NULL)||(keybd->key->xkbInfo==NULL))
> >  	 xkb= NULL;
> >      else xkb= keybd->key->xkbInfo->desc;
> >      if ((names->keycodes==NULL)&&(names->types==NULL)&&
> 
> what was the error message that prompted this hunk? Cursory inspection of
> the code seems like a keybd of NULL is valid.

I have accidentaly combined 2 separate patches to single commit.

keybd is dereferenced unconditionaly in dunction calls that are done later in
same function. I will screate the separate commits in next set.

> 
> > diff --git a/xkb/xkbLEDs.c b/xkb/xkbLEDs.c
> > index f617537..1682671 100644
> > --- a/xkb/xkbLEDs.c
> > +++ b/xkb/xkbLEDs.c
> > @@ -714,10 +714,12 @@ XkbSrvLedInfoPtr	sli;
> >  	    }	
> >  	}
> >      }
> > -    if ((sli->names==NULL)&&(needed_parts&XkbXI_IndicatorNamesMask))
> > -	sli->names= calloc(XkbNumIndicators, sizeof(Atom));
> > -    if ((sli->maps==NULL)&&(needed_parts&XkbXI_IndicatorMapsMask))
> > -	sli->maps= calloc(XkbNumIndicators, sizeof(XkbIndicatorMapRec));
> > +    if (sli) {
> > +	if ((sli->names==NULL)&&(needed_parts&XkbXI_IndicatorNamesMask))
> > +	    sli->names= calloc(XkbNumIndicators, sizeof(Atom));
> > +	if ((sli->maps==NULL)&&(needed_parts&XkbXI_IndicatorMapsMask))
> > +	    sli->maps= calloc(XkbNumIndicators, sizeof(XkbIndicatorMapRec));
> > +    }
> >      return sli;
> >  }
> >  
> > -- 
> > 1.6.3.3
> 
> these seem to be two unrelated hunks, should be two separate patches.
> Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net> for the second hunk
> though with the commit message above.
>  
> Cheers,
>   Peter


More information about the xorg-devel mailing list