On Wed, Jun 12, 2019 at 04:34:21PM -0400, Tom Talpey wrote: > On 6/12/2019 4:13 PM, Jason Gunthorpe wrote: > > On Wed, Jun 12, 2019 at 04:07:53PM -0400, Tom Talpey wrote: > > > On 6/12/2019 11:21 AM, Jason Gunthorpe wrote: > > > > On Tue, Jun 11, 2019 at 11:11:08AM -0400, Tom Talpey wrote: > > > > > On 6/11/2019 9:21 AM, Bernard Metzler wrote: > > > > > > Hi all, > > > > > > > > > > > > If enabled for siw, during receive operation, a crc32c over > > > > > > header and data is being generated and checked. So far, siw > > > > > > was generating that CRC from the content of the just written > > > > > > target buffer. What kept me busy last weekend were spurious > > > > > > CRC errors, if running qperf. I finally found the application > > > > > > is constantly writing the target buffer while data are placed > > > > > > concurrently, which sometimes races with the CRC computation > > > > > > for that buffer, and yields a broken CRC. > > > > > > > > > > Well, that's a clear bug in the application, assuming siw has > > > > > not yet delivered a send completion for the operation using > > > > > the buffer. This is a basic Verbs API contract. > > > > > > > > May be so, but a kernel driver must not make any assumptions about the > > > > content of memory controlled by user. So it is clearly wrong to write > > > > data to a user buffer and then read it again to compute a CRC. > > > > > > But it's not a user buffer. It's been mapped into the kernel for the > > > purpose of registering and performing data transfer This is standard > > > i/o processing. Both kernel and user have access. > > > > It is a user buffer because the user has access. In fact it may not > > even be mapped into the kernel address space. > > Belaboring this point a bit, but SIW certainly maps it, in order to > copy. An adapter maps it, via dma_map, in order to do the same. My > point is simply that if the kernel tried to prevent that, the whole > i/o model would break down. > > In other words, if a hardware adapter were doing this same thing, > would you consider it out of spec? If so, why? Yes. HW placing/reading data in user controlled memory and requiring it not to change dynamically is a security flaw. It may not be expressly specified as part of the IBA, but it is certainly wrong from a system design perspective. Jason
