[PATCH V2] usb: gadget: composite: Fix possible double free memory bug

[PATCH V2] usb: gadget: composite: Fix possible double free memory bug

[Chandana Kishori Chiluveru]

composite_dev_cleanup call from the failure of configfs_composite_bind
frees up the cdev->os_desc_req and cdev->req. If the previous calls of
bind and unbind is successful these will carry stale values.

Consider the below sequence of function calls:
configfs_composite_bind()
        composite_dev_prepare()
                - Allocate cdev->req, cdev->req->buf
        composite_os_desc_req_prepare()
                - Allocate cdev->os_desc_req, cdev->os_desc_req->buf
configfs_composite_unbind()
        composite_dev_cleanup()
                - free the cdev->os_desc_req->buf and cdev->req->buf
Next composition switch
configfs_composite_bind()
        - If it fails goto err_comp_cleanup will call the
	  composite_dev_cleanup() function
        composite_dev_cleanup()
	        - calls kfree up with the stale values of cdev->req->buf and
		  cdev->os_desc_req from the previous configfs_composite_bind
		  call. The free call on these stale values leads to double free.

Hence, Fix this issue by setting request and buffer pointer to NULL after
kfree.

Signed-off-by: Chandana Kishori Chiluveru <cchiluve@codeaurora.org>

Changes in v2:
	- Modified commit text.
---
 drivers/usb/gadget/composite.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index b8a1584..992f1e2 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2155,14 +2155,18 @@ void composite_dev_cleanup(struct usb_composite_dev *cdev)
 			usb_ep_dequeue(cdev->gadget->ep0, cdev->os_desc_req);
 
 		kfree(cdev->os_desc_req->buf);
+		cdev->os_desc_req->buf = NULL;
 		usb_ep_free_request(cdev->gadget->ep0, cdev->os_desc_req);
+		cdev->os_desc_req = NULL;
 	}
 	if (cdev->req) {
 		if (cdev->setup_pending)
 			usb_ep_dequeue(cdev->gadget->ep0, cdev->req);
 
 		kfree(cdev->req->buf);
+		cdev->req->buf = NULL;
 		usb_ep_free_request(cdev->gadget->ep0, cdev->req);
+		cdev->req = NULL;
 	}
 	cdev->next_string_id = 0;
 	device_remove_file(&cdev->gadget->dev, &dev_attr_suspended);
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH V2] usb: gadget: composite: Fix possible double free memory bug

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]


Hi,

Chandana Kishori Chiluveru <cchiluve@codeaurora.org> writes:
> composite_dev_cleanup call from the failure of configfs_composite_bind
> frees up the cdev->os_desc_req and cdev->req. If the previous calls of
> bind and unbind is successful these will carry stale values.
>
> Consider the below sequence of function calls:
> configfs_composite_bind()
>         composite_dev_prepare()
>                 - Allocate cdev->req, cdev->req->buf
>         composite_os_desc_req_prepare()
>                 - Allocate cdev->os_desc_req, cdev->os_desc_req->buf
> configfs_composite_unbind()
>         composite_dev_cleanup()
>                 - free the cdev->os_desc_req->buf and cdev->req->buf
> Next composition switch
> configfs_composite_bind()
>         - If it fails goto err_comp_cleanup will call the
> 	  composite_dev_cleanup() function
>         composite_dev_cleanup()
> 	        - calls kfree up with the stale values of cdev->req->buf and
> 		  cdev->os_desc_req from the previous configfs_composite_bind
> 		  call. The free call on these stale values leads to double free.
>
> Hence, Fix this issue by setting request and buffer pointer to NULL after
> kfree.
>
> Signed-off-by: Chandana Kishori Chiluveru <cchiluve@codeaurora.org>
>
> Changes in v2:
> 	- Modified commit text.

These two lines...

> ---

... should be after this tearline :-)

We don't need that in the commit log

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]