| 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
| ||
|
Message-ID: <20191114025301.GD30608@b29397-desktop>
Date: Thu, 14 Nov 2019 02:50:42 +0000
From: Peter Chen <peter.chen@....com>
To: Jayshri Pawar <jpawar@...ence.com>
CC: "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"felipe.balbi@...ux.intel.com" <felipe.balbi@...ux.intel.com>,
"heikki.krogerus@...ux.intel.com" <heikki.krogerus@...ux.intel.com>,
"rogerq@...com" <rogerq@...com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jbergsagel@...com" <jbergsagel@...com>,
"nsekhar@...com" <nsekhar@...com>, "nm@...com" <nm@...com>,
"kurahul@...ence.com" <kurahul@...ence.com>,
"pawell@...ence.com" <pawell@...ence.com>,
"sparmar@...ence.com" <sparmar@...ence.com>
Subject: Re: [RFC PATCH] usb: gadget: f_tcm: Added DMA32 flag while allocation
of command buffer
On 19-11-13 10:24:32, Jayshri Pawar wrote:
> There is a problem when function driver allocate memory for buffer
> used by DMA from outside dma_mask space.
> It appears during testing f_tcm driver with cdns3 controller.
> In the result cdns3 driver was not able to map virtual buffer to DMA.
> This fix should be improved depending on dma_mask associated with device.
> Adding GFP_DMA32 flag while allocationg command data buffer only for 32
> bit controllers.
Hi Jayshri,
This issue should be fixed by setting DMA_MASK correctly for controller,
you can't limit user's memory region. At usb_ep_queue, the UDC driver
will call DMA MAP API, for Cadence, it is usb_gadget_map_request_by_dev.
For the system without SMMU (IO-MMU), it will use swiotlb to make sure
the data buffer used for DMA transfer is within DMA mask for controller,
There is a reserved low memory region for debounce buffer in swiotlb
use case.
Peter
>
> Signed-off-by: Pawel Laszczak <pawell@...ence.com>
> Signed-off-by: Jayshri Pawar <jpawar@...ence.com>
> ---
> drivers/usb/gadget/function/f_tcm.c | 20 ++++++++++++++------
> include/linux/usb/gadget.h | 2 ++
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 36504931b2d1..a78d5fad3d84 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -213,7 +213,8 @@ static int bot_send_read_response(struct usbg_cmd *cmd)
> }
>
> if (!gadget->sg_supported) {
> - cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC);
> + cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC |
> + gadget->dma_flag);
> if (!cmd->data_buf)
> return -ENOMEM;
>
> @@ -257,7 +258,8 @@ static int bot_send_write_request(struct usbg_cmd *cmd)
> }
>
> if (!gadget->sg_supported) {
> - cmd->data_buf = kmalloc(se_cmd->data_length, GFP_KERNEL);
> + cmd->data_buf = kmalloc(se_cmd->data_length, GFP_KERNEL |
> + gadget->dma_flag);
> if (!cmd->data_buf)
> return -ENOMEM;
>
> @@ -305,6 +307,7 @@ static void bot_cmd_complete(struct usb_ep *ep, struct usb_request *req)
> static int bot_prepare_reqs(struct f_uas *fu)
> {
> int ret;
> + struct usb_gadget *gadget = fuas_to_gadget(fu);
>
> fu->bot_req_in = usb_ep_alloc_request(fu->ep_in, GFP_KERNEL);
> if (!fu->bot_req_in)
> @@ -327,7 +330,8 @@ static int bot_prepare_reqs(struct f_uas *fu)
> fu->bot_status.req->complete = bot_status_complete;
> fu->bot_status.csw.Signature = cpu_to_le32(US_BULK_CS_SIGN);
>
> - fu->cmd.buf = kmalloc(fu->ep_out->maxpacket, GFP_KERNEL);
> + fu->cmd.buf = kmalloc(fu->ep_out->maxpacket, GFP_KERNEL |
> + gadget->dma_flag);
> if (!fu->cmd.buf)
> goto err_buf;
>
> @@ -515,7 +519,8 @@ static int uasp_prepare_r_request(struct usbg_cmd *cmd)
> struct uas_stream *stream = cmd->stream;
>
> if (!gadget->sg_supported) {
> - cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC);
> + cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC |
> + gadget->dma_flag);
> if (!cmd->data_buf)
> return -ENOMEM;
>
> @@ -763,11 +768,13 @@ static int uasp_alloc_stream_res(struct f_uas *fu, struct uas_stream *stream)
>
> static int uasp_alloc_cmd(struct f_uas *fu)
> {
> + struct usb_gadget *gadget = fuas_to_gadget(fu);
> fu->cmd.req = usb_ep_alloc_request(fu->ep_cmd, GFP_KERNEL);
> if (!fu->cmd.req)
> goto err;
>
> - fu->cmd.buf = kmalloc(fu->ep_cmd->maxpacket, GFP_KERNEL);
> + fu->cmd.buf = kmalloc(fu->ep_cmd->maxpacket, GFP_KERNEL |
> + gadget->dma_flag);
> if (!fu->cmd.buf)
> goto err_buf;
>
> @@ -980,7 +987,8 @@ static int usbg_prepare_w_request(struct usbg_cmd *cmd, struct usb_request *req)
> struct usb_gadget *gadget = fuas_to_gadget(fu);
>
> if (!gadget->sg_supported) {
> - cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC);
> + cmd->data_buf = kmalloc(se_cmd->data_length, GFP_ATOMIC |
> + gadget->dma_flag);
> if (!cmd->data_buf)
> return -ENOMEM;
>
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 124462d65eac..d6c9cd222600 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -373,6 +373,7 @@ struct usb_gadget_ops {
> * @connected: True if gadget is connected.
> * @lpm_capable: If the gadget max_speed is FULL or HIGH, this flag
> * indicates that it supports LPM as per the LPM ECN & errata.
> + * @dma_flag: dma zone to be used for buffer allocation.
> *
> * Gadgets have a mostly-portable "gadget driver" implementing device
> * functions, handling all usb configurations and interfaces. Gadget
> @@ -427,6 +428,7 @@ struct usb_gadget {
> unsigned deactivated:1;
> unsigned connected:1;
> unsigned lpm_capable:1;
> + unsigned int dma_flag;
> };
> #define work_to_gadget(w) (container_of((w), struct usb_gadget, work))
>
> --
> 2.20.1
>
--
Thanks,
Peter Chen
Powered by blists - more mailing lists