linux-kernel - Re: [PATCH v4 1/3] USB: use genalloc for USB HCs with local memory

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Tue, 21 May 2019 10:16:57 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     laurentiu.tudor@....com
Cc:     hch@....de, stern@...land.harvard.edu, linux-usb@...r.kernel.org,
        marex@...x.de, leoyang.li@....com, linux-kernel@...r.kernel.org,
        robin.murphy@....com, noring@...rew.org, JuergenUrban@....de
Subject: Re: [PATCH v4 1/3] USB: use genalloc for USB HCs with local memory

On Thu, May 16, 2019 at 02:47:19PM +0300, laurentiu.tudor@....com wrote:
> From: Laurentiu Tudor <laurentiu.tudor@....com>
> 
> For HCs that have local memory, replace the current DMA API usage
> with a genalloc generic allocator to manage the mappings for these
> devices.
> This is in preparation for dropping the existing "coherent" dma
> mem declaration APIs. Current implementation was relying on a short
> circuit in the DMA API that in the end, was acting as an allocator
> for these type of devices.
> 
> For context, see thread here: https://lkml.org/lkml/2019/4/22/357
> 
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@....com>
> ---
>  drivers/usb/core/buffer.c   | 15 +++++++++++----
>  drivers/usb/host/ohci-hcd.c | 23 ++++++++++++++++++-----
>  include/linux/usb/hcd.h     |  3 +++
>  3 files changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
> index f641342cdec0..22a8f3f5679b 100644
> --- a/drivers/usb/core/buffer.c
> +++ b/drivers/usb/core/buffer.c
> @@ -16,6 +16,7 @@
>  #include <linux/io.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dmapool.h>
> +#include <linux/genalloc.h>
>  #include <linux/usb.h>
>  #include <linux/usb/hcd.h>
>  
> @@ -124,10 +125,12 @@ void *hcd_buffer_alloc(
>  	if (size == 0)
>  		return NULL;
>  
> +	if (hcd->driver->flags & HCD_LOCAL_MEM)
> +		return gen_pool_dma_alloc(hcd->localmem_pool, size, dma);

Does this patch now break things?  hcd->localmem_pool at this point in
time is NULL, so this call will fail.  There's no chance for any host
controller driver to actually set up this pool in this patch, so is
bisection broken?

I think you fix this up in later patches, right?

And if so, why do we even need HCD_LOCAL_MEM anymore?  Can't we just
test for the presence of hcd->localmem_pool in order to determine which
allocation method to use?

thanks,

greg k-h