On Thu, Jan 15, 2015 at 11:17:57AM +0100, Olivier Fourdan wrote: > XkbSetGeometry request embeds lots of strings which have their length > encoded on a CARD16 which needs to be swapped when the server and client > have different endianess. > > The function _GetCountedString() that parses these strings from the requests > does the swapping of the length in the input buffer directly. > > However, ProcXkbSetGeometry() may call _XkbSetGeometry() more than once > (if there is more than one keyboard), thus causing on swapped clients the > same data to be swapped twice in memory, further causing a server crash > because the strings lengths on the second time are way off bounds. > > To allow _GetCountedString() to run reliably more than once with swapped > clients, do not swap the data in the buffer, use a variable instead. > > Signed-off-by: Olivier Fourdan <[email protected]> > --- > xkb/xkb.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/xkb/xkb.c b/xkb/xkb.c > index 15c7f34..f8c2635 100644 > --- a/xkb/xkb.c > +++ b/xkb/xkb.c > @@ -4965,10 +4965,10 @@ _GetCountedString(char **wire_inout, Bool swap) > > wire = *wire_inout; > plen = (CARD16 *) wire; > + len = *plen; > if (swap) { > - swaps(plen); > + swaps(&len); > } > - len = *plen; > str = malloc(len + 1); > if (str) { > memcpy(str, &wire[2], len); > -- > 2.1.0
you could drop plen and go len = *(CARD16*)wire; ... either way, Reviewed-by: Peter Hutterer <[email protected]> Cheers, Peter _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
