Re: [Xen-devel] [PATCH] console: avoid buffer overflow in guest_console_write()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.11.2019 13:37, Andrew Cooper wrote:
> On 29/11/2019 12:19, Jan Beulich wrote:
>> On 29.11.2019 13:15, Andrew Cooper wrote:
>>> On 29/11/2019 12:13, Jan Beulich wrote:
>>>> On 29.11.2019 13:01, Ian Jackson wrote:
>>>>> Jan Beulich writes ("Re: [PATCH] console: avoid buffer overflow in 
>>>>> guest_console_write()"):
>>>>>> On 29.11.2019 11:22, Andrew Cooper wrote:
>>>>>>> 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).
>>>>> I don't know what i wrong with
>>>>>    (size_t)0
>>>>> which is shorter, even !
>>>> True. Yet it contains a cast, no matter how risk-free it may be
>>>> in this case. With a cast, I could as well have written (yet
>>>> shorter) (size_t)count.
>>> Given that min() has a very strict typecheck, I think we should permit
>>> any use of an explicit cast in a single operand, because it *is* safer
>>> than switching to the min_t() route to make things compile.
>> Well, I can switch to (size_t)count if this is liked better
>> overall.
> 
> Personally, I'd prefer this option most of all.

Okay, I've switched to this, but while doing so I started wondering
why we'd then not use

        kcount = min(count, (unsigned int)sizeof(kbuf) - 1);

which is an (often slightly cheaper) 32-bit operation (and which
is what I had actually started from).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel