Re: [PATCH] drivers: usb: gadget: udc: remove pointer dereference after free

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andy,

Quoting Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>:

On Wed, 2017-02-08 at 13:15 -0600, Gustavo A. R. Silva wrote:
Remove pointer dereference and write after free.

It's wrong description. There is no write after free. The memory is
still in pool and one may access it. Though the access is *formally*
illegal.

Code itself looks interesting.


Addresses-Coverity-ID: 1091173
Signed-off-by: Gustavo A. R. Silva <garsilva@xxxxxxxxxxxxxx>
---
 drivers/usb/gadget/udc/pch_udc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/pch_udc.c
b/drivers/usb/gadget/udc/pch_udc.c
index a97da64..8a365aa 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -1523,7 +1523,6 @@ static void pch_udc_free_dma_chain(struct
pch_udc_dev *dev,
 		td = phys_to_virt(addr);
 		addr2 = (dma_addr_t)td->next;
 		pci_pool_free(dev->data_requests, td, addr);
-		td->next = 0x00;

I think the better fix is to move this line before pci_pool_free() call.
I dunno those td->next = 0x00; make any sense there.


Yeah, I think it is better to do as you say.
It seems to me that the intention of that line of code is to get rid of the address 'next' points to, after it has been backed up into 'addr2'.

I will also change the description.

Is it done under some lock / serialization?

 		addr = addr2;
 	}
 	req->chain_len = 1;

--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy

Thanks
--
Gustavo A. R. Silva




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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux