[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] console: avoid buffer overflow in guest_console_write()
On 29.11.2019 11:22, Andrew Cooper wrote: > On 29/11/2019 10:13, Jan Beulich wrote: >> The switch of guest_console_write()'s second parameter from plain to >> unsigned int has caused the function's main loop header to no longer >> guard the min_t() use within the function against effectively negative >> values, due to the casts hidden inside the macro. Replace by a plain >> min(), converting one of the arguments suitably without involving any >> cast. >> >> Fixes: ea601ec9995b ("xen/console: Rework HYPERCALL_console_io interface") >> Reported-by: Ilja Van Sprundel <ivansprundel@xxxxxxxxxxxx> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> >> >> --- a/xen/drivers/char/console.c >> +++ b/xen/drivers/char/console.c >> @@ -538,7 +538,7 @@ static long guest_console_write(XEN_GUES >> __HYPERVISOR_console_io, "iih", >> CONSOLEIO_write, count, buffer); >> >> - kcount = min_t(int, count, sizeof(kbuf)-1); >> + kcount = min(count + sizeof(char[0]), sizeof(kbuf) - 1); > > Is sizeof(array[0]) always 0, or is this just a GCC-ism ? Godbolt > suggests is 0 on all compiler we support. > > Either way, isn't the more common idiom + 0ul ? Personally, I feel that > is clearer to follow. I decided against + 0ul or alike because in principle size_t and unsigned long are different types. In particular 32-bit x86 gcc uses unsigned int for size_t, and hence min()'s type safety check would cause the build to fail there. The same risk obviously exists for any 32-bit arch (e.g. Arm32, but I haven't checked what type it actually uses). > That said, given the severity and urgency of this > extremely-lucky-its-not-an-XSA, Reviewed-by: Andrew Cooper > <andrew.cooper3@xxxxxxxxxx>, but ideally using the +0ul form. Thanks. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |