On Wed, Jun 13, 2018 at 02:09:09 +0000, Valeriy E. Ushakov wrote: > Module Name: src > Committed By: uwe > Date: Wed Jun 13 02:09:09 UTC 2018 > > Modified Files: > src/sys/dev/wscons: wsevent.c > > Log Message: > wsevent_copyout_events50 - don't leak garbage from the kernel stack. > > On 64-bit machines struct timespec50 has padding between 32-bit tv_sec > and long tv_nsec that is not affected by normal assignment. Scrub it > before we uiomove struct owscons_event.
I was looking at mouse events on an amd64 VM with # hexdump -e '/4 " %2d" /4 " %5d" /8 " %d" /8 ".%09d" "\n"' /dev/wsmouse note: wscons event sources give you compat event structs unless you request the current version with an ioctl (which is kinda hard to do in hexdump :). I noticed that the first reported event always had bogus timestamp. Took me a bit of time to realize what was going on. I fixed it in wsevent.c (indentation reduced for readability): +#if INT32_MAX < LONG_MAX /* scrub padding */ + memset(&ev50.time, 0, offsetof(struct timespec50, tv_nsec)); +#endif timespec_to_timespec50(&ev->time, &ev50.time); but I wonder if this scrubbing should be moved into timespec_to_timespec50() - after all the most likley use of the compat struct is to write or copyout it in the compat code, so the same problem probably happens elsewhere. On amd64 the compiler is smart enough to convert memset() to a few movq's. The compiler is not smart enough to notice that tv_nsec is written to in timespec_to_timespec50(), so memset(&ev50.time, 0, sizeof(ev50.time)); timespec_to_timespec50(...); would still emit two movq's immediately followed by another movq to tv_nsec. Hence this specific arguments in the call to memset(). Comments? PS: The next logical question is if there's a tool that can help audit the rest of the kernel for problems like that. :) -uwe