On Fri, Jan 16, 2015 at 08:08:59PM +0100, Olivier Fourdan wrote: > The XkbSetGeometry request embeds data which needs to be swapped when the > server and the client have different endianess. > > _XkbSetGeometry() invokes functions that swap these data directly in the > input buffer. > > 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 _XkbSetGeometry() to run reliably more than once with swapped > clients, do not swap the data in the buffer, use variables instead. > > Signed-off-by: Olivier Fourdan <[email protected]>
Reviewed-by: Peter Hutterer <[email protected]> Cheers, Peter > --- > xkb/xkb.c | 35 +++++++++++++++++++---------------- > 1 file changed, 19 insertions(+), 16 deletions(-) > > diff --git a/xkb/xkb.c b/xkb/xkb.c > index 15c7f34..b9a3ac4 100644 > --- a/xkb/xkb.c > +++ b/xkb/xkb.c > @@ -4961,14 +4961,13 @@ static char * > _GetCountedString(char **wire_inout, Bool swap) > { > char *wire, *str; > - CARD16 len, *plen; > + CARD16 len; > > wire = *wire_inout; > - plen = (CARD16 *) wire; > + len = *(CARD16 *) wire; > if (swap) { > - swaps(plen); > + swaps(&len); > } > - len = *plen; > str = malloc(len + 1); > if (str) { > memcpy(str, &wire[2], len); > @@ -4985,25 +4984,28 @@ _CheckSetDoodad(char **wire_inout, > { > char *wire; > xkbDoodadWireDesc *dWire; > + xkbAnyDoodadWireDesc any; > + xkbTextDoodadWireDesc text; > XkbDoodadPtr doodad; > > dWire = (xkbDoodadWireDesc *) (*wire_inout); > + any = dWire->any; > wire = (char *) &dWire[1]; > if (client->swapped) { > - swapl(&dWire->any.name); > - swaps(&dWire->any.top); > - swaps(&dWire->any.left); > - swaps(&dWire->any.angle); > + swapl(&any.name); > + swaps(&any.top); > + swaps(&any.left); > + swaps(&any.angle); > } > CHK_ATOM_ONLY(dWire->any.name); > - doodad = XkbAddGeomDoodad(geom, section, dWire->any.name); > + doodad = XkbAddGeomDoodad(geom, section, any.name); > if (!doodad) > return BadAlloc; > doodad->any.type = dWire->any.type; > doodad->any.priority = dWire->any.priority; > - doodad->any.top = dWire->any.top; > - doodad->any.left = dWire->any.left; > - doodad->any.angle = dWire->any.angle; > + doodad->any.top = any.top; > + doodad->any.left = any.left; > + doodad->any.angle = any.angle; > switch (doodad->any.type) { > case XkbOutlineDoodad: > case XkbSolidDoodad: > @@ -5026,12 +5028,13 @@ _CheckSetDoodad(char **wire_inout, > dWire->text.colorNdx); > return BadMatch; > } > + text = dWire->text; > if (client->swapped) { > - swaps(&dWire->text.width); > - swaps(&dWire->text.height); > + swaps(&text.width); > + swaps(&text.height); > } > - doodad->text.width = dWire->text.width; > - doodad->text.height = dWire->text.height; > + doodad->text.width = text.width; > + doodad->text.height = text.height; > doodad->text.color_ndx = dWire->text.colorNdx; > doodad->text.text = _GetCountedString(&wire, client->swapped); > doodad->text.font = _GetCountedString(&wire, client->swapped); > -- > 2.1.0 > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
