XKB key names are limited to four octets. Initially I didn't know that, and tried to use longer names, like this: $ cat t0.xkb xkb_keymap { xkb_keycodes "test_keycodes" { minimum = 8; maximum = 255; <FOOBAR> = 10; }; xkb_types "complete" { include "complete" }; xkb_compatibility "complete" { include "complete" }; xkb_symbols "us" { include "us" }; xkb_geometry "kinesis" { include "kinesis" }; }; $ xkbcomp t0.xkb t0.xkm syntax error: line 5 of t0.xkb last scanned symbol is: FOOBARm Errors encountered in t0.xkb; not compiled. There's obviously a nul-termination problem in that error message. Possibly overrunning a fixed-size buffer too? -zefram
http://patchwork.freedesktop.org/patch/14036/
commit cdcd552041fc1325a2a81e3374fadb0dd15950dc Author: Peter Hutterer <peter.hutterer@who-t.net> Date: Thu Jul 11 13:26:18 2013 +1000 Always terminate the scanBuf string (#66345)
Hmm. The fix doesn't seem fully correct to me. http://cgit.freedesktop.org/xorg/app/xkbcomp/commit/?id=cdcd552041fc1325a2a81e3374fadb0dd15950dc Shouldn't the line + if (i < sizeof(scanBuf) - i) read instead: + if (i < sizeof(scanBuf) - 1) ? But... when this if is executed, i *is* at the very most sizeof(scanBuf) - 1, so I don't see what the use of last is gaining us here. Wouldn't it be enough to just move the "scanBuf[i++] = '\0';" out of the next if statement? If so, maybe this should also be done for yyGetString? The relevant snippet from above commit: @@ -466,12 +467,20 @@ yyGetKeyName(void) if (i < sizeof(scanBuf) - 1) scanBuf[i++] = ch; } + + if (i < sizeof(scanBuf) - i) + last = i; + else + last = sizeof(scanBuf) - 1; + + scanBuf[last] = '\0'; + if ((ch == '>') && (i < 5)) { - scanBuf[i++] = '\0'; scanStrLine = lineNum; return KEYNAME; }
(In reply to comment #3) > Shouldn't the line > + if (i < sizeof(scanBuf) - i) > read instead: > + if (i < sizeof(scanBuf) - 1) whoops, yes, you're right. > But... when this if is executed, i *is* at the very most sizeof(scanBuf) - 1, > so I don't see what the use of last is gaining us here. Wouldn't it be > enough > to just move the "scanBuf[i++] = '\0';" out of the next if statement? looks like I was overcomplicating things. you're right. please attach a patch for that > If so, maybe this should also be done for yyGetString? yep
Created attachment 84807 [details] [review] correct handling of overlong keynames/strings The requested patch.
commit 24d18e0a844041ef82441adb16aa18cc4b4814ae Author: Benno Schulenberg <bensberg@justemail.net> Date: Wed Aug 28 20:03:30 2013 +0200 Making sure that a copied string is always null-terminated (#66345).
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.