lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 28 Dec 2015 14:25:50 +0800
From:	changbin.du@...el.com
To:	balbi@...com, gregkh@...uxfoundation.org
Cc:	r.baldyga@...kerion.com, nab@...ux-iscsi.org,
	andrzej.p@...sung.com, laurent.pinchart@...asonboard.com,
	prabhakar.csengg@...il.com, kopasiak90@...il.com,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	"Du, Changbin" <changbin.du@...el.com>
Subject: [PATCH v2] usb: gadget: fix double mem free for usb_request caused by wild pointers

From: "Du, Changbin" <changbin.du@...el.com>

acm, ecm, hid, ncm, phonet, rndis and uvc functions all have double
memory free issue. Set pointers to NULL after freed to avoid this.

Here explain how it happen on acm function, others has analogical case.

If acm_bind fails before allocate notification and acm->notify_req is
not set to NULL after freed last time, double free will happen.

kernel BUG at mm/slub.c:3392!
invalid opcode: 0000 [#1] PREEMPT SMP
EIP is at kfree+0x172/0x180
Call Trace:
[<80c0e3b6>] ? usb_ep_autoconfig_ss+0x86/0x170
[<80c13345>] gs_free_req+0x15/0x30
[<80c12df1>] acm_bind+0x1c1/0x2d0
[<80c0e9be>] usb_add_function+0x6e/0x120
[<80c213cb>] acm_function_bind_config+0x2b/0x90

Signed-off-by: Du, Changbin <changbin.du@...el.com>
---
changes from v1: 
  1. add same fix for ecm, hid, ncm... fucntions
  2. patch title changed
---
 drivers/usb/gadget/function/f_acm.c    |  8 ++++++--
 drivers/usb/gadget/function/f_ecm.c    |  2 ++
 drivers/usb/gadget/function/f_hid.c    |  5 ++++-
 drivers/usb/gadget/function/f_ncm.c    |  2 ++
 drivers/usb/gadget/function/f_phonet.c | 11 ++++++++---
 drivers/usb/gadget/function/f_rndis.c  |  2 ++
 drivers/usb/gadget/function/f_uvc.c    |  5 ++++-
 7 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index 2fa1e80..e10c8d4 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -699,8 +699,10 @@ acm_bind(struct usb_configuration *c, struct usb_function *f)
 	return 0;
 
 fail:
-	if (acm->notify_req)
+	if (acm->notify_req) {
 		gs_free_req(acm->notify, acm->notify_req);
+		acm->notify_req = NULL;
+	}
 
 	ERROR(cdev, "%s/%p: can't bind, err %d\n", f->name, f, status);
 
@@ -713,8 +715,10 @@ static void acm_unbind(struct usb_configuration *c, struct usb_function *f)
 
 	acm_string_defs[0].id = 0;
 	usb_free_all_descriptors(f);
-	if (acm->notify_req)
+	if (acm->notify_req) {
 		gs_free_req(acm->notify, acm->notify_req);
+		acm->notify_req = NULL;
+	}
 }
 
 static void acm_free_func(struct usb_function *f)
diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index 7ad60ee..23092a2 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -809,6 +809,7 @@ fail:
 	if (ecm->notify_req) {
 		kfree(ecm->notify_req->buf);
 		usb_ep_free_request(ecm->notify, ecm->notify_req);
+		ecm->notify_req = NULL;
 	}
 
 	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
@@ -907,6 +908,7 @@ static void ecm_unbind(struct usb_configuration *c, struct usb_function *f)
 
 	kfree(ecm->notify_req->buf);
 	usb_ep_free_request(ecm->notify, ecm->notify_req);
+	ecm->notify_req = NULL;
 }
 
 static struct usb_function *ecm_alloc(struct usb_function_instance *fi)
diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 99285b4..827e385 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -679,8 +679,10 @@ fail:
 	ERROR(f->config->cdev, "hidg_bind FAILED\n");
 	if (hidg->req != NULL) {
 		kfree(hidg->req->buf);
-		if (hidg->in_ep != NULL)
+		if (hidg->in_ep != NULL) {
 			usb_ep_free_request(hidg->in_ep, hidg->req);
+			hidg->req = NULL;
+		}
 	}
 
 	return status;
@@ -912,6 +914,7 @@ static void hidg_unbind(struct usb_configuration *c, struct usb_function *f)
 	usb_ep_disable(hidg->in_ep);
 	kfree(hidg->req->buf);
 	usb_ep_free_request(hidg->in_ep, hidg->req);
+	hidg->req = NULL;
 
 	usb_free_all_descriptors(f);
 }
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 7ad798a..510d1d7 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1459,6 +1459,7 @@ fail:
 	if (ncm->notify_req) {
 		kfree(ncm->notify_req->buf);
 		usb_ep_free_request(ncm->notify, ncm->notify_req);
+		ncm->notify_req = NULL;
 	}
 
 	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
@@ -1561,6 +1562,7 @@ static void ncm_unbind(struct usb_configuration *c, struct usb_function *f)
 
 	kfree(ncm->notify_req->buf);
 	usb_ep_free_request(ncm->notify, ncm->notify_req);
+	ncm->notify_req = NULL;
 }
 
 static struct usb_function *ncm_alloc(struct usb_function_instance *fi)
diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index 157441d..5e146e1 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -569,8 +569,10 @@ static int pn_bind(struct usb_configuration *c, struct usb_function *f)
 	return 0;
 
 err_req:
-	for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++)
+	for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++) {
 		usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
+		fp->out_reqv[i] = NULL;
+	}
 	usb_free_all_descriptors(f);
 err:
 	ERROR(cdev, "USB CDC Phonet: cannot autoconfigure\n");
@@ -662,9 +664,12 @@ static void pn_unbind(struct usb_configuration *c, struct usb_function *f)
 	/* We are already disconnected */
 	if (fp->in_req)
 		usb_ep_free_request(fp->in_ep, fp->in_req);
-	for (i = 0; i < phonet_rxq_size; i++)
-		if (fp->out_reqv[i])
+	for (i = 0; i < phonet_rxq_size; i++) {
+		if (fp->out_reqv[i]) {
 			usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
+			fp->out_reqv[i] = NULL;
+		}
+	}
 
 	usb_free_all_descriptors(f);
 }
diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
index e587767..804a717 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -821,6 +821,7 @@ fail:
 	if (rndis->notify_req) {
 		kfree(rndis->notify_req->buf);
 		usb_ep_free_request(rndis->notify, rndis->notify_req);
+		rndis->notify_req = NULL;
 	}
 
 	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
@@ -948,6 +949,7 @@ static void rndis_unbind(struct usb_configuration *c, struct usb_function *f)
 
 	kfree(rndis->notify_req->buf);
 	usb_ep_free_request(rndis->notify, rndis->notify_req);
+	rndis->notify_req = NULL;
 }
 
 static struct usb_function *rndis_alloc(struct usb_function_instance *fi)
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 29b41b5..f3a93bb 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -736,8 +736,10 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 error:
 	v4l2_device_unregister(&uvc->v4l2_dev);
 
-	if (uvc->control_req)
+	if (uvc->control_req) {
 		usb_ep_free_request(cdev->gadget->ep0, uvc->control_req);
+		uvc->control_req = NULL;
+	}
 	kfree(uvc->control_buf);
 
 	usb_free_all_descriptors(f);
@@ -864,6 +866,7 @@ static void uvc_unbind(struct usb_configuration *c, struct usb_function *f)
 	v4l2_device_unregister(&uvc->v4l2_dev);
 
 	usb_ep_free_request(cdev->gadget->ep0, uvc->control_req);
+	uvc->control_req = NULL;
 	kfree(uvc->control_buf);
 
 	usb_free_all_descriptors(f);
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ