[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH] used ring: define the meaning and requirements of the len field.
"Michael S. Tsirkin" <mst@redhat.com> writes: > On Tue, Apr 07, 2015 at 10:49:54AM +0930, Rusty Russell wrote: >> "Michael S. Tsirkin" <mst@redhat.com> writes: >> > On Fri, Mar 20, 2015 at 11:48:55AM +1030, Rusty Russell wrote: >> >> We said what it was for, and noted why. We didn't place any requirements >> >> on it, nor clearly spell out the implications of its use. >> >> >> >> This clarification comes particularly from noticing that QEMU didn't >> >> set len correctly, and philosophising over the correct value when >> >> an error has occurred. >> > >> > I'm having second thoughts about this philosophising here :) >> >> That's what philosophising is all about, I think! >> >> > Imagine this driver with untrusted buffers used by multiple >> > applications. Assume application 1 uses the device. >> > We get len value from device but with your change applied, we don't >> > really know whether any bytes > len have been modified. >> > Passing them to application 2 will thus leak some info from application >> > 1 - thus driver must zero out the rest of buffer before reuse. >> >> I don't see it. Program1 and Program2 read from /dev/fubar. >> Internally, /dev/fubar reuses a 4k buffer. >> >> virtio says 1k was written to the buffer, so 1k is copied to Program1. >> Buffer is reposted, and virtio says 2k was written to the buffer, so 2k >> is copied to Program2. >> >> If Program2 has some other way of accessing the left over buffer, that's >> a problem. But I think that will always require special driver >> handling. >> >> Cheers, >> Rusty. > > I think it's the presence of copies that makes the problem > disappear. But the original problem also wasn't there > with copies I think. > > Here's what I had in mind without copies: > > Program 1 initializes buffer and supplies buffer to driver > (e.g. len = read(buf, 2048)). driver does get user pages and gives it to > device, which writes to first 2K but tells driver only first 1K is > modified. Now len == 1K, so Program 1 assumes data after first 1K > is unmodified. Program 1 removes some private data from first 1K but > assumes following 1K is unmodified and copies whole buffer to program 2, > leaking info in the second 1K. Why copy whole 2K you ask? > Maybe exact length is also private info we don't want to leak. That's quite a convoluted scenario. But yes, the driver would have to zero the rest, and that's made clear by the proposed change: +The driver MUST NOT make assumptions about data in device-writable buffers +beyond the first \field{len} bytes, and SHOULD ignore this data. Far worse is if the device returns 2k when it's only written 1k. Then the (more straightforward) case of Program 1 reading 2k, and the driver allocating an unzeroed buffer and returning 2k of data. Cheers, Rusty.
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]