[PATCH xserver] xkb: Initialize pad bytes sent in replies of geometry requests.

[Peter Hutterer]

On Wed, Mar 09, 2011 at 03:45:40PM +0200, Rami Ylimäki wrote:
> Valgrind complains about uninitialized data being written to clients.
> 
> Reviewed-by: Erkki Seppälä <erkki.seppala at vincit.fi>
> Signed-off-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
> ---
>  xkb/xkb.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/xkb/xkb.c b/xkb/xkb.c
> index 8d61e01..39dbab4 100644
> --- a/xkb/xkb.c
> +++ b/xkb/xkb.c
> @@ -4305,7 +4305,7 @@ ProcXkbSetNames(ClientPtr client)
>  static char *
>  XkbWriteCountedString(char *wire,char *str,Bool swap)
>  {
> -    CARD16 len,*pLen;
> +    CARD16 len,*pLen, paddedLen;
>  
>      if (!str)
>          return wire;
> @@ -4317,8 +4317,9 @@ XkbWriteCountedString(char *wire,char *str,Bool swap)
>  	register int n;
>  	swaps(pLen,n);
>      }
> -    memcpy(&wire[2],str,len);
> -    wire+= ((2+len+3)/4)*4;
> +    paddedLen= pad_to_int32(sizeof(len)+len)-sizeof(len);
> +    strncpy(&wire[sizeof(len)],str,paddedLen);
> +    wire+= sizeof(len)+paddedLen;
>      return wire;
>  }

looking at this again, does this really fix the issue?

assuming a string of strlen 11, paddedLen is 14.
strncpy will copy 12 bytes (strlen + \0), so you write 14 bytes in total but
advance wire by 16 bytes. the last 2 are uninitialized.

which would have the same effect as before, right?

Cheers,
  Peter

>  
> @@ -4429,6 +4430,7 @@ xkbShapeWireDesc *	shapeWire;
>  	if (shape->approx!=NULL)
>  	     shapeWire->approxNdx= XkbOutlineIndex(shape,shape->approx);
>  	else shapeWire->approxNdx= XkbNoShape;
> +	shapeWire->pad= 0;
>  	if (swap) {
>  	    register int n;
>  	    swapl(&shapeWire->name,n);
> @@ -4441,6 +4443,7 @@ xkbShapeWireDesc *	shapeWire;
>  	    olWire= (xkbOutlineWireDesc *)wire;
>  	    olWire->nPoints= ol->num_points;
>  	    olWire->cornerRadius= ol->corner_radius;
> +	    olWire->pad= 0;
>  	    wire= (char *)&olWire[1];
>  	    ptWire= (xkbPointWireDesc *)wire;
>  	    for (p=0,pt=ol->points;p<ol->num_points;p++,pt++) {
> @@ -4554,6 +4557,8 @@ xkbOverlayWireDesc *	olWire;
>     olWire= (xkbOverlayWireDesc *)wire;
>     olWire->name= ol->name;
>     olWire->nRows= ol->num_rows;
> +   olWire->pad1= 0;
> +   olWire->pad2= 0;
>     if (swap) {
>  	register int n;
>  	swapl(&olWire->name,n);
> @@ -4566,6 +4571,7 @@ xkbOverlayWireDesc *	olWire;
>  	rowWire= (xkbOverlayRowWireDesc *)wire;
>  	rowWire->rowUnder= row->row_under;
>  	rowWire->nKeys= row->num_keys;
> +	rowWire->pad1= 0;
>  	wire= (char *)&rowWire[1];
>  	for (k=0,key=row->keys;k<row->num_keys;k++,key++) {
>  	    xkbOverlayKeyWireDesc *	keyWire;
> -- 
> 1.6.3.3
>