[PATCH v2] usb: dwc3: exynos: Fix error handling of clk_prepare_enable

2019-01-21 Thread Alexey Khoroshilov 
If clk_prepare_enable() fails in dwc3_exynos_probe() or in
dwc3_exynos_resume(), exynos->clks[0] is left undisabled
because of usage preincrement in while condition.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
Fixes: 9f2168367a0a ("usb: dwc3: exynos: Rework clock handling and prepare for 
new variants")
---
v2: Add fix for the second instance in dwc3_exynos_resume() per Marek 
Szyprowski note.

 drivers/usb/dwc3/dwc3-exynos.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index cb7fcd7c0ad8..c1e9ea621f41 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -78,7 +78,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
for (i = 0; i < exynos->num_clks; i++) {
ret = clk_prepare_enable(exynos->clks[i]);
if (ret) {
-   while (--i > 0)
+   while (i-- > 0)
clk_disable_unprepare(exynos->clks[i]);
return ret;
}
@@ -223,7 +223,7 @@ static int dwc3_exynos_resume(struct device *dev)
for (i = 0; i < exynos->num_clks; i++) {
ret = clk_prepare_enable(exynos->clks[i]);
if (ret) {
-   while (--i > 0)
+   while (i-- > 0)
clk_disable_unprepare(exynos->clks[i]);
return ret;
}
-- 
2.7.4

[PATCH] usb: dwc3: exynos: Fix error handling of clk_prepare_enable

2019-01-18 Thread Alexey Khoroshilov 
If clk_prepare_enable() fails in dwc3_exynos_probe(),
exynos->clks[0] is left undisabled because of usage
preincrement in while condition.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
Fixes: 9f2168367a0a ("usb: dwc3: exynos: Rework clock handling and prepare for 
new variants")
---
 drivers/usb/dwc3/dwc3-exynos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index cb7fcd7c0ad8..e7f6cff27b9a 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -78,7 +78,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
for (i = 0; i < exynos->num_clks; i++) {
ret = clk_prepare_enable(exynos->clks[i]);
if (ret) {
-   while (--i > 0)
+   while (i-- > 0)
clk_disable_unprepare(exynos->clks[i]);
return ret;
}
-- 
2.7.4

[PATCH] usb: phy: tahvo: fix error handling in tahvo_usb_probe()

2017-10-20 Thread Alexey Khoroshilov 
If devm_extcon_dev_allocate() fails, we should disable clk before return.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
Fixes: 860d2686fda7 ("usb: phy: tahvo: Use 
devm_extcon_dev_[allocate|register]() and replace deprecated API")
---
 drivers/usb/phy/phy-tahvo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-tahvo.c b/drivers/usb/phy/phy-tahvo.c
index 8babd318c0ed..3bc65350f000 100644
--- a/drivers/usb/phy/phy-tahvo.c
+++ b/drivers/usb/phy/phy-tahvo.c
@@ -368,7 +368,7 @@ static int tahvo_usb_probe(struct platform_device *pdev)
tu->extcon = devm_extcon_dev_allocate(&pdev->dev, tahvo_cable);
if (IS_ERR(tu->extcon)) {
dev_err(&pdev->dev, "failed to allocate memory for extcon\n");
-   return -ENOMEM;
+   goto err_disable_clk;
}
 
ret = devm_extcon_dev_register(&pdev->dev, tu->extcon);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: gadget: pch_udc: add checks for dma mapping errors

2017-09-02 Thread Alexey Khoroshilov 
On 24.08.2017 04:02, Jack Pham wrote:
> On Thu, Aug 24, 2017 at 01:47:08AM +0300, Alexey Khoroshilov wrote:
>> There are no checks for dma mapping errors in pch_udc.
>> Tha patch adds the checks and error handling code.
>> Compile tested only.
>>
>> Found by Linux Driver Verification project (linuxtesting.org).
>>
>> Signed-off-by: Alexey Khoroshilov 
>> ---
>>  drivers/usb/gadget/udc/pch_udc.c | 15 +--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/udc/pch_udc.c 
>> b/drivers/usb/gadget/udc/pch_udc.c
>> index 84dcbcd756f0..a305f8392082 100644
>> --- a/drivers/usb/gadget/udc/pch_udc.c
>> +++ b/drivers/usb/gadget/udc/pch_udc.c
>> @@ -1767,7 +1767,7 @@ static struct usb_request 
>> *pch_udc_alloc_request(struct usb_ep *usbep,
>>  req->req.dma = DMA_ADDR_INVALID;
>>  req->dma = DMA_ADDR_INVALID;
>>  INIT_LIST_HEAD(&req->queue);
>> -if (!ep->dev->dma_addr)
>> +if (ep->dev->dma_addr != DMA_ADDR_INVALID)
>>  return &req->req;
>>  /* ep0 in requests are allocated from data pool here */
>>  dma_desc = dma_pool_alloc(ep->dev->data_requests, gfp,
>> @@ -1879,6 +1879,13 @@ static int pch_udc_pcd_queue(struct usb_ep *usbep, 
>> struct usb_request *usbreq,
>>usbreq->length,
>>DMA_FROM_DEVICE);
>>  }
>> +if (dma_mapping_error(&dev->pdev->dev, req->dma)) {
>> +req->dma = DMA_ADDR_INVALID;
>> +retval = -ENOMEM;
>> +if ((unsigned long)(usbreq->buf) & 0x03)
>> +kfree(req->buf);
>> +goto probe_end;
>> +}
>>  req->dma_mapped = 1;
>>  }
>>  if (usbreq->length > 0) {
>> @@ -2961,6 +2968,10 @@ static int init_dma_pools(struct pch_udc_dev *dev)
>>  dev->dma_addr = dma_map_single(&dev->pdev->dev, ep0out_buf,
>> UDC_EP0OUT_BUFF_SIZE * 4,
>> DMA_FROM_DEVICE);
>> +if (dma_mapping_error(&dev->pdev->dev, dev->dma_addr)) {
>> +dev->dma_addr = DMA_ADDR_INVALID;
>> +return -ENOMEM;
>> +}
> 
> Wouldn't this driver be better off using the
> usb_gadget_{map,unmap}_request() functions provided by UDC core.c?
> dma_mapping_error() is provided for free that way.
> 

I think so, but it requires quite significant rework of the driver.
I would not do that without access to hardware.

--
Alexey

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: pch_udc: add checks for dma mapping errors

2017-08-23 Thread Alexey Khoroshilov 
There are no checks for dma mapping errors in pch_udc.
Tha patch adds the checks and error handling code.
Compile tested only.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/udc/pch_udc.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index 84dcbcd756f0..a305f8392082 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -1767,7 +1767,7 @@ static struct usb_request *pch_udc_alloc_request(struct 
usb_ep *usbep,
req->req.dma = DMA_ADDR_INVALID;
req->dma = DMA_ADDR_INVALID;
INIT_LIST_HEAD(&req->queue);
-   if (!ep->dev->dma_addr)
+   if (ep->dev->dma_addr != DMA_ADDR_INVALID)
return &req->req;
/* ep0 in requests are allocated from data pool here */
dma_desc = dma_pool_alloc(ep->dev->data_requests, gfp,
@@ -1879,6 +1879,13 @@ static int pch_udc_pcd_queue(struct usb_ep *usbep, 
struct usb_request *usbreq,
  usbreq->length,
  DMA_FROM_DEVICE);
}
+   if (dma_mapping_error(&dev->pdev->dev, req->dma)) {
+   req->dma = DMA_ADDR_INVALID;
+   retval = -ENOMEM;
+   if ((unsigned long)(usbreq->buf) & 0x03)
+   kfree(req->buf);
+   goto probe_end;
+   }
req->dma_mapped = 1;
}
if (usbreq->length > 0) {
@@ -2961,6 +2968,10 @@ static int init_dma_pools(struct pch_udc_dev *dev)
dev->dma_addr = dma_map_single(&dev->pdev->dev, ep0out_buf,
   UDC_EP0OUT_BUFF_SIZE * 4,
   DMA_FROM_DEVICE);
+   if (dma_mapping_error(&dev->pdev->dev, dev->dma_addr)) {
+   dev->dma_addr = DMA_ADDR_INVALID;
+   return -ENOMEM;
+   }
return 0;
 }
 
@@ -3038,7 +3049,7 @@ static void pch_udc_remove(struct pci_dev *pdev)
dma_pool_destroy(dev->stp_requests);
}
 
-   if (dev->dma_addr)
+   if (dev->dma_addr != DMA_ADDR_INVALID)
dma_unmap_single(&dev->pdev->dev, dev->dma_addr,
 UDC_EP0OUT_BUFF_SIZE * 4, DMA_FROM_DEVICE);
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Inconsistency in usb_add_gadget_udc_release() interface

2017-08-16 Thread Alexey Khoroshilov 
On 16.08.2017 18:24, Alan Stern wrote:
> On Wed, 16 Aug 2017, Alexey Khoroshilov wrote:
> 
>> Hello,
>>
>> usb_add_gadget_udc_release() gets release() argument that allows to
>> release user resources.
>>
>> As far as I can see, the release() is called on error paths 
>> of usb_add_gadget_udc_release() as a result of
>> put_device(&gadget->dev);
>> except for the only path going via err1.
>>
>> As a result a caller of the usb_add_gadget_udc_release() have no chance
>> to know if the release() was invoked or not.
>>
>> It may lead to memory leaks (drivers/usb/gadget/udc/snps_udc_core.c)
>> or to double free (drivers/usb/gadget/udc/fsl_udc_core.c).
>>
>> Is my reading correct? If so, should we always call release() on error paths?
> 
> How about this (untested)?
> 

It looks reasonable. I would only suggest also to make contract
description more explicit, e.g.

/**
 * usb_add_gadget_udc_release - adds a new gadget to the udc class
driver list
 * @parent: the parent device to this udc. Usually the controller driver's
 * device.
 * @gadget: the gadget to be added to the list.
 * @release: a gadget release function.
 *
 * Returns zero on success, negative errno otherwise.
+* Calls the gadget release function in the latter case.
 */

--
Alexey Khoroshilov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Inconsistency in usb_add_gadget_udc_release() interface

2017-08-15 Thread Alexey Khoroshilov 
Hello,

usb_add_gadget_udc_release() gets release() argument that allows to
release user resources.

As far as I can see, the release() is called on error paths 
of usb_add_gadget_udc_release() as a result of
put_device(&gadget->dev);
except for the only path going via err1.

As a result a caller of the usb_add_gadget_udc_release() have no chance
to know if the release() was invoked or not.

It may lead to memory leaks (drivers/usb/gadget/udc/snps_udc_core.c)
or to double free (drivers/usb/gadget/udc/fsl_udc_core.c).

Is my reading correct? If so, should we always call release() on error paths?

--
Alexey Khoroshilov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: mv_u3d: fix error handling in mv_u3d_probe()

2017-03-31 Thread Alexey Khoroshilov 
There are several inconsistencies in the error handling code.
1. If clk_get() fails, it goes to clk_put().
2. If pdata->phy_init() fails, it does not disable u3d->clk.
3. In case of failure after stopping u3d, it does pdata->phy_deinit() 
   and clk_disable(u3d->clk) twice.
4. It ignores failures in clk_enable().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/udc/mv_u3d_core.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/mv_u3d_core.c 
b/drivers/usb/gadget/udc/mv_u3d_core.c
index d365449a295a..772049afe166 100644
--- a/drivers/usb/gadget/udc/mv_u3d_core.c
+++ b/drivers/usb/gadget/udc/mv_u3d_core.c
@@ -1835,13 +1835,18 @@ static int mv_u3d_probe(struct platform_device *dev)
}
 
/* we will access controller register, so enable the u3d controller */
-   clk_enable(u3d->clk);
+   retval = clk_enable(u3d->clk);
+   if (retval) {
+   dev_err(&dev->dev, "clk_enable error %d\n", retval);
+   goto err_u3d_enable;
+   }
 
if (pdata->phy_init) {
retval = pdata->phy_init(u3d->phy_regs);
if (retval) {
dev_err(&dev->dev, "init phy error %d\n", retval);
-   goto err_u3d_enable;
+   clk_disable(u3d->clk);
+   goto err_phy_init;
}
}
 
@@ -1974,15 +1979,13 @@ static int mv_u3d_probe(struct platform_device *dev)
dma_free_coherent(&dev->dev, u3d->ep_context_size,
u3d->ep_context, u3d->ep_context_dma);
 err_alloc_ep_context:
-   if (pdata->phy_deinit)
-   pdata->phy_deinit(u3d->phy_regs);
-   clk_disable(u3d->clk);
+err_phy_init:
 err_u3d_enable:
iounmap(u3d->cap_regs);
 err_map_cap_regs:
 err_get_cap_regs:
-err_get_clk:
clk_put(u3d->clk);
+err_get_clk:
kfree(u3d);
 err_alloc_private:
 err_pdata:
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usbtmc: don't return zero on failure path in usbtmc_probe()

2017-03-17 Thread Alexey Khoroshilov 
usbtmc_probe() returns zero in case of allocation failures.

The patch fixes that. By the way it rearranges error lables just to improve
readability of quite complex dependencies in error handling code.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/class/usbtmc.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index f03692ec5520..38fc3fd3727e 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1469,8 +1469,10 @@ static int usbtmc_probe(struct usb_interface *intf,
if (data->iin_ep_present) {
/* allocate int urb */
data->iin_urb = usb_alloc_urb(0, GFP_KERNEL);
-   if (!data->iin_urb)
-   goto error_register;
+   if (!data->iin_urb) {
+   retcode = -ENOMEM;
+   goto err_alloc_urb;
+   }
 
/* Protect interrupt in endpoint data until iin_urb is freed */
kref_get(&data->kref);
@@ -1478,8 +1480,10 @@ static int usbtmc_probe(struct usb_interface *intf,
/* allocate buffer for interrupt in */
data->iin_buffer = kmalloc(data->iin_wMaxPacketSize,
GFP_KERNEL);
-   if (!data->iin_buffer)
-   goto error_register;
+   if (!data->iin_buffer) {
+   retcode = -ENOMEM;
+   goto err_data;
+   }
 
/* fill interrupt urb */
usb_fill_int_urb(data->iin_urb, data->usb_dev,
@@ -1491,7 +1495,7 @@ static int usbtmc_probe(struct usb_interface *intf,
retcode = usb_submit_urb(data->iin_urb, GFP_KERNEL);
if (retcode) {
dev_err(&intf->dev, "Failed to submit iin_urb\n");
-   goto error_register;
+   goto err_data;
}
}
 
@@ -1502,16 +1506,18 @@ static int usbtmc_probe(struct usb_interface *intf,
dev_err(&intf->dev, "Not able to get a minor"
" (base %u, slice default): %d\n", USBTMC_MINOR_BASE,
retcode);
-   goto error_register;
+   goto err_register;
}
dev_dbg(&intf->dev, "Using minor number %d\n", intf->minor);
 
return 0;
 
-error_register:
-   sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
+err_register:
sysfs_remove_group(&intf->dev.kobj, &data_attr_grp);
+err_data:
usbtmc_free_int(data);
+err_alloc_urb:
+   sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
kref_put(&data->kref, usbtmc_delete);
return retcode;
 }
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error

2016-11-03 Thread Alexey Khoroshilov 
On 03.11.2016 16:34, Felipe Balbi wrote:
> 
> Hi,
> 
> Alexey Khoroshilov  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

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] usb: gadget: mv_u3d: mv_u3d_start_queue() refactoring

2016-11-03 Thread Alexey Khoroshilov 
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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] usb: gadget: mv_u3d: add check for dma mapping error

2016-11-03 Thread Alexey Khoroshilov 
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 
---
 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: mv_u3d: add check for dma mapping error

2016-10-28 Thread Alexey Khoroshilov 
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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: goku_udc: fix memory leak in goku_probe()

2016-08-25 Thread Alexey Khoroshilov 
Memory allocated for goku_udc device is not deallocated at error
paths in goku_probe(), because gadget_release() destructor
is not registered yet.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/udc/goku_udc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/udc/goku_udc.c 
b/drivers/usb/gadget/udc/goku_udc.c
index d2205d9e0c8b..8b290c128d97 100644
--- a/drivers/usb/gadget/udc/goku_udc.c
+++ b/drivers/usb/gadget/udc/goku_udc.c
@@ -1839,6 +1839,8 @@ static int goku_probe(struct pci_dev *pdev, const struct 
pci_device_id *id)
 err:
if (dev)
goku_remove (pdev);
+   /* gadget_release is not registered yet, kfree explicitly */
+   kfree(dev);
return retval;
 }
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] USB: mos7720: fix non-atomic allocation in write path

2016-08-11 Thread Alexey Khoroshilov 
There is an allocation with GFP_KERNEL flag in mos7720_write(),
while it may be called from interrupt context.

Follow-up for commit 191252837626 ("USB: kobil_sct: fix non-atomic allocation 
in write path")

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/serial/mos7720.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index 5608af4a369d..de9992b492b0 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -1252,7 +1252,7 @@ static int mos7720_write(struct tty_struct *tty, struct 
usb_serial_port *port,
 
if (urb->transfer_buffer == NULL) {
urb->transfer_buffer = kmalloc(URB_TRANSFER_BUFFER_SIZE,
-  GFP_KERNEL);
+  GFP_ATOMIC);
if (!urb->transfer_buffer)
goto exit;
}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] USB: mos7840: fix non-atomic allocation in write path

2016-08-11 Thread Alexey Khoroshilov 
There is an allocation with GFP_KERNEL flag in mos7840_write(),
while it may be called from interrupt context.

Follow-up for commit 191252837626 ("USB: kobil_sct: fix non-atomic allocation 
in write path")

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/serial/mos7840.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index ed378fb232e7..1de2c01c078d 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -1341,7 +1341,7 @@ static int mos7840_write(struct tty_struct *tty, struct 
usb_serial_port *port,
 
if (urb->transfer_buffer == NULL) {
urb->transfer_buffer =
-   kmalloc(URB_TRANSFER_BUFFER_SIZE, GFP_KERNEL);
+   kmalloc(URB_TRANSFER_BUFFER_SIZE, GFP_ATOMIC);
if (!urb->transfer_buffer)
goto exit;
}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] USB: whci-hcd: add more checks for dma mapping error

2016-03-26 Thread Alexey Khoroshilov 
Fixing checks for dma mapping error in qset_fill_page_list(),
I have missed two similar problems in qset_add_urb_sg() and
in qset_add_urb_sg_linearize().

v2: check validity of dma_addr with dma_mapping_error()
in qset_free_std() as suggested by Vladimir Zapolskiy.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/host/whci/qset.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/whci/qset.c b/drivers/usb/host/whci/qset.c
index 1a8e960d073b..c0e6812426b3 100644
--- a/drivers/usb/host/whci/qset.c
+++ b/drivers/usb/host/whci/qset.c
@@ -314,7 +314,7 @@ void qset_free_std(struct whc *whc, struct whc_std *std)
kfree(std->bounce_buf);
}
if (std->pl_virt) {
-   if (std->dma_addr)
+   if (!dma_mapping_error(whc->wusbhc.dev, std->dma_addr))
dma_unmap_single(whc->wusbhc.dev, std->dma_addr,
 std->num_pointers * sizeof(struct 
whc_page_list_entry),
 DMA_TO_DEVICE);
@@ -535,9 +535,11 @@ static int qset_add_urb_sg(struct whc *whc, struct 
whc_qset *qset, struct urb *u
list_for_each_entry(std, &qset->stds, list_node) {
if (std->ntds_remaining == -1) {
pl_len = std->num_pointers * sizeof(struct 
whc_page_list_entry);
-   std->ntds_remaining = ntds--;
std->dma_addr = dma_map_single(whc->wusbhc.dev, 
std->pl_virt,
   pl_len, DMA_TO_DEVICE);
+   if (dma_mapping_error(whc->wusbhc.dev, std->dma_addr))
+   return -EFAULT;
+   std->ntds_remaining = ntds--;
}
}
return 0;
@@ -618,6 +620,8 @@ static int qset_add_urb_sg_linearize(struct whc *whc, 
struct whc_qset *qset,
 
std->dma_addr = dma_map_single(&whc->umc->dev, std->bounce_buf, 
std->len,
   is_out ? DMA_TO_DEVICE : 
DMA_FROM_DEVICE);
+   if (dma_mapping_error(&whc->umc->dev, std->dma_addr))
+   return -EFAULT;
 
if (qset_fill_page_list(whc, std, mem_flags) < 0)
return -ENOMEM;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] USB: whci-hcd: add more checks for dma mapping error

2016-03-25 Thread Alexey Khoroshilov 

On 26.03.2016 01:03, Vladimir Zapolskiy wrote:

On 25.03.2016 22:23, Alexey Khoroshilov wrote:

Fixing checks for dma mapping error in qset_fill_page_list(),
I have missed two similar problems in qset_add_urb_sg() and
in qset_add_urb_sg_linearize().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
  drivers/usb/host/whci/qset.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/whci/qset.c b/drivers/usb/host/whci/qset.c
index 1a8e960d073b..a8e9b618e643 100644
--- a/drivers/usb/host/whci/qset.c
+++ b/drivers/usb/host/whci/qset.c
@@ -535,9 +535,11 @@ static int qset_add_urb_sg(struct whc *whc, struct 
whc_qset *qset, struct urb *u
list_for_each_entry(std, &qset->stds, list_node) {
if (std->ntds_remaining == -1) {
pl_len = std->num_pointers * sizeof(struct 
whc_page_list_entry);
-   std->ntds_remaining = ntds--;
std->dma_addr = dma_map_single(whc->wusbhc.dev, 
std->pl_virt,
   pl_len, DMA_TO_DEVICE);
+   if (dma_mapping_error(whc->wusbhc.dev, std->dma_addr))
+   return -EFAULT;


Resources are leaked on error path:
* std->pl_virt  -- most probably, at least it is freed on error path above,
* dma mappings done in a loop before met error,



As far as I can see, it is not the case.
If qset_add_urb_sg() returns error code, the caller (qset_add_urb()) 
invokes qset_free_stds() that performs all resource deallocations.


As for the error path above, I consider it as a typical krealloc() 
pattern, since it does not frees memory allocated at previous iterations 
of the cycle.


Thank you,
Alexey

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: whci-hcd: add more checks for dma mapping error

2016-03-25 Thread Alexey Khoroshilov 
Fixing checks for dma mapping error in qset_fill_page_list(),
I have missed two similar problems in qset_add_urb_sg() and
in qset_add_urb_sg_linearize().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/host/whci/qset.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/whci/qset.c b/drivers/usb/host/whci/qset.c
index 1a8e960d073b..a8e9b618e643 100644
--- a/drivers/usb/host/whci/qset.c
+++ b/drivers/usb/host/whci/qset.c
@@ -535,9 +535,11 @@ static int qset_add_urb_sg(struct whc *whc, struct 
whc_qset *qset, struct urb *u
list_for_each_entry(std, &qset->stds, list_node) {
if (std->ntds_remaining == -1) {
pl_len = std->num_pointers * sizeof(struct 
whc_page_list_entry);
-   std->ntds_remaining = ntds--;
std->dma_addr = dma_map_single(whc->wusbhc.dev, 
std->pl_virt,
   pl_len, DMA_TO_DEVICE);
+   if (dma_mapping_error(whc->wusbhc.dev, std->dma_addr))
+   return -EFAULT;
+   std->ntds_remaining = ntds--;
}
}
return 0;
@@ -618,6 +620,8 @@ static int qset_add_urb_sg_linearize(struct whc *whc, 
struct whc_qset *qset,
 
std->dma_addr = dma_map_single(&whc->umc->dev, std->bounce_buf, 
std->len,
   is_out ? DMA_TO_DEVICE : 
DMA_FROM_DEVICE);
+   if (dma_mapping_error(&whc->umc->dev, std->dma_addr))
+   return -EFAULT;
 
if (qset_fill_page_list(whc, std, mem_flags) < 0)
return -ENOMEM;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: bdc_udc: fix race condition in bdc_udc_exit()

2016-02-26 Thread Alexey Khoroshilov 
bdc_ep_disable() expects to be called with bdc->lock held.
The assumption is met in all the cases except for call from bdc_udc_exit(),
that is called from bdc_remove(). As a result a race can happen or unheld
bdc->lock can be unlocked in bdc_req_complete().

The patch proposes to acquire-release bdc->lock around bdc_ep_disable()
in bdc_udc_exit().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/udc/bdc/bdc_udc.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/gadget/udc/bdc/bdc_udc.c 
b/drivers/usb/gadget/udc/bdc/bdc_udc.c
index 7f77db5d1278..aae7458d8986 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_udc.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_udc.c
@@ -581,8 +581,13 @@ err0:
 
 void bdc_udc_exit(struct bdc *bdc)
 {
+   unsigned long flags;
+
dev_dbg(bdc->dev, "%s()\n", __func__);
+   spin_lock_irqsave(&bdc->lock, flags);
bdc_ep_disable(bdc->bdc_ep_array[1]);
+   spin_unlock_irqrestore(&bdc->lock, flags);
+
usb_del_gadget_udc(&bdc->gadget);
bdc_free_ep(bdc);
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: whci-hcd: add check for dma mapping error

2015-11-20 Thread Alexey Khoroshilov 
qset_fill_page_list() do not check for dma mapping errors.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/host/whci/qset.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/host/whci/qset.c b/drivers/usb/host/whci/qset.c
index dc31c425ce01..9f1c0538b211 100644
--- a/drivers/usb/host/whci/qset.c
+++ b/drivers/usb/host/whci/qset.c
@@ -377,6 +377,10 @@ static int qset_fill_page_list(struct whc *whc, struct 
whc_std *std, gfp_t mem_f
if (std->pl_virt == NULL)
return -ENOMEM;
std->dma_addr = dma_map_single(whc->wusbhc.dev, std->pl_virt, pl_len, 
DMA_TO_DEVICE);
+   if (dma_mapping_error(whc->wusbhc.dev, std->dma_addr)) {
+   kfree(std->pl_virt);
+   return -EFAULT;
+   }
 
for (p = 0; p < std->num_pointers; p++) {
std->pl_virt[p].buf_ptr = cpu_to_le64(dma_addr);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: gadget: pch-udc: fix lock

2015-09-29 Thread Alexey Khoroshilov 
Looks good, but
1. We still have
spin_lock(&dev->lock);
dev->driver->disconnect(&dev->gadget);
spin_unlock(&dev->lock);
in
pch_udc_vbus_session()
pch_udc_pcd_pullup().

And also there is lock around dev->driver->setup() in
pch_udc_svc_control_out()
pch_udc_svc_intf_interrupt()
pch_udc_svc_cfg_interrupt().
Is it ok?

2. Deadlocks mentioned in my original report are still there:
pch_udc_isr()
  spin_lock(&dev->lock);
  pch_udc_dev_isr(dev, dev_intr);
pch_udc_svc_ur_interrupt(dev);
  empty_req_queue(ep);
complete_req(ep, req, -ESHUTDOWN);
  spin_lock(&dev->lock);  <--- deadlock
  if (dev->driver) { spin_lock(&dev->lock); } <--- deadlock


--
Alexey


On 28.09.2015 18:49, Felipe Balbi wrote:
> gadget methods should be called without
> spinlocks held.
> 
> Reported-by: Alexey Khoroshilov 
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/usb/gadget/udc/pch_udc.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/pch_udc.c 
> b/drivers/usb/gadget/udc/pch_udc.c
> index e5f4c5274298..3181fc9c1c49 100644
> --- a/drivers/usb/gadget/udc/pch_udc.c
> +++ b/drivers/usb/gadget/udc/pch_udc.c
> @@ -2747,18 +2747,18 @@ static void pch_udc_dev_isr(struct pch_udc_dev *dev, 
> u32 dev_intr)
>   if (dev_intr & UDC_DEVINT_US) {
>   if (dev->driver
>   && dev->driver->suspend) {
> - spin_lock(&dev->lock);
> - dev->driver->suspend(&dev->gadget);
>   spin_unlock(&dev->lock);
> + dev->driver->suspend(&dev->gadget);
> + spin_lock(&dev->lock);
>   }
>  
>   vbus = pch_vbus_gpio_get_value(dev);
>   if ((dev->vbus_session == 0)
>   && (vbus != 1)) {
>   if (dev->driver && dev->driver->disconnect) {
> - spin_lock(&dev->lock);
> - dev->driver->disconnect(&dev->gadget);
>   spin_unlock(&dev->lock);
> + dev->driver->disconnect(&dev->gadget);
> + spin_lock(&dev->lock);
>   }
>   pch_udc_reconnect(dev);
>   } else if ((dev->vbus_session == 0)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Deadlock in pch_udc_svc_ur_interrupt()

2015-09-28 Thread Alexey Khoroshilov 
Dear colleagus,

It seems code handling USB_RESET interrupt contains unavoidable deadlock.

pch_udc_isr() locks dev->lock, then calls to pch_udc_dev_isr(dev, dev_intr)
that seems to have a couple of locks dev->lock itself:

pch_udc_isr()
  spin_lock(&dev->lock);
  pch_udc_dev_isr(dev, dev_intr);
pch_udc_svc_ur_interrupt(dev);
  empty_req_queue(ep);
complete_req(ep, req, -ESHUTDOWN);
  spin_lock(&dev->lock);  <--- deadlock
  if (dev->driver) { spin_lock(&dev->lock); } <--- deadlock

Found by Linux Driver Verification project (linuxtesting.org).

--
Alexey Khoroshilov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: amd5536udc: fix error handling in udc_pci_probe()

2015-09-05 Thread Alexey Khoroshilov 
If a failure happens early in udc_pci_probe(), error handling code
just kfree(dev) and returns. The patch adds proper resource
deallocations in udc_pci_probe() itself,
since udc_pci_remove() is not suitabe to be called so early
in initialization process.

By the way, iounmap(dev->regs) is replaced by iounmap(dev->virt_addr)
in udc_pci_remove() for clarity.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/udc/amd5536udc.c | 43 +
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index fdacddb18c00..175ca93fe5e2 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -3138,8 +3138,8 @@ static void udc_pci_remove(struct pci_dev *pdev)
writel(AMD_BIT(UDC_DEVCFG_SOFTRESET), &dev->regs->cfg);
if (dev->irq_registered)
free_irq(pdev->irq, dev);
-   if (dev->regs)
-   iounmap(dev->regs);
+   if (dev->virt_addr)
+   iounmap(dev->virt_addr);
if (dev->mem_region)
release_mem_region(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
@@ -3226,17 +3226,13 @@ static int udc_pci_probe(
 
/* init */
dev = kzalloc(sizeof(struct udc), GFP_KERNEL);
-   if (!dev) {
-   retval = -ENOMEM;
-   goto finished;
-   }
+   if (!dev)
+   return -ENOMEM;
 
/* pci setup */
if (pci_enable_device(pdev) < 0) {
-   kfree(dev);
-   dev = NULL;
retval = -ENODEV;
-   goto finished;
+   goto err_pcidev;
}
dev->active = 1;
 
@@ -3246,28 +3242,22 @@ static int udc_pci_probe(
 
if (!request_mem_region(resource, len, name)) {
dev_dbg(&pdev->dev, "pci device used already\n");
-   kfree(dev);
-   dev = NULL;
retval = -EBUSY;
-   goto finished;
+   goto err_memreg;
}
dev->mem_region = 1;
 
dev->virt_addr = ioremap_nocache(resource, len);
if (dev->virt_addr == NULL) {
dev_dbg(&pdev->dev, "start address cannot be mapped\n");
-   kfree(dev);
-   dev = NULL;
retval = -EFAULT;
-   goto finished;
+   goto err_ioremap;
}
 
if (!pdev->irq) {
dev_err(&pdev->dev, "irq not set\n");
-   kfree(dev);
-   dev = NULL;
retval = -ENODEV;
-   goto finished;
+   goto err_irq;
}
 
spin_lock_init(&dev->lock);
@@ -3283,10 +3273,8 @@ static int udc_pci_probe(
 
if (request_irq(pdev->irq, udc_irq, IRQF_SHARED, name, dev) != 0) {
dev_dbg(&pdev->dev, "request_irq(%d) fail\n", pdev->irq);
-   kfree(dev);
-   dev = NULL;
retval = -EBUSY;
-   goto finished;
+   goto err_irq;
}
dev->irq_registered = 1;
 
@@ -3314,8 +3302,17 @@ static int udc_pci_probe(
return 0;
 
 finished:
-   if (dev)
-   udc_pci_remove(pdev);
+   udc_pci_remove(pdev);
+   return retval;
+
+err_irq:
+   iounmap(dev->virt_addr);
+err_ioremap:
+   release_mem_region(resource, len);
+err_memreg:
+   pci_disable_device(pdev);
+err_pcidev:
+   kfree(dev);
return retval;
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: mv_udc_core: fix phy_regs I/O memory leak

2015-07-19 Thread Alexey Khoroshilov 
There was an omission in transition to devm_xxx resource handling.
iounmap(udc->phy_regs) were removed, but ioremap() was left
without devm_.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
Fixes: 3517c31a8ece6 ("usb: gadget: mv_udc: use devm_xxx for probe")
---
 drivers/usb/gadget/udc/mv_udc_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/mv_udc_core.c 
b/drivers/usb/gadget/udc/mv_udc_core.c
index d32160d6463f..5da37c957b53 100644
--- a/drivers/usb/gadget/udc/mv_udc_core.c
+++ b/drivers/usb/gadget/udc/mv_udc_core.c
@@ -2167,7 +2167,7 @@ static int mv_udc_probe(struct platform_device *pdev)
return -ENODEV;
}
 
-   udc->phy_regs = ioremap(r->start, resource_size(r));
+   udc->phy_regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
if (udc->phy_regs == NULL) {
dev_err(&pdev->dev, "failed to map phy I/O memory\n");
return -EBUSY;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: host: oxu210hp-hcd: Fix deadlock in oxu_qh_alloc()

2015-01-23 Thread Alexey Khoroshilov 
oxu_qh_alloc() acquires oxu->mem_lock spinlock, finds free qh slot
and calls ehci_qtd_alloc() to initialize qh->dummy, but
ehci_qtd_alloc() acquires oxu->mem_lock as well.
That means an unavoidable deadlock in oxu_qh_alloc().

The patch fixes the issue by introduction of unlocked version
of ehci_qtd_alloc().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/host/oxu210hp-hcd.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index 036924e640f5..15d4cf2c35cd 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -573,13 +573,11 @@ static inline void oxu_qtd_free(struct oxu_hcd *oxu, 
struct ehci_qtd *qtd)
spin_unlock(&oxu->mem_lock);
 }
 
-static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
+static struct ehci_qtd *ehci_qtd_alloc_unlocked(struct oxu_hcd *oxu)
 {
int i;
struct ehci_qtd *qtd = NULL;
 
-   spin_lock(&oxu->mem_lock);
-
for (i = 0; i < QTD_NUM; i++)
if (!oxu->qtd_used[i])
break;
@@ -598,6 +596,17 @@ static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
oxu->qtd_used[i] = 1;
}
 
+   return qtd;
+}
+
+static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
+{
+   struct ehci_qtd *qtd;
+
+   spin_lock(&oxu->mem_lock);
+
+   qtd = ehci_qtd_alloc_unlocked(oxu);
+
spin_unlock(&oxu->mem_lock);
 
return qtd;
@@ -651,7 +660,7 @@ static struct ehci_qh *oxu_qh_alloc(struct oxu_hcd *oxu)
INIT_LIST_HEAD(&qh->qtd_list);
 
/* dummy td enables safe urb queuing */
-   qh->dummy = ehci_qtd_alloc(oxu);
+   qh->dummy = ehci_qtd_alloc_unlocked(oxu);
if (qh->dummy == NULL) {
oxu_dbg(oxu, "no dummy td\n");
oxu->qh_used[i] = 0;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb/kaweth: use GFP_ATOMIC under spin_lock in usb_start_wait_urb()

2015-01-09 Thread Alexey Khoroshilov 
Commit e4c7f259c5be ("USB: kaweth.c: use GFP_ATOMIC under spin_lock")
makes sure that kaweth_internal_control_msg() allocates memory with GFP_ATOMIC,
but kaweth_internal_control_msg() also calls usb_start_wait_urb()
that still allocates memory with GFP_NOIO.

The patch fixes usb_start_wait_urb() as well.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/net/usb/kaweth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c
index dcb6d33141e0..1e9cdca37014 100644
--- a/drivers/net/usb/kaweth.c
+++ b/drivers/net/usb/kaweth.c
@@ -1276,7 +1276,7 @@ static int usb_start_wait_urb(struct urb *urb, int 
timeout, int* actual_length)
 awd.done = 0;
 
 urb->context = &awd;
-status = usb_submit_urb(urb, GFP_NOIO);
+status = usb_submit_urb(urb, GFP_ATOMIC);
 if (status) {
 // something went wrong
 usb_free_urb(urb);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usbip: fix error handling in stub_probe()

2014-11-28 Thread Alexey Khoroshilov 
If usb_hub_claim_port() fails, no resources are deallocated and
if stub_add_files() fails, port is not released.

The patch fixes these issues and rearranges error handling code.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/usbip/stub_dev.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c
index fac20e0434c0..a3ec49bdc1e6 100644
--- a/drivers/usb/usbip/stub_dev.c
+++ b/drivers/usb/usbip/stub_dev.c
@@ -311,7 +311,6 @@ static int stub_probe(struct usb_device *udev)
 {
struct stub_device *sdev = NULL;
const char *udev_busid = dev_name(&udev->dev);
-   int err = 0;
struct bus_id_priv *busid_priv;
int rc;
 
@@ -372,23 +371,28 @@ static int stub_probe(struct usb_device *udev)
(struct usb_dev_state *) udev);
if (rc) {
dev_dbg(&udev->dev, "unable to claim port\n");
-   return rc;
+   goto err_port;
}
 
-   err = stub_add_files(&udev->dev);
-   if (err) {
+   rc = stub_add_files(&udev->dev);
+   if (rc) {
dev_err(&udev->dev, "stub_add_files for %s\n", udev_busid);
-   dev_set_drvdata(&udev->dev, NULL);
-   usb_put_dev(udev);
-   kthread_stop_put(sdev->ud.eh);
-
-   busid_priv->sdev = NULL;
-   stub_device_free(sdev);
-   return err;
+   goto err_files;
}
busid_priv->status = STUB_BUSID_ALLOC;
 
return 0;
+err_files:
+   usb_hub_release_port(udev->parent, udev->portnum,
+(struct usb_dev_state *) udev);
+err_port:
+   dev_set_drvdata(&udev->dev, NULL);
+   usb_put_dev(udev);
+   kthread_stop_put(sdev->ud.eh);
+
+   busid_priv->sdev = NULL;
+   stub_device_free(sdev);
+   return rc;
 }
 
 static void shutdown_busid(struct bus_id_priv *busid_priv)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: dbgp gadget: fix use after free in dbgp_unbind()

2014-08-10 Thread Alexey Khoroshilov 
After dbgp_bind()-dbgp_unbind() cycle happens, static variable dbgp
contains pointers to already deallocated memory (dbgp.serial and dbgp.req).
If the next dbgp_bind() fails, for example in usb_ep_alloc_request(),
dbgp_bind() calls dbgp_unbind() on failure path,
and dbgp_unbind() frees dbgp.serial that still stores a pointer
to already deallocated memory.

The patch sets pointers to NULL in dbgp_unbind().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/legacy/dbgp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c
index 986fc511a2ed..225e385a6160 100644
--- a/drivers/usb/gadget/legacy/dbgp.c
+++ b/drivers/usb/gadget/legacy/dbgp.c
@@ -222,10 +222,12 @@ static void dbgp_unbind(struct usb_gadget *gadget)
 {
 #ifdef CONFIG_USB_G_DBGP_SERIAL
kfree(dbgp.serial);
+   dbgp.serial = NULL;
 #endif
if (dbgp.req) {
kfree(dbgp.req->buf);
usb_ep_free_request(gadget->ep0, dbgp.req);
+   dbgp.req = NULL;
}
 
gadget->ep0->driver_data = NULL;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: host: max3421-hcd: unconditionally use GFP_ATOMIC in max3421_urb_enqueue()

2014-06-19 Thread Alexey Khoroshilov 
As far as kzalloc() is called with spinlock held,
we have to pass GFP_ATOMIC regardless of mem_flags argument.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/host/max3421-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 858efcfda50b..ed22424dbec7 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1551,7 +1551,7 @@ max3421_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, 
gfp_t mem_flags)
max3421_ep = urb->ep->hcpriv;
if (!max3421_ep) {
/* gets freed in max3421_endpoint_disable: */
-   max3421_ep = kzalloc(sizeof(struct max3421_ep), mem_flags);
+   max3421_ep = kzalloc(sizeof(struct max3421_ep), GFP_ATOMIC);
if (!max3421_ep) {
retval = -ENOMEM;
goto out;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: gr_udc: unconditionally use GFP_ATOMIC in gr_queue_ext()

2014-05-07 Thread Alexey Khoroshilov 
As far as gr_queue() is called with spinlock held,
we have to pass GFP_ATOMIC regardless of gfp argument.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/gr_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index f984ee75324d..19a1b52c4210 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -1679,7 +1679,7 @@ static int gr_queue_ext(struct usb_ep *_ep, struct 
usb_request *_req,
if (ep->is_in)
gr_dbgprint_request("EXTERN", ep, req);
 
-   ret = gr_queue(ep, req, gfp_flags);
+   ret = gr_queue(ep, req, GFP_ATOMIC);
 
spin_unlock(&ep->dev->lock);
 
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: cdc-acm: fix double usb_autopm_put_interface() in acm_port_activate()

2014-04-11 Thread Alexey Khoroshilov 
If acm_submit_read_urbs() fails in acm_port_activate(), error handling
code calls usb_autopm_put_interface() while it is already called
before acm_submit_read_urbs(). The patch reorganizes error handling code
to avoid double decrement of USB interface's PM-usage counter.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/class/cdc-acm.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 900f7ff805ee..d5d2c922186a 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -518,13 +518,16 @@ static int acm_port_activate(struct tty_port *port, 
struct tty_struct *tty)
if (usb_submit_urb(acm->ctrlurb, GFP_KERNEL)) {
dev_err(&acm->control->dev,
"%s - usb_submit_urb(ctrl irq) failed\n", __func__);
+   usb_autopm_put_interface(acm->control);
goto error_submit_urb;
}
 
acm->ctrlout = ACM_CTRL_DTR | ACM_CTRL_RTS;
if (acm_set_control(acm, acm->ctrlout) < 0 &&
-   (acm->ctrl_caps & USB_CDC_CAP_LINE))
+   (acm->ctrl_caps & USB_CDC_CAP_LINE)) {
+   usb_autopm_put_interface(acm->control);
goto error_set_control;
+   }
 
usb_autopm_put_interface(acm->control);
 
@@ -549,7 +552,6 @@ error_submit_read_urbs:
 error_set_control:
usb_kill_urb(acm->ctrlurb);
 error_submit_urb:
-   usb_autopm_put_interface(acm->control);
 error_get_interface:
 disconnected:
mutex_unlock(&acm->mutex);
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: wusbcore: fix usb_dev leaks

2013-10-18 Thread Alexey Khoroshilov 
cbaf_probe() does cbaf->usb_dev = usb_get_dev(interface_to_usbdev(iface)),
but there is no usb_put_dev() anywhere in cbaf.

The patch adds usb_put_dev() to cbaf_disconnect() and to an error path in 
cbaf_probe().
Also it adds missed usb_put_intf(iface) to the error path.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/wusbcore/cbaf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/wusbcore/cbaf.c b/drivers/usb/wusbcore/cbaf.c
index 7f78f30..4035004 100644
--- a/drivers/usb/wusbcore/cbaf.c
+++ b/drivers/usb/wusbcore/cbaf.c
@@ -623,6 +623,8 @@ static int cbaf_probe(struct usb_interface *iface,
 
 error_create_group:
 error_check:
+   usb_put_intf(iface);
+   usb_put_dev(cbaf->usb_dev);
kfree(cbaf->buffer);
 error_kmalloc_buffer:
kfree(cbaf);
@@ -637,6 +639,7 @@ static void cbaf_disconnect(struct usb_interface *iface)
sysfs_remove_group(&dev->kobj, &cbaf_dev_attr_group);
usb_set_intfdata(iface, NULL);
usb_put_intf(iface);
+   usb_put_dev(cbaf->usb_dev);
kfree(cbaf->buffer);
/* paranoia: clean up crypto keys */
kzfree(cbaf);
-- 
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: amd5536udc: unconditionally use GFP_ATOMIC in udc_queue()

2013-08-01 Thread Alexey Khoroshilov 
As far as prep_dma() is called with spinlock held,
we have to pass GFP_ATOMIC regardless of gfp argument.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/amd5536udc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
index f52dcfe..a9a4346 100644
--- a/drivers/usb/gadget/amd5536udc.c
+++ b/drivers/usb/gadget/amd5536udc.c
@@ -1122,7 +1122,7 @@ udc_queue(struct usb_ep *usbep, struct usb_request 
*usbreq, gfp_t gfp)
goto finished;
}
if (ep->dma) {
-   retval = prep_dma(ep, req, gfp);
+   retval = prep_dma(ep, req, GFP_ATOMIC);
if (retval != 0)
goto finished;
/* write desc pointer to enable DMA */
@@ -1190,7 +1190,7 @@ udc_queue(struct usb_ep *usbep, struct usb_request 
*usbreq, gfp_t gfp)
 * for PPB modes, because of chain creation reasons
 */
if (ep->in) {
-   retval = prep_dma(ep, req, gfp);
+   retval = prep_dma(ep, req, GFP_ATOMIC);
if (retval != 0)
goto finished;
}
-- 
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: gadget: mv_u3d_core: fix violation of locking discipline in mv_u3d_ep_disable()

2013-07-29 Thread Alexey Khoroshilov 
On 07/29/2013 04:52 PM, Felipe Balbi wrote:
> Hi,
>
> On Fri, Jul 26, 2013 at 07:26:05PM +0400, Alexey Khoroshilov wrote:
>> On 07/25/2013 09:30 PM, Felipe Balbi wrote:
>>> On Wed, Jul 24, 2013 at 12:20:17AM +0400, Alexey Khoroshilov wrote:
>>>> mv_u3d_nuke() expects to be calles with ep->u3d->lock held,
>>>> because mv_u3d_done() does. But mv_u3d_ep_disable() calls it
>>>> without lock that can lead to unpleasant consequences.
>>>>
>>>> Found by Linux Driver Verification project (linuxtesting.org).
>>>>
>>>> Signed-off-by: Alexey Khoroshilov 
>>> which commit introduced the bug ? Which kernels are affected by this bug ?
>> The bug is present from the very beginning: commit 3d4eb9d of 15 June 2012.
>> So it is in the mainline since v3.5.
> Alright, do you want to have the same fix in stable kernels ? Is it
> necessary at all or do we consider it 'never worked before' and send it
> in the next merge window ?

It is a tricky point. From one point of view it 'never worked before',
but as far as the bug leads to a data race with unpredictable
consequences I would prefer to have it fixed within 3.11 timeframe.

--
Alexey

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: gadget: mv_u3d_core: fix violation of locking discipline in mv_u3d_ep_disable()

2013-07-26 Thread Alexey Khoroshilov 
Hi Felipe,

On 07/25/2013 09:30 PM, Felipe Balbi wrote:
> On Wed, Jul 24, 2013 at 12:20:17AM +0400, Alexey Khoroshilov wrote:
>> mv_u3d_nuke() expects to be calles with ep->u3d->lock held,
>> because mv_u3d_done() does. But mv_u3d_ep_disable() calls it
>> without lock that can lead to unpleasant consequences.
>>
>> Found by Linux Driver Verification project (linuxtesting.org).
>>
>> Signed-off-by: Alexey Khoroshilov 
> which commit introduced the bug ? Which kernels are affected by this bug ?
The bug is present from the very beginning: commit 3d4eb9d of 15 June 2012.
So it is in the mainline since v3.5.

--
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: mv_u3d_core: fix violation of locking discipline in mv_u3d_ep_disable()

2013-07-23 Thread Alexey Khoroshilov 
mv_u3d_nuke() expects to be calles with ep->u3d->lock held,
because mv_u3d_done() does. But mv_u3d_ep_disable() calls it
without lock that can lead to unpleasant consequences.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/mv_u3d_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/mv_u3d_core.c b/drivers/usb/gadget/mv_u3d_core.c
index 07fdb3e..650847d 100644
--- a/drivers/usb/gadget/mv_u3d_core.c
+++ b/drivers/usb/gadget/mv_u3d_core.c
@@ -645,6 +645,7 @@ static int  mv_u3d_ep_disable(struct usb_ep *_ep)
struct mv_u3d_ep *ep;
struct mv_u3d_ep_context *ep_context;
u32 epxcr, direction;
+   unsigned long flags;
 
if (!_ep)
return -EINVAL;
@@ -661,7 +662,9 @@ static int  mv_u3d_ep_disable(struct usb_ep *_ep)
direction = mv_u3d_ep_dir(ep);
 
/* nuke all pending requests (does flush) */
+   spin_lock_irqsave(&u3d->lock, flags);
mv_u3d_nuke(ep, -ESHUTDOWN);
+   spin_unlock_irqrestore(&u3d->lock, flags);
 
/* Disable the endpoint for Rx or Tx and reset the endpoint type */
if (direction == MV_U3D_EP_DIR_OUT) {
-- 
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: fix PTR_ERR translation in init_usb_class()

2013-06-05 Thread Alexey Khoroshilov 
There is a misprint in init_usb_class():
IS_ERR is used to get error code instead of PTR_ERR.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/core/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/file.c b/drivers/usb/core/file.c
index e5387a4..6a4c407 100644
--- a/drivers/usb/core/file.c
+++ b/drivers/usb/core/file.c
@@ -94,7 +94,7 @@ static int init_usb_class(void)
kref_init(&usb_class->kref);
usb_class->class = class_create(THIS_MODULE, "usbmisc");
if (IS_ERR(usb_class->class)) {
-   result = IS_ERR(usb_class->class);
+   result = PTR_ERR(usb_class->class);
printk(KERN_ERR "class_create failed for usb devices\n");
kfree(usb_class);
usb_class = NULL;
-- 
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: r8a66597-udc: do not unlock unheld spinlock in r8a66597_sudmac_irq()

2013-05-29 Thread Alexey Khoroshilov 
r8a66597_irq() processes sudmac part (r8a66597_sudmac_irq()) before locking 
r8a66597->lock.
But transfer_complete(), that is called inside 
(r8a66597_sudmac_irq()->sudmac_finish()->transfer_complete()),
expects r8a66597->lock is locked. As a result unheld spinlock can be unlocked.

The patch just moves locking before calling r8a66597_sudmac_irq().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/gadget/r8a66597-udc.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/r8a66597-udc.c 
b/drivers/usb/gadget/r8a66597-udc.c
index 7ff7d9c..2dd213c 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1469,11 +1469,11 @@ static irqreturn_t r8a66597_irq(int irq, void 
*_r8a66597)
u16 savepipe;
u16 mask0;
 
+   spin_lock(&r8a66597->lock);
+
if (r8a66597_is_sudmac(r8a66597))
r8a66597_sudmac_irq(r8a66597);
 
-   spin_lock(&r8a66597->lock);
-
intsts0 = r8a66597_read(r8a66597, INTSTS0);
intenb0 = r8a66597_read(r8a66597, INTENB0);
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb/core/devio.c: Don't use GFP_KERNEL while we cannot reset a storage device

2013-03-08 Thread Alexey Khoroshilov 
As it was described by Oliver Neukum in commit acbe2fe
"USB: Don't use GFP_KERNEL while we cannot reset a storage device":

  Memory allocations with GFP_KERNEL can cause IO to a storage device
  which can fail resulting in a need to reset the device. Therefore
  GFP_KERNEL cannot be safely used between usb_lock_device()
  and usb_unlock_device(). Replace by GFP_NOIO.

The patch fixes the same issue in usb/core/devio.c.
All the allocations fixed are under usb_lock_device() from usbdev_do_ioctl().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/core/devio.c |   22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 8823e98..4be27e3 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -275,10 +275,10 @@ static struct async *alloc_async(unsigned int 
numisoframes)
 {
struct async *as;
 
-   as = kzalloc(sizeof(struct async), GFP_KERNEL);
+   as = kzalloc(sizeof(struct async), GFP_NOIO);
if (!as)
return NULL;
-   as->urb = usb_alloc_urb(numisoframes, GFP_KERNEL);
+   as->urb = usb_alloc_urb(numisoframes, GFP_NOIO);
if (!as->urb) {
kfree(as);
return NULL;
@@ -887,7 +887,7 @@ static int proc_control(struct dev_state *ps, void __user 
*arg)
sizeof(struct usb_ctrlrequest));
if (ret)
return ret;
-   tbuf = (unsigned char *)__get_free_page(GFP_KERNEL);
+   tbuf = (unsigned char *)__get_free_page(GFP_NOIO);
if (!tbuf) {
ret = -ENOMEM;
goto done;
@@ -983,7 +983,7 @@ static int proc_bulk(struct dev_state *ps, void __user *arg)
ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
if (ret)
return ret;
-   if (!(tbuf = kmalloc(len1, GFP_KERNEL))) {
+   if (!(tbuf = kmalloc(len1, GFP_NOIO))) {
ret = -ENOMEM;
goto done;
}
@@ -1211,7 +1211,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct 
usbdevfs_urb *uurb,
/* min 8 byte setup packet */
if (uurb->buffer_length < 8)
return -EINVAL;
-   dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL);
+   dr = kmalloc(sizeof(struct usb_ctrlrequest), GFP_NOIO);
if (!dr)
return -ENOMEM;
if (copy_from_user(dr, uurb->buffer, 8)) {
@@ -1278,7 +1278,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct 
usbdevfs_urb *uurb,
return -EINVAL;
isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
   uurb->number_of_packets;
-   if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL)))
+   if (!(isopkt = kmalloc(isofrmlen, GFP_NOIO)))
return -ENOMEM;
if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
ret = -EFAULT;
@@ -1326,7 +1326,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct 
usbdevfs_urb *uurb,
 
if (num_sgs) {
as->urb->sg = kmalloc(num_sgs * sizeof(struct scatterlist),
- GFP_KERNEL);
+ GFP_NOIO);
if (!as->urb->sg) {
ret = -ENOMEM;
goto error;
@@ -1337,7 +1337,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct 
usbdevfs_urb *uurb,
totlen = uurb->buffer_length;
for (i = 0; i < as->urb->num_sgs; i++) {
u = (totlen > USB_SG_SIZE) ? USB_SG_SIZE : totlen;
-   buf = kmalloc(u, GFP_KERNEL);
+   buf = kmalloc(u, GFP_NOIO);
if (!buf) {
ret = -ENOMEM;
goto error;
@@ -1355,7 +1355,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct 
usbdevfs_urb *uurb,
}
} else if (uurb->buffer_length > 0) {
as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
-   GFP_KERNEL);
+   GFP_NOIO);
if (!as->urb->transfer_buffer) {
ret = -ENOMEM;
goto error;
@@ -1467,7 +1467,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct 
usbdevfs_urb *uurb,
ret = usb_submit_urb(as->urb, GFP_ATOMIC);
spin_unlock_irq(&ps->lock);
} else {
-   ret = usb_submit_urb(as->urb, GFP_KERNEL);
+   ret = usb_submit_urb(as->urb, GFP_NOIO);
}
 
if (

[PATCH] uwb: fix uwb_dev_unlock() missed at an error path in uwb_rc_cmd_async()

2012-11-26 Thread Alexey Khoroshilov 
There is the only path in uwb_rc_cmd_async() where rc->uwb_dev is left unlocked.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/uwb/reset.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/uwb/reset.c b/drivers/uwb/reset.c
index 7032285..8b47c9c 100644
--- a/drivers/uwb/reset.c
+++ b/drivers/uwb/reset.c
@@ -97,6 +97,7 @@ int uwb_rc_cmd_async(struct uwb_rc *rc, const char *cmd_name,
neh = uwb_rc_neh_add(rc, cmd, expected_type, expected_event, cb, arg);
if (IS_ERR(neh)) {
result = PTR_ERR(neh);
+   uwb_dev_unlock(&rc->uwb_dev);
goto out;
}
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: omninet: fix potential tty NULL dereference

2012-09-13 Thread Alexey Khoroshilov 
Add check for return value of tty_port_tty_get,
since it can return NULL after port hangup that may happen anytime.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/serial/omninet.c |8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/omninet.c b/drivers/usb/serial/omninet.c
index 6f3d705..493c892 100644
--- a/drivers/usb/serial/omninet.c
+++ b/drivers/usb/serial/omninet.c
@@ -185,10 +185,12 @@ static void omninet_read_bulk_callback(struct urb *urb)
 
if (urb->actual_length && header->oh_len) {
struct tty_struct *tty = tty_port_tty_get(&port->port);
-   tty_insert_flip_string(tty, data + OMNINET_DATAOFFSET,
+   if (tty) {
+   tty_insert_flip_string(tty, data + OMNINET_DATAOFFSET,
header->oh_len);
-   tty_flip_buffer_push(tty);
-   tty_kref_put(tty);
+   tty_flip_buffer_push(tty);
+   tty_kref_put(tty);
+   }
}
 
/* Continue trying to always read  */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] wusb: Fix potential memory leak in wusb_dev_sec_add()

2012-08-08 Thread Alexey Khoroshilov 
Do not leak memory by updating pointer with potentially NULL realloc return 
value.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/wusbcore/security.c |7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
index fa810a8..dd88441 100644
--- a/drivers/usb/wusbcore/security.c
+++ b/drivers/usb/wusbcore/security.c
@@ -202,7 +202,7 @@ int wusb_dev_sec_add(struct wusbhc *wusbhc,
 {
int result, bytes, secd_size;
struct device *dev = &usb_dev->dev;
-   struct usb_security_descriptor *secd;
+   struct usb_security_descriptor *secd, *new_secd;
const struct usb_encryption_descriptor *etd, *ccm1_etd = NULL;
const void *itr, *top;
char buf[64];
@@ -221,11 +221,12 @@ int wusb_dev_sec_add(struct wusbhc *wusbhc,
goto out;
}
secd_size = le16_to_cpu(secd->wTotalLength);
-   secd = krealloc(secd, secd_size, GFP_KERNEL);
-   if (secd == NULL) {
+   new_secd = krealloc(secd, secd_size, GFP_KERNEL);
+   if (new_secd == NULL) {
dev_err(dev, "Can't allocate space for security descriptors\n");
goto out;
}
+   secd = new_secd;
result = usb_get_descriptor(usb_dev, USB_DT_SECURITY,
0, secd, secd_size);
if (result < secd_size) {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: whci-hcd: Fix potential memory leak in qset_add_urb_sg()

2012-08-08 Thread Alexey Khoroshilov 
Do not leak memory by updating pointer with potentially
NULL realloc return value.

By the way remove unused local variable:
struct whc_page_list_entry *entry;
More precisely, it was used to increment uninitialized value within one of 
cycles.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/usb/host/whci/qset.c |   11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/whci/qset.c b/drivers/usb/host/whci/qset.c
index 76083ae..dc31c42 100644
--- a/drivers/usb/host/whci/qset.c
+++ b/drivers/usb/host/whci/qset.c
@@ -436,7 +436,7 @@ static int qset_add_urb_sg(struct whc *whc, struct whc_qset 
*qset, struct urb *u
int i;
int ntds = 0;
struct whc_std *std = NULL;
-   struct whc_page_list_entry *entry;
+   struct whc_page_list_entry *new_pl_virt;
dma_addr_t prev_end = 0;
size_t pl_len;
int p = 0;
@@ -508,12 +508,15 @@ static int qset_add_urb_sg(struct whc *whc, struct 
whc_qset *qset, struct urb *u
 
pl_len = std->num_pointers * sizeof(struct 
whc_page_list_entry);
 
-   std->pl_virt = krealloc(std->pl_virt, pl_len, 
mem_flags);
-   if (std->pl_virt == NULL) {
+   new_pl_virt = krealloc(std->pl_virt, pl_len, mem_flags);
+   if (new_pl_virt == NULL) {
+   kfree(std->pl_virt);
+   std->pl_virt = NULL;
return -ENOMEM;
}
+   std->pl_virt = new_pl_virt;
 
-   for (;p < std->num_pointers; p++, entry++) {
+   for (;p < std->num_pointers; p++) {
std->pl_virt[p].buf_ptr = cpu_to_le64(dma_addr);
dma_addr = (dma_addr + WHCI_PAGE_SIZE) & 
~(WHCI_PAGE_SIZE-1);
}
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html