On Tue, Mar 22, 2022 at 11:02:17AM +0100, Halil Pasic wrote: > The problem I'm addressing was discovered by the LTP test covering > cve-2018-1000204. > > A short description of what happens follows: > 1) The test case issues a command code 00 (TEST UNIT READY) via the SG_IO > interface with: dxfer_len == 524288, dxdfer_dir == SG_DXFER_FROM_DEV > and a corresponding dxferp. The peculiar thing about this is that TUR > is not reading from the device. > 2) In sg_start_req() the invocation of blk_rq_map_user() effectively > bounces the user-space buffer. As if the device was to transfer into > it. Since commit a45b599ad808 ("scsi: sg: allocate with __GFP_ZERO in > sg_build_indirect()") we make sure this first bounce buffer is > allocated with GFP_ZERO. > 3) For the rest of the story we keep ignoring that we have a TUR, so the > device won't touch the buffer we prepare as if the we had a > DMA_FROM_DEVICE type of situation. My setup uses a virtio-scsi device > and the buffer allocated by SG is mapped by the function > virtqueue_add_split() which uses DMA_FROM_DEVICE for the "in" sgs (here > scatter-gather and not scsi generics). This mapping involves bouncing > via the swiotlb (we need swiotlb to do virtio in protected guest like > s390 Secure Execution, or AMD SEV). > 4) When the SCSI TUR is done, we first copy back the content of the second > (that is swiotlb) bounce buffer (which most likely contains some > previous IO data), to the first bounce buffer, which contains all > zeros. Then we copy back the content of the first bounce buffer to > the user-space buffer. > 5) The test case detects that the buffer, which it zero-initialized, > ain't all zeros and fails. > > One can argue that this is an swiotlb problem, because without swiotlb > we leak all zeros, and the swiotlb should be transparent in a sense that > it does not affect the outcome (if all other participants are well > behaved). > > Copying the content of the original buffer into the swiotlb buffer is > the only way I can think of to make swiotlb transparent in such > scenarios. So let's do just that if in doubt, but allow the driver > to tell us that the whole mapped buffer is going to be overwritten, > in which case we can preserve the old behavior and avoid the performance > impact of the extra bounce. > > Signed-off-by: Halil Pasic <pasic@xxxxxxxxxxxxx> > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > [pasic@xxxxxxxxxxxxx: resolved merge conflicts] > --- > Documentation/core-api/dma-attributes.rst | 8 ++++++++ > include/linux/dma-mapping.h | 8 ++++++++ > kernel/dma/swiotlb.c | 3 ++- > 3 files changed, 18 insertions(+), 1 deletion(-) What is the git commit id of this patch in Linus's tree?