[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 00/25] chardev: Convert qemu_chr_write() to t
Re: [Qemu-devel] [PATCH v3 00/25] chardev: Convert qemu_chr_write() to take a size_t argument
Wed, 20 Feb 2019 08:20:04 -0600
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0
On 2/20/19 5:30 AM, Daniel P. Berrangé wrote:
>> Since Paolo you suggested the change, could you give some convincing
>> arguments that it's worth taking the plunge?
> The chardev write/read methods will end up calling libc read/write
> methods, whose parameters are "size_t count".
In my mind, that's the convincing reason. We should model our read/write
after the libc read/write, which means size_t input and ssize_t returns.
> Thus if there is QEMU code that could currently (mistakenly) pass a
> negative value for length to qemu_chr_write, unless something stops
> it, this is going to be cast to a size_t when we finally call read/
> write on the FD, leading to a large positive value & array out of
> bounds read/write.
> IOW we already have inconsistent use of signed vs unsigned in our code
> which has potential to cause bugs. Converting chardev to use size_t
> we get rid fo the mismatch with the underlying libc APIs we call,
> which ultimately eliminates an area of risk longer term. There is a
> chance it could uncover some pre-existing dormant bugs, but provided
> we do due diligence to check callers I think its a win to be consistent
> with libc APIs in size_t usage for read/write.
And hopefully this exercise of making the conversion serves as a good
audit to help us gain confidence in our code and/or fix bugs it uncovers.
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- Re: [Qemu-devel] [PATCH v3 18/25] s390x/3270: Let insert_IAC_escape_char() use size_t, (continued)