qemu-s390x
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [qemu-s390x] [Qemu-devel] [PATCH v3 00/25] chardev: Convert qemu_chr


From: Eric Blake
Subject: Re: [qemu-s390x] [Qemu-devel] [PATCH v3 00/25] chardev: Convert qemu_chr_write() to take a size_t argument
Date: Wed, 20 Feb 2019 08:20:04 -0600
User-agent: 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]