Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1579860 > unrolled thread
| Started by | "Gustavo A. R. Silva" <garsilva@embeddedor.com> |
|---|---|
| First post | 2017-02-13 16:40 +0100 |
| Last post | 2017-02-13 17:10 +0100 |
| Articles | 8 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH 0/2] avoid use of freed pointer "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-02-13 16:40 +0100
[PATCH 2/2] usb: gadget: udc: remove unnecessary variable and update function prototype "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-02-13 17:10 +0100
Re: [PATCH 2/2] usb: gadget: udc: remove unnecessary variable and update function prototype Michal Nazarewicz <mina86@mina86.com> - 2017-02-13 17:10 +0100
Re: [PATCH 1/2] usb: gadget: udc: avoid use of freed pointer Michal Nazarewicz <mina86@mina86.com> - 2017-02-13 17:10 +0100
Re: [PATCH 1/2] usb: gadget: udc: avoid use of freed pointer "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-02-14 23:00 +0100
[PATCH v2 1/2] usb: gadget: udc: avoid use of freed pointer "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-02-14 23:00 +0100
[PATCH v2 2/2] usb: gadget: udc: remove unnecessary variable and update function prototype "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-02-14 23:10 +0100
[PATCH 1/2] usb: gadget: udc: avoid use of freed pointer "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-02-13 17:10 +0100
| From | "Gustavo A. R. Silva" <garsilva@embeddedor.com> |
|---|---|
| Date | 2017-02-13 16:40 +0100 |
| Subject | [PATCH 0/2] avoid use of freed pointer |
| Message-ID | <taqTn-5xK-19@gated-at.bofh.it> |
Hello,
This patch series addesses Coverity ID 1091172, which reports the
use of a freed pointer. udc_free_dma_chain() function was rewritten
in order to fix this issue.
Unnecessary 'ret_val' variable was removed and the function prototype
was modified.
Thanks
Gustavo A. R. Silva (2):
usb: gadget: udc: avoid use of freed pointer
usb: gadget: udc: remove unnecessary variable and update function
prototype
drivers/usb/gadget/udc/amd5536udc.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
--
2.5.0
[toc] | [next] | [standalone]
| From | "Gustavo A. R. Silva" <garsilva@embeddedor.com> |
|---|---|
| Date | 2017-02-13 17:10 +0100 |
| Subject | [PATCH 2/2] usb: gadget: udc: remove unnecessary variable and update function prototype |
| Message-ID | <tarmp-5YZ-3@gated-at.bofh.it> |
| In reply to | #1579860 |
Remove unnecessary variable and update function prototype.
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
drivers/usb/gadget/udc/amd5536udc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index ded97a3..3f64a06 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -608,9 +608,8 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
}
/* frees pci pool descriptors of a DMA chain */
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
+static void udc_free_dma_chain(struct udc *dev, struct udc_request *req)
{
- int ret_val = 0;
struct udc_data_dma *td = req->td_data;
unsigned int i;
@@ -629,8 +628,6 @@ static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
td = NULL;
addr = addr_aux;
}
-
- return ret_val;
}
/* Frees request packet, called by gadget driver */
--
2.5.0
[toc] | [prev] | [next] | [standalone]
| From | Michal Nazarewicz <mina86@mina86.com> |
|---|---|
| Date | 2017-02-13 17:10 +0100 |
| Subject | Re: [PATCH 2/2] usb: gadget: udc: remove unnecessary variable and update function prototype |
| Message-ID | <tarmq-5YZ-31@gated-at.bofh.it> |
| In reply to | #1579909 |
On Mon, Feb 13 2017, Gustavo A. R. Silva wrote:
> Remove unnecessary variable and update function prototype.
>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
> ---
> drivers/usb/gadget/udc/amd5536udc.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
> index ded97a3..3f64a06 100644
> --- a/drivers/usb/gadget/udc/amd5536udc.c
> +++ b/drivers/usb/gadget/udc/amd5536udc.c
> @@ -608,9 +608,8 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
> }
>
> /* frees pci pool descriptors of a DMA chain */
> -static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
> +static void udc_free_dma_chain(struct udc *dev, struct udc_request *req)
> {
> - int ret_val = 0;
> struct udc_data_dma *td = req->td_data;
> unsigned int i;
>
> @@ -629,8 +628,6 @@ static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
> td = NULL;
> addr = addr_aux;
> }
> -
> - return ret_val;
> }
>
> /* Frees request packet, called by gadget driver */
> --
> 2.5.0
>
--
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
[toc] | [prev] | [next] | [standalone]
| From | Michal Nazarewicz <mina86@mina86.com> |
|---|---|
| Date | 2017-02-13 17:10 +0100 |
| Subject | Re: [PATCH 1/2] usb: gadget: udc: avoid use of freed pointer |
| Message-ID | <tarmp-5YZ-7@gated-at.bofh.it> |
| In reply to | #1579860 |
On Mon, Feb 13 2017, Gustavo A. R. Silva wrote:
> Rewrite udc_free_dma_chain() function to avoid use of pointer after free.
>
> Addresses-Coverity-ID: 1091172
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
> ---
> drivers/usb/gadget/udc/amd5536udc.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
> index ea03ca7..ded97a3 100644
> --- a/drivers/usb/gadget/udc/amd5536udc.c
> +++ b/drivers/usb/gadget/udc/amd5536udc.c
> @@ -611,21 +611,23 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
> static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
> {
> int ret_val = 0;
> - struct udc_data_dma *td;
> - struct udc_data_dma *td_last = NULL;
> + struct udc_data_dma *td = req->td_data;
> unsigned int i;
>
> + dma_addr_t addr_aux = 0x00;
Perhaps call it ‘addr_next’ or ‘next’?
> + dma_addr_t addr = (dma_addr_t)td->next;
> + td->next = 0x00;
> +
> DBG(dev, "free chain req = %p\n", req);
>
> /* do not free first desc., will be done by free for request */
> - td_last = req->td_data;
> - td = phys_to_virt(td_last->next);
> -
> for (i = 1; i < req->chain_len; i++) {
> - pci_pool_free(dev->data_requests, td,
> - (dma_addr_t)td_last->next);
> - td_last = td;
> - td = phys_to_virt(td_last->next);
> + td = phys_to_virt(addr);
> + addr_aux = (dma_addr_t)td->next;
> + td->next = 0x00;
This is unnecessary.
> + pci_pool_free(dev->data_requests, td, addr);
> + td = NULL;
Ditto.
> + addr = addr_aux;
> }
>
> return ret_val;
> --
> 2.5.0
>
--
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
[toc] | [prev] | [next] | [standalone]
| From | "Gustavo A. R. Silva" <garsilva@embeddedor.com> |
|---|---|
| Date | 2017-02-14 23:00 +0100 |
| Subject | Re: [PATCH 1/2] usb: gadget: udc: avoid use of freed pointer |
| Message-ID | <taTiG-7dj-15@gated-at.bofh.it> |
| In reply to | #1579910 |
Hi Michal,
Quoting Michal Nazarewicz <mina86@mina86.com>:
> On Mon, Feb 13 2017, Gustavo A. R. Silva wrote:
>> Rewrite udc_free_dma_chain() function to avoid use of pointer after free.
>>
>> Addresses-Coverity-ID: 1091172
>> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>
> Acked-by: Michal Nazarewicz <mina86@mina86.com>
>
>> ---
>> drivers/usb/gadget/udc/amd5536udc.c | 20 +++++++++++---------
>> 1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/udc/amd5536udc.c
>> b/drivers/usb/gadget/udc/amd5536udc.c
>> index ea03ca7..ded97a3 100644
>> --- a/drivers/usb/gadget/udc/amd5536udc.c
>> +++ b/drivers/usb/gadget/udc/amd5536udc.c
>> @@ -611,21 +611,23 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
>> static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
>> {
>> int ret_val = 0;
>> - struct udc_data_dma *td;
>> - struct udc_data_dma *td_last = NULL;
>> + struct udc_data_dma *td = req->td_data;
>> unsigned int i;
>>
>> + dma_addr_t addr_aux = 0x00;
>
> Perhaps call it ‘addr_next’ or ‘next’?
>
>> + dma_addr_t addr = (dma_addr_t)td->next;
>> + td->next = 0x00;
>> +
>> DBG(dev, "free chain req = %p\n", req);
>>
>> /* do not free first desc., will be done by free for request */
>> - td_last = req->td_data;
>> - td = phys_to_virt(td_last->next);
>> -
>> for (i = 1; i < req->chain_len; i++) {
>> - pci_pool_free(dev->data_requests, td,
>> - (dma_addr_t)td_last->next);
>> - td_last = td;
>> - td = phys_to_virt(td_last->next);
>> + td = phys_to_virt(addr);
>> + addr_aux = (dma_addr_t)td->next;
>> + td->next = 0x00;
>
> This is unnecessary.
>
>> + pci_pool_free(dev->data_requests, td, addr);
>> + td = NULL;
>
> Ditto.
>
>> + addr = addr_aux;
>> }
>>
>> return ret_val;
>> --
>> 2.5.0
>>
>
Thanks for your comments, I will send version 2 shortly.
--
Gustavo A. R. Silva
[toc] | [prev] | [next] | [standalone]
| From | "Gustavo A. R. Silva" <garsilva@embeddedor.com> |
|---|---|
| Date | 2017-02-14 23:00 +0100 |
| Subject | [PATCH v2 1/2] usb: gadget: udc: avoid use of freed pointer |
| Message-ID | <taTiG-7dj-17@gated-at.bofh.it> |
| In reply to | #1580895 |
Rewrite udc_free_dma_chain() function to avoid use of pointer after free.
Addresses-Coverity-ID: 1091172
Acked-by: Michal Nazarewicz <mina86@mina86.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
Changes in v2:
Remove 'td->next = 0x00' inside for loop.
Remove unnecessary pointer nullification after free.
Rename variable addr_aux to addr_next.
drivers/usb/gadget/udc/amd5536udc.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index ea03ca7..821d088 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -611,21 +611,20 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
{
int ret_val = 0;
- struct udc_data_dma *td;
- struct udc_data_dma *td_last = NULL;
+ struct udc_data_dma *td = req->td_data;
unsigned int i;
+ dma_addr_t addr_next = 0x00;
+ dma_addr_t addr = (dma_addr_t)td->next;
+
DBG(dev, "free chain req = %p\n", req);
/* do not free first desc., will be done by free for request */
- td_last = req->td_data;
- td = phys_to_virt(td_last->next);
-
for (i = 1; i < req->chain_len; i++) {
- pci_pool_free(dev->data_requests, td,
- (dma_addr_t)td_last->next);
- td_last = td;
- td = phys_to_virt(td_last->next);
+ td = phys_to_virt(addr);
+ addr_next = (dma_addr_t)td->next;
+ pci_pool_free(dev->data_requests, td, addr);
+ addr = addr_next;
}
return ret_val;
--
2.5.0
[toc] | [prev] | [next] | [standalone]
| From | "Gustavo A. R. Silva" <garsilva@embeddedor.com> |
|---|---|
| Date | 2017-02-14 23:10 +0100 |
| Subject | [PATCH v2 2/2] usb: gadget: udc: remove unnecessary variable and update function prototype |
| Message-ID | <taTsm-7x0-33@gated-at.bofh.it> |
| In reply to | #1580896 |
Remove unnecessary variable and update function prototype.
Acked-by: Michal Nazarewicz <mina86@mina86.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
Changes in v2:
None.
drivers/usb/gadget/udc/amd5536udc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index 821d088..67dd209 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -608,9 +608,8 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
}
/* frees pci pool descriptors of a DMA chain */
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
+static void udc_free_dma_chain(struct udc *dev, struct udc_request *req)
{
- int ret_val = 0;
struct udc_data_dma *td = req->td_data;
unsigned int i;
@@ -626,8 +625,6 @@ static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
pci_pool_free(dev->data_requests, td, addr);
addr = addr_next;
}
-
- return ret_val;
}
/* Frees request packet, called by gadget driver */
--
2.5.0
[toc] | [prev] | [next] | [standalone]
| From | "Gustavo A. R. Silva" <garsilva@embeddedor.com> |
|---|---|
| Date | 2017-02-13 17:10 +0100 |
| Subject | [PATCH 1/2] usb: gadget: udc: avoid use of freed pointer |
| Message-ID | <tarmp-5YZ-9@gated-at.bofh.it> |
| In reply to | #1579860 |
Rewrite udc_free_dma_chain() function to avoid use of pointer after free.
Addresses-Coverity-ID: 1091172
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
drivers/usb/gadget/udc/amd5536udc.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/amd5536udc.c b/drivers/usb/gadget/udc/amd5536udc.c
index ea03ca7..ded97a3 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -611,21 +611,23 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
{
int ret_val = 0;
- struct udc_data_dma *td;
- struct udc_data_dma *td_last = NULL;
+ struct udc_data_dma *td = req->td_data;
unsigned int i;
+ dma_addr_t addr_aux = 0x00;
+ dma_addr_t addr = (dma_addr_t)td->next;
+ td->next = 0x00;
+
DBG(dev, "free chain req = %p\n", req);
/* do not free first desc., will be done by free for request */
- td_last = req->td_data;
- td = phys_to_virt(td_last->next);
-
for (i = 1; i < req->chain_len; i++) {
- pci_pool_free(dev->data_requests, td,
- (dma_addr_t)td_last->next);
- td_last = td;
- td = phys_to_virt(td_last->next);
+ td = phys_to_virt(addr);
+ addr_aux = (dma_addr_t)td->next;
+ td->next = 0x00;
+ pci_pool_free(dev->data_requests, td, addr);
+ td = NULL;
+ addr = addr_aux;
}
return ret_val;
--
2.5.0
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web