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

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-next>] [day] [month] [year] [list]

Date:	Sat,  9 Mar 2013 00:50:47 +0800
From:	Alexey Khoroshilov <khoroshilov@...ras.ru>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	Alexey Khoroshilov <khoroshilov@...ras.ru>,
	Hans de Goede <hdegoede@...hat.com>,
	Alan Stern <stern@...land.harvard.edu>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	ldv-project@...uxtesting.org
Subject: [PATCH] usb/core/devio.c: Don't use GFP_KERNEL while we cannot reset a storage device

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 <khoroshilov@...ras.ru>
---
 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 (ret) {
@@ -1798,7 +1798,7 @@ static int proc_ioctl(struct dev_state *ps, struct usbdevfs_ioctl *ctl)
 
 	/* alloc buffer */
 	if ((size = _IOC_SIZE(ctl->ioctl_code)) > 0) {
-		if ((buf = kmalloc(size, GFP_KERNEL)) == NULL)
+		if ((buf = kmalloc(size, GFP_NOIO)) == NULL)
 			return -ENOMEM;
 		if ((_IOC_DIR(ctl->ioctl_code) & _IOC_WRITE)) {
 			if (copy_from_user(buf, ctl->data, size)) {
-- 
1.7.9.5

--
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/