Re: [Xen-devel] [Qemu-devel] [PATCH v3 00/25] chardev: Convert qemu_chr_write() to take a size_t argument

[Eric Blake <eblake@xxxxxxxxxx>]

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

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