* [PATCH] usb: gadget: mv_u3d: add check for dma mapping error
@ 2016-10-28 23:12 Alexey Khoroshilov
2016-11-03 8:52 ` Felipe Balbi
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Khoroshilov @ 2016-10-28 23:12 UTC (permalink / raw)
To: Felipe Balbi
Cc: Alexey Khoroshilov, Greg Kroah-Hartman, linux-usb, linux-kernel,
ldv-project
mv_u3d_req_to_trb() does not check for dma mapping errors.
By the way, the patch improves readability of mv_u3d_start_queue()
by rearranging its code with two semantic modifications:
- assignment zero to ep->processing if usb_gadget_map_request() fails;
- propagation of error code from mv_u3d_req_to_trb() instead of
hardcoded -ENOMEM.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/usb/gadget/udc/mv_u3d_core.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
index b9e19a591322..8d726bd767fd 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -462,6 +462,12 @@ static int mv_u3d_req_to_trb(struct mv_u3d_req *req)
req->trb_head->trb_hw,
trb_num * sizeof(*trb_hw),
DMA_BIDIRECTIONAL);
+ if (dma_mapping_error(u3d->gadget.dev.parent,
+ req->trb_head->trb_dma)) {
+ kfree(req->trb_head->trb_hw);
+ kfree(req->trb_head);
+ return -EFAULT;
+ }
req->chain = 1;
}
@@ -487,30 +493,32 @@ mv_u3d_start_queue(struct mv_u3d_ep *ep)
ret = usb_gadget_map_request(&u3d->gadget, &req->req,
mv_u3d_ep_dir(ep));
if (ret)
- return ret;
+ goto break_processing;
req->req.status = -EINPROGRESS;
req->req.actual = 0;
req->trb_count = 0;
- /* build trbs and push them to device queue */
- if (!mv_u3d_req_to_trb(req)) {
- ret = mv_u3d_queue_trb(ep, req);
- if (ret) {
- ep->processing = 0;
- return ret;
- }
- } else {
- ep->processing = 0;
+ /* build trbs */
+ ret = mv_u3d_req_to_trb(req);
+ if (ret) {
dev_err(u3d->dev, "%s, mv_u3d_req_to_trb fail\n", __func__);
- return -ENOMEM;
+ goto break_processing;
}
+ /* and push them to device queue */
+ ret = mv_u3d_queue_trb(ep, req);
+ if (ret)
+ goto break_processing;
+
/* irq handler advances the queue */
- if (req)
- list_add_tail(&req->queue, &ep->queue);
+ list_add_tail(&req->queue, &ep->queue);
return 0;
+
+break_processing:
+ ep->processing = 0;
+ return ret;
}
static int mv_u3d_ep_enable(struct usb_ep *_ep,
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: gadget: mv_u3d: add check for dma mapping error
2016-10-28 23:12 [PATCH] usb: gadget: mv_u3d: add check for dma mapping error Alexey Khoroshilov
@ 2016-11-03 8:52 ` Felipe Balbi
2016-11-03 13:16 ` [PATCH v2 1/2] " Alexey Khoroshilov
0 siblings, 1 reply; 6+ messages in thread
From: Felipe Balbi @ 2016-11-03 8:52 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Alexey Khoroshilov, Greg Kroah-Hartman, linux-usb, linux-kernel,
ldv-project
[-- Attachment #1: Type: text/plain, Size: 2686 bytes --]
Hi,
Alexey Khoroshilov <khoroshilov@ispras.ru> writes:
> mv_u3d_req_to_trb() does not check for dma mapping errors.
>
> By the way, the patch improves readability of mv_u3d_start_queue()
> by rearranging its code with two semantic modifications:
> - assignment zero to ep->processing if usb_gadget_map_request() fails;
> - propagation of error code from mv_u3d_req_to_trb() instead of
> hardcoded -ENOMEM.
cleanups and fixes should be done separately.
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
> drivers/usb/gadget/udc/mv_u3d_core.c | 34 +++++++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
> index b9e19a591322..8d726bd767fd 100644
> --- a/drivers/usb/gadget/udc/mv_u3d_core.c
> +++ b/drivers/usb/gadget/udc/mv_u3d_core.c
> @@ -462,6 +462,12 @@ static int mv_u3d_req_to_trb(struct mv_u3d_req *req)
> req->trb_head->trb_hw,
> trb_num * sizeof(*trb_hw),
> DMA_BIDIRECTIONAL);
> + if (dma_mapping_error(u3d->gadget.dev.parent,
> + req->trb_head->trb_dma)) {
> + kfree(req->trb_head->trb_hw);
> + kfree(req->trb_head);
> + return -EFAULT;
> + }
>
> req->chain = 1;
> }
this is one patch: add dma_mapping_error() check
AKA $subject :-p
> @@ -487,30 +493,32 @@ mv_u3d_start_queue(struct mv_u3d_ep *ep)
> ret = usb_gadget_map_request(&u3d->gadget, &req->req,
> mv_u3d_ep_dir(ep));
> if (ret)
> - return ret;
> + goto break_processing;
>
> req->req.status = -EINPROGRESS;
> req->req.actual = 0;
> req->trb_count = 0;
>
> - /* build trbs and push them to device queue */
> - if (!mv_u3d_req_to_trb(req)) {
> - ret = mv_u3d_queue_trb(ep, req);
> - if (ret) {
> - ep->processing = 0;
> - return ret;
> - }
> - } else {
> - ep->processing = 0;
> + /* build trbs */
> + ret = mv_u3d_req_to_trb(req);
> + if (ret) {
> dev_err(u3d->dev, "%s, mv_u3d_req_to_trb fail\n", __func__);
> - return -ENOMEM;
> + goto break_processing;
> }
>
> + /* and push them to device queue */
> + ret = mv_u3d_queue_trb(ep, req);
> + if (ret)
> + goto break_processing;
> +
> /* irq handler advances the queue */
> - if (req)
> - list_add_tail(&req->queue, &ep->queue);
> + list_add_tail(&req->queue, &ep->queue);
>
> return 0;
> +
> +break_processing:
> + ep->processing = 0;
> + return ret;
> }
>
> static int mv_u3d_ep_enable(struct usb_ep *_ep,
this is another, unrelated patch. Please split
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error
2016-11-03 8:52 ` Felipe Balbi
@ 2016-11-03 13:16 ` Alexey Khoroshilov
2016-11-03 13:16 ` [PATCH v2 2/2] usb: gadget: mv_u3d: mv_u3d_start_queue() refactoring Alexey Khoroshilov
2016-11-03 13:34 ` [PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error Felipe Balbi
0 siblings, 2 replies; 6+ messages in thread
From: Alexey Khoroshilov @ 2016-11-03 13:16 UTC (permalink / raw)
To: Felipe Balbi
Cc: Alexey Khoroshilov, Greg Kroah-Hartman, linux-usb, linux-kernel,
ldv-project
mv_u3d_req_to_trb() does not check for dma mapping errors.
Found by Linux Driver Verification project (linuxtesting.org).
v2: split fix and clenup to separate patches.
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/usb/gadget/udc/mv_u3d_core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
index b9e19a591322..6f3be0ba9ac8 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -462,6 +462,12 @@ static int mv_u3d_req_to_trb(struct mv_u3d_req *req)
req->trb_head->trb_hw,
trb_num * sizeof(*trb_hw),
DMA_BIDIRECTIONAL);
+ if (dma_mapping_error(u3d->gadget.dev.parent,
+ req->trb_head->trb_dma)) {
+ kfree(req->trb_head->trb_hw);
+ kfree(req->trb_head);
+ return -EFAULT;
+ }
req->chain = 1;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] usb: gadget: mv_u3d: mv_u3d_start_queue() refactoring
2016-11-03 13:16 ` [PATCH v2 1/2] " Alexey Khoroshilov
@ 2016-11-03 13:16 ` Alexey Khoroshilov
2016-11-03 13:34 ` [PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error Felipe Balbi
1 sibling, 0 replies; 6+ messages in thread
From: Alexey Khoroshilov @ 2016-11-03 13:16 UTC (permalink / raw)
To: Felipe Balbi
Cc: Alexey Khoroshilov, Greg Kroah-Hartman, linux-usb, linux-kernel,
ldv-project
The patch improves readability of mv_u3d_start_queue()
by rearranging its code with two semantic modifications:
- assignment zero to ep->processing if usb_gadget_map_request() fails;
- propagation of error code from mv_u3d_req_to_trb() instead of
hardcoded -ENOMEM.
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/usb/gadget/udc/mv_u3d_core.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
index 6f3be0ba9ac8..8d726bd767fd 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -493,30 +493,32 @@ mv_u3d_start_queue(struct mv_u3d_ep *ep)
ret = usb_gadget_map_request(&u3d->gadget, &req->req,
mv_u3d_ep_dir(ep));
if (ret)
- return ret;
+ goto break_processing;
req->req.status = -EINPROGRESS;
req->req.actual = 0;
req->trb_count = 0;
- /* build trbs and push them to device queue */
- if (!mv_u3d_req_to_trb(req)) {
- ret = mv_u3d_queue_trb(ep, req);
- if (ret) {
- ep->processing = 0;
- return ret;
- }
- } else {
- ep->processing = 0;
+ /* build trbs */
+ ret = mv_u3d_req_to_trb(req);
+ if (ret) {
dev_err(u3d->dev, "%s, mv_u3d_req_to_trb fail\n", __func__);
- return -ENOMEM;
+ goto break_processing;
}
+ /* and push them to device queue */
+ ret = mv_u3d_queue_trb(ep, req);
+ if (ret)
+ goto break_processing;
+
/* irq handler advances the queue */
- if (req)
- list_add_tail(&req->queue, &ep->queue);
+ list_add_tail(&req->queue, &ep->queue);
return 0;
+
+break_processing:
+ ep->processing = 0;
+ return ret;
}
static int mv_u3d_ep_enable(struct usb_ep *_ep,
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error
2016-11-03 13:16 ` [PATCH v2 1/2] " Alexey Khoroshilov
2016-11-03 13:16 ` [PATCH v2 2/2] usb: gadget: mv_u3d: mv_u3d_start_queue() refactoring Alexey Khoroshilov
@ 2016-11-03 13:34 ` Felipe Balbi
2016-11-03 18:09 ` Alexey Khoroshilov
1 sibling, 1 reply; 6+ messages in thread
From: Felipe Balbi @ 2016-11-03 13:34 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Alexey Khoroshilov, Greg Kroah-Hartman, linux-usb, linux-kernel,
ldv-project
[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]
Hi,
Alexey Khoroshilov <khoroshilov@ispras.ru> writes:
> mv_u3d_req_to_trb() does not check for dma mapping errors.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> v2: split fix and clenup to separate patches.
I'll fix this time when applying, but keep in mind we don't want these
notes in the commit log. They should come after the tearline (---)
below, together with the diffstat ;-)
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
> drivers/usb/gadget/udc/mv_u3d_core.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c b/drivers/usb/gadget/udc/mv_u3d_core.c
> index b9e19a591322..6f3be0ba9ac8 100644
> --- a/drivers/usb/gadget/udc/mv_u3d_core.c
> +++ b/drivers/usb/gadget/udc/mv_u3d_core.c
> @@ -462,6 +462,12 @@ static int mv_u3d_req_to_trb(struct mv_u3d_req *req)
> req->trb_head->trb_hw,
> trb_num * sizeof(*trb_hw),
> DMA_BIDIRECTIONAL);
> + if (dma_mapping_error(u3d->gadget.dev.parent,
> + req->trb_head->trb_dma)) {
> + kfree(req->trb_head->trb_hw);
> + kfree(req->trb_head);
> + return -EFAULT;
> + }
>
> req->chain = 1;
> }
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error
2016-11-03 13:34 ` [PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error Felipe Balbi
@ 2016-11-03 18:09 ` Alexey Khoroshilov
0 siblings, 0 replies; 6+ messages in thread
From: Alexey Khoroshilov @ 2016-11-03 18:09 UTC (permalink / raw)
To: Felipe Balbi; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, ldv-project
On 03.11.2016 16:34, Felipe Balbi wrote:
>
> Hi,
>
> Alexey Khoroshilov <khoroshilov@ispras.ru> writes:
>> mv_u3d_req_to_trb() does not check for dma mapping errors.
>>
>> Found by Linux Driver Verification project (linuxtesting.org).
>>
>> v2: split fix and clenup to separate patches.
>
> I'll fix this time when applying, but keep in mind we don't want these
> notes in the commit log. They should come after the tearline (---)
> below, together with the diffstat ;-)
ok, thank you
--
Alexey
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-03 18:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-28 23:12 [PATCH] usb: gadget: mv_u3d: add check for dma mapping error Alexey Khoroshilov
2016-11-03 8:52 ` Felipe Balbi
2016-11-03 13:16 ` [PATCH v2 1/2] " Alexey Khoroshilov
2016-11-03 13:16 ` [PATCH v2 2/2] usb: gadget: mv_u3d: mv_u3d_start_queue() refactoring Alexey Khoroshilov
2016-11-03 13:34 ` [PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error Felipe Balbi
2016-11-03 18:09 ` Alexey Khoroshilov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).