GNOME Bugzilla – Bug 774613
X11 lock handling sucks because snprintf does too
Last modified: 2016-11-17 13:37:44 UTC
Whilst tracking this down for Weston, I noticed that Mutter also got tripped up because snprintf sucks. Specifically, it: * accepts total size including trailing NUL byte as an input argument * returns total size _excluding_ trailing NUL byte * ... returns total size of string, not total string as actually written to the buffer This meant that the input size was wrong, and the check on the return value was useless as it did not account for the off-by-one/clamping. End result was that the lock file got written without the trailing LF, which is harmless but different to both Xorg and Mutter. The attached patch fixes that, and also ensures that the input buffer is NUL-terminated, to avoid 'echo aaaaaaaaaaaaaaaa > /tmp/.X0-lock' breaking anything.
*different to both Xorg and Weston
Created attachment 340130 [details] [review] xwayland: Fix lockfile size confusion Similarly to Weston (where this code originated), there were two errors in the X11 lockfile handling. Firstly, after reading 11 characters from the lock file (which could have been placed by any process), there was no guarantee of NUL-termination, meaning strtol could've theoretically run off the end of the string. Secondly, whilst writing the new lock, the trailing NUL byte was not correctly accounted for. The size passed as an input to snprintf takes the maximum size of the string including the trailing NUL, whilst the return (and the input to write) gives the actual size of the string without the trailing NUL. The code did attempt to check the return value, however snprintf returns the size of the _potential_ string written, before snprintf culls it, so this was off by one, and the LF was not being written. Signed-off-by: Daniel Stone <daniels@collabora.com>
The one upside to the old behaviour, however, is that until a couple of days ago, Weston choked on anything but \0 as the final character, so the broken lock writing was kind of beneficial there.
Review of attachment 340130 [details] [review]: OK
Oh, and I don't have a GNOME account, so can't commit myself
Attachment 340130 [details] pushed as f99a086 - xwayland: Fix lockfile size confusion Ah OK, no problem :-)
Thanks Florian!