Re: [PATCH v5 20/38] kmsan: handle memory sent to/from USB

From: Alan Stern <stern@rowland.harvard.edu>
To: Andrey Konovalov <andreyknvl@google.com>
Cc: Alexander Potapenko <glider@google.com>,
	 Andrew Morton <akpm@linux-foundation.org>,
	 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	 Eric Dumazet <edumazet@google.com>,
	Wolfram Sang <wsa@the-dreams.de>,  Petr Mladek <pmladek@suse.com>,
	Vegard Nossum <vegard.nossum@oracle.com>,
	 Dmitry Vyukov <dvyukov@google.com>,
	Marco Elver <elver@google.com>,
	 Linux Memory Management List <linux-mm@kvack.org>,
	 Alexander Viro <viro@zeniv.linux.org.uk>,
	 Andreas Dilger <adilger.kernel@dilger.ca>,
	 Andrey Ryabinin <aryabinin@virtuozzo.com>,
	 Andy Lutomirski <luto@kernel.org>,
	 Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Arnd Bergmann <arnd@arndb.de>,
	 Christoph Hellwig <hch@infradead.org>,
	Christoph Hellwig <hch@lst.de>,
	 "Darrick J. Wong" <darrick.wong@oracle.com>,
	 "David S. Miller" <davem@davemloft.net>,
	 Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	 Eric Biggers <ebiggers@google.com>,
	Eric Van Hensbergen <ericvh@gmail.com>,
	 Harry Wentland <harry.wentland@amd.com>,
	 Herbert Xu <herbert@gondor.apana.org.au>,
	 Ilya Leoshkevich <iii@linux.ibm.com>,
	Ingo Molnar <mingo@elte.hu>,  Jason Wang <jasowang@redhat.com>,
	Jens Axboe <axboe@kernel.dk>,
	 Marek Szyprowski <m.szyprowski@samsung.com>,
	 Mark Rutland <mark.rutland@arm.com>,
	 "Martin K. Petersen" <martin.petersen@oracle.com>,
	 Martin Schwidefsky <schwidefsky@de.ibm.com>,
	 Matthew Wilcox <willy@infradead.org>,
	 "Michael S . Tsirkin" <mst@redhat.com>,
	Michal Hocko <mhocko@suse.com>,  Michal Simek <monstr@monstr.eu>,
	Qian Cai <cai@lca.pw>,  Randy Dunlap <rdunlap@infradead.org>,
	Robin Murphy <robin.murphy@arm.com>,
	 Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	 Steven Rostedt <rostedt@goodmis.org>,
	Takashi Iwai <tiwai@suse.com>,  Theodore Ts'o <tytso@mit.edu>,
	Thomas Gleixner <tglx@linutronix.de>,
	 Vasily Gorbik <gor@linux.ibm.com>
Subject: Re: [PATCH v5 20/38] kmsan: handle memory sent to/from USB
Date: Tue, 14 Apr 2020 16:45:38 -0400 (EDT)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.2004141641400.4717-100000@netrider.rowland.org> (raw)
In-Reply-To: <CAAeHK+xcwbrAZmg27iKzvaHM_0W-m2e_n9hiedf=+2Q0gySK5Q@mail.gmail.com>

On Tue, 14 Apr 2020, Andrey Konovalov wrote:

> On Tue, Apr 14, 2020 at 5:50 PM Alan Stern <stern@rowland.harvard.edu> wrote:

> > This has got a couple of problems.
> >
> > Firstly, for control URBs it doesn't check urb->setup_packet, which
> > should always be initialized regardless of the direction because it
> > always gets sent to the device.
> 
> Ah, yes, we actually had an info-leak report for a bug in the sound
> subsystem that was leaking data this way, which was misattributed to
> raw-gadget. This makes sense to add and seems quite easy.
> 
> > Secondly, some URBs use scatter-gather transfers, and they don't always
> > store the buffer address in urb->transfer_buffer (indeed, sometimes the
> > buffer is located outside of the kernel's address map).  Instead they
> > use urb->sg and urb->num_sgs.  To get an idea for how it all works,
> > look at usb_hcd_map_urb_for_dma()  in hcd.c.
> 
> Oh, this is something we need to look into.
> 
> > Thirdly, the information we get back from the device doesn't always
> > cover the entire transfer buffer; sometimes the device sends less data
> > than we asked for.  Perhaps you don't care very much about this case.
> 
> So you mean that we should look at actual_length rather than
> transfer_buffer_length when initializing memory that comes from the
> device?

Yes, in theory.  Isochronous transfers are difficult, because the data 
doesn't have to be contiguous in memory.  If you want to handle them 
properly, you have to loop through the entries in the 
urb->iso_frame_desc[] array and treat them individually.

Alan Stern