linux-kernel - Re: [PATCH v3 0/2] media: uvcvideo: Code cleanup for dev->status

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 16 Dec 2022 10:45:19 -0500
From:   Alan Stern <stern@...land.harvard.edu>
To:     Ricardo Ribalda <ribalda@...omium.org>
Cc:     Max Staudt <mstaudt@...omium.org>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Ming Lei <tom.leiming@...il.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Yunke Cao <yunkec@...omium.org>,
        Christoph Hellwig <hch@....de>, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-media@...r.kernel.org
Subject: Re: [PATCH v3 0/2] media: uvcvideo: Code cleanup for dev->status

On Fri, Dec 16, 2022 at 09:55:09AM +0100, Ricardo Ribalda wrote:
> Hi Alan
> 
> On Thu, 15 Dec 2022 at 16:34, Alan Stern <stern@...land.harvard.edu> wrote:
> >
> > On Thu, Dec 15, 2022 at 11:57:17AM +0100, Ricardo Ribalda wrote:
> > > There is no need to make a kzalloc just for 16 bytes. Let's embed the data
> > > into the main data structure.
> > >
> > > Now that we are at it, lets remove all the castings and open coding of
> > > offsets for it.
> > >
> > > [Christoph, do you think dma wise we are violating any non written rules? :) thanks]
> >
> > There _is_ a rule, and it is not exactly unwritten.  The kerneldoc for
> > the transfer_buffer member of struct urb says:
> >
> >         This buffer must be suitable for DMA; allocate it with
> >         kmalloc() or equivalent.
> >
> > Which in general means that the buffer must not be part of a larger
> > structure -- not unless the driver can guarantee that the structure will
> > _never_ be accessed while a USB transfer to/from the buffer is taking
> > place.
> >
> 
> Thanks a lot for the clarification. I was mainly looking at the kerneldoc from:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/usb.h#n1687
> 
> and I could not see any reference to the DMA requirements.
> 
> Mind if I send a patch to add a reference there?

Not at all.  But if you change the documentation for usb_fill_int_urb() 
then you should also change it for usb_fill_control_urb() and 
usb_fill_bulk_urb().

Alan Stern

> > There are examples all over the USB subsystem where buffers as small as
> > one or two bytes get kmalloc'ed in order to obey this rule.  16 bytes is
> > certainly big enough that you shouldn't worry about it being allocated
> > separately.
> >
> Yep, we should keep it malloced. Thanks a lot for looking into this :)
> 
> 
> > Alan Stern
> 
> 
> 
> -- 
> Ricardo Ribalda