[PATCH libX11 v2] XKB: fix XkbGetKeyboardByName with Xming server

[Peter Hutterer]

On Thu, Jan 21, 2016 at 10:48:05AM +0100, Olivier Fourdan wrote:
> XkbGetKeyboardByName relies on flags to read the data from the server.
> 
> If the X server sends us the wrong flags or if a subreply is smaller
> than it should be, XkbGetKeyboardByName will not read all the available
> data and leave data in the buffer, which will cause the next _XReply()
> to fail with:
> 
> [xcb] Extra reply data still left in queue
> [xcb] This is most likely caused by a broken X extension library
> [xcb] Aborting, sorry about that.
> xcb_io.c:576: _XReply: Assertion `!xcb_xlib_extra_reply_data_left' failed.
> Aborted
> 
> Check if there is some extra data left at the end of
> XkbGetKeyboardByName() and discard that data if any is found.
> 
> Many thanks to Peter Hutterer <peter.hutterer at who-t.net> for finding the
> root cause of the issue and Adam Jackson <ajax at redhat.com> for helping
> with the analysis!
> 
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
> v2: bail out if extra data is found, do not return success.

fwiw, I checked the other usages of _XEatData and none of them have a
warning message, so I think we can do the same behaviour here - as long as
we return NULL an error is signalled and we should be fine.

also: note the all-space, not tab, indentation. tabs were expensive in the
80s.

Cheers,
   Peter

> 
>  src/xkb/XKBGetByName.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/src/xkb/XKBGetByName.c b/src/xkb/XKBGetByName.c
> index 973052c..af2536a 100644
> --- a/src/xkb/XKBGetByName.c
> +++ b/src/xkb/XKBGetByName.c
> @@ -28,12 +28,23 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE.
>  #ifdef HAVE_CONFIG_H
>  #include <config.h>
>  #endif
> +#include <stdio.h>
> +
>  #include "Xlibint.h"
>  #include <X11/extensions/XKBproto.h>
>  #include "XKBlibint.h"
>  
>  /***====================================================================***/
>  
> +static void
> +_discardExtraLen(Display *dpy, int extraLen)
> +{
> +    fprintf(stderr,
> +	"XkbGetKeyboardByName: server returned wrong data, discarding %i bytes\n",
> +	extraLen);
> +    _XEatData(dpy, extraLen);
> +}
> +
>  XkbDescPtr
>  XkbGetKeyboardByName(Display *dpy,
>                       unsigned deviceSpec,
> @@ -44,7 +55,7 @@ XkbGetKeyboardByName(Display *dpy,
>  {
>      register xkbGetKbdByNameReq *req;
>      xkbGetKbdByNameReply rep;
> -    int len, extraLen;
> +    int len, extraLen = 0;
>      char *str;
>      XkbDescPtr xkb;
>      int mapLen, codesLen, typesLen, compatLen;
> @@ -204,12 +215,16 @@ XkbGetKeyboardByName(Display *dpy,
>          if (status != Success)
>              goto BAILOUT;
>      }
> +    if (extraLen > 0)
> +	goto BAILOUT;
>      UnlockDisplay(dpy);
>      SyncHandle();
>      return xkb;
>   BAILOUT:
>      if (xkb != NULL)
>          XkbFreeKeyboard(xkb, XkbAllComponentsMask, xTrue);
> +    if (extraLen > 0)
> +	_discardExtraLen(dpy, extraLen);
>      UnlockDisplay(dpy);
>      SyncHandle();
>      return NULL;
> -- 
> 2.5.0
>