netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: marcel@holtmann.org, bluez-devel@lists.sourceforge.net,
	netdev@oss.sgi.com, viro@zenII.linux.org.uk
Subject: some bluetooth fixes
Date: Fri, 6 Feb 2004 05:00:42 +0100	[thread overview]
Message-ID: <20040206050042.20a2b3b0.ak@suse.de> (raw)


While reading bluetooth code I noticed some bugs and potential overflows.

Also I commented one ref-counting bug.

Patch against 2.6.2.

-Andi

diff -u linux-2.6.2-work32/net/bluetooth/rfcomm/tty.c-o linux-2.6.2-work32/net/bluetooth/rfcomm/tty.c
--- linux-2.6.2-work32/net/bluetooth/rfcomm/tty.c-o	2003-09-28 10:53:25.000000000 +0200
+++ linux-2.6.2-work32/net/bluetooth/rfcomm/tty.c	2004-02-06 04:58:28.000000000 +0100
@@ -349,7 +349,7 @@
 	struct rfcomm_dev_list_req *dl;
 	struct rfcomm_dev_info *di;
 	struct list_head *p;
-	int n = 0, size;
+	int n = 0, size, err;
 	u16 dev_num;
 
 	BT_DBG("");
@@ -362,8 +362,8 @@
 
 	size = sizeof(*dl) + dev_num * sizeof(*di);
 
-	if (verify_area(VERIFY_WRITE, (void *)arg, size))
-		return -EFAULT;
+	if (size > (4*PAGE_SIZE)/sizeof(*di))
+		return -EINVAL;
 
 	if (!(dl = kmalloc(size, GFP_KERNEL)))
 		return -ENOMEM;
@@ -389,9 +389,9 @@
 	dl->dev_num = n;
 	size = sizeof(*dl) + n * sizeof(*di);
 
-	copy_to_user((void *) arg, dl, size);
+	err = copy_to_user((void *) arg, dl, size) ? -EFAULT : 0;
 	kfree(dl);
-	return 0;
+	return err;
 }
 
 static int rfcomm_get_dev_info(unsigned long arg)
@@ -549,8 +549,10 @@
 
 	BT_DBG("dev %p dst %s channel %d opened %d", dev, batostr(&dev->dst), dev->channel, dev->opened);
 
-	if (dev->opened++ != 0)
+	if (dev->opened++ != 0) {
+		rfcomm_dev_put(dev);
 		return 0;
+	}
 
 	dlc = dev->dlc;
 
@@ -563,8 +565,10 @@
 	set_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
 
 	err = rfcomm_dlc_open(dlc, &dev->src, &dev->dst, dev->channel);
-	if (err < 0)
+	if (err < 0) { 
+		rfcomm_dev_put(dev);
 		return err;
+	}
 
 	/* Wait for DLC to connect */
 	add_wait_queue(&dev->wait, &wait);
diff -u linux-2.6.2-work32/net/bluetooth/cmtp/sock.c-o linux-2.6.2-work32/net/bluetooth/cmtp/sock.c
--- linux-2.6.2-work32/net/bluetooth/cmtp/sock.c-o	2004-02-05 08:09:54.000000000 +0100
+++ linux-2.6.2-work32/net/bluetooth/cmtp/sock.c	2004-02-05 14:57:57.000000000 +0100
@@ -87,8 +87,10 @@
 		if (!nsock)
 			return err;
 
-		if (nsock->sk->sk_state != BT_CONNECTED)
+		if (nsock->sk->sk_state != BT_CONNECTED) { 
+			fput(nsock->file);
 			return -EBADFD;
+		}
 
 		err = cmtp_add_connection(&ca, nsock);
 		if (!err) {
diff -u linux-2.6.2-work32/net/bluetooth/hci_sock.c-o linux-2.6.2-work32/net/bluetooth/hci_sock.c
--- linux-2.6.2-work32/net/bluetooth/hci_sock.c-o	2004-02-05 08:09:54.000000000 +0100
+++ linux-2.6.2-work32/net/bluetooth/hci_sock.c	2004-02-05 14:57:59.000000000 +0100
@@ -392,6 +392,8 @@
 
 	skb->pkt_type = *((unsigned char *) skb->data);
 	skb_pull(skb, 1);
+	/* AK: looks broken. Who guarantees that hdev doesn't go away while
+	   the skb is queued ? */
 	skb->dev = (void *) hdev;
 
 	if (skb->pkt_type == HCI_COMMAND_PKT) {
diff -u linux-2.6.2-work32/net/bluetooth/hci_conn.c-o linux-2.6.2-work32/net/bluetooth/hci_conn.c
--- linux-2.6.2-work32/net/bluetooth/hci_conn.c-o	2004-02-05 08:09:54.000000000 +0100
+++ linux-2.6.2-work32/net/bluetooth/hci_conn.c	2004-02-05 15:06:10.000000000 +0100
@@ -353,21 +353,24 @@
 	struct hci_conn_info *ci;
 	struct hci_dev *hdev;
 	struct list_head *p;
-	int n = 0, size;
+	int n = 0, size, err;
 
 	if (copy_from_user(&req, (void *) arg, sizeof(req)))
 		return -EFAULT;
 
-	if (!(hdev = hci_dev_get(req.dev_id)))
-		return -ENODEV;
+	if (req.conn_num >= (2*PAGE_SIZE)/sizeof(struct hci_conn_info))
+		return -EINVAL; 
 
 	size = req.conn_num * sizeof(struct hci_conn_info) + sizeof(req);
 
-	if (verify_area(VERIFY_WRITE, (void *)arg, size))
-		return -EFAULT;
-
 	if (!(cl = (void *) kmalloc(size, GFP_KERNEL)))
 		return -ENOMEM;
+
+	if (!(hdev = hci_dev_get(req.dev_id))) { 
+		kfree(cl);
+		return -ENODEV;
+	}
+
 	ci = cl->conn_info;
 
 	hci_dev_lock_bh(hdev);
@@ -375,6 +378,9 @@
 		register struct hci_conn *c;
 		c = list_entry(p, struct hci_conn, list);
 
+		if (n >= req.conn_num) 
+			break;
+
 		bacpy(&(ci + n)->bdaddr, &c->dst);
 		(ci + n)->handle = c->handle;
 		(ci + n)->type  = c->type;
@@ -391,10 +397,10 @@
 
 	hci_dev_put(hdev);
 
-	copy_to_user((void *) arg, cl, size);
+	err = copy_to_user((void *) arg, cl, size) ? -EFAULT : 0; 
 	kfree(cl);
 
-	return 0;
+	return err;
 }
 
 int hci_get_conn_info(struct hci_dev *hdev, unsigned long arg)
@@ -407,9 +413,6 @@
 	if (copy_from_user(&req, (void *) arg, sizeof(req)))
 		return -EFAULT;
 
-	if (verify_area(VERIFY_WRITE, ptr, sizeof(ci)))
-		return -EFAULT;
-
 	hci_dev_lock_bh(hdev);
 	conn = hci_conn_hash_lookup_ba(hdev, req.type, &req.bdaddr);
 	if (conn) {
@@ -425,6 +428,5 @@
 	if (!conn)
 		return -ENOENT;
 
-	copy_to_user(ptr, &ci, sizeof(ci));
-	return 0;
+	return copy_to_user(ptr, &ci, sizeof(ci)) ? -EFAULT : 0;
 }
diff -u linux-2.6.2-work32/net/bluetooth/hci_core.c-o linux-2.6.2-work32/net/bluetooth/hci_core.c
--- linux-2.6.2-work32/net/bluetooth/hci_core.c-o	2004-02-05 08:09:54.000000000 +0100
+++ linux-2.6.2-work32/net/bluetooth/hci_core.c	2004-02-05 15:01:46.000000000 +0100
@@ -715,18 +715,16 @@
 	struct list_head *p;
 	int n = 0, size;
 	__u16 dev_num;
-
+	int err;
+	
 	if (get_user(dev_num, (__u16 *) arg))
 		return -EFAULT;
 
-	if (!dev_num)
+	if (!dev_num || dev_num >= (4*PAGE_SIZE)/sizeof(*dr))
 		return -EINVAL;
 	
 	size = dev_num * sizeof(*dr) + sizeof(*dl);
 
-	if (verify_area(VERIFY_WRITE, (void *) arg, size))
-		return -EFAULT;
-
 	if (!(dl = kmalloc(size, GFP_KERNEL)))
 		return -ENOMEM;
 	dr = dl->dev_req;
@@ -745,10 +743,10 @@
 	dl->dev_num = n;
 	size = n * sizeof(*dr) + sizeof(*dl);
 
-	copy_to_user((void *) arg, dl, size);
+	err = copy_to_user((void *) arg, dl, size);
 	kfree(dl);
 
-	return 0;
+	return err ? -EFAULT : 0;
 }
 
 int hci_get_dev_info(unsigned long arg)

             reply	other threads:[~2004-02-06  4:00 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-02-06  4:00 Andi Kleen [this message]
2004-02-06 14:58 ` some bluetooth fixes Marcel Holtmann
2004-02-07  2:24   ` Andi Kleen
2004-02-07 11:13     ` Marcel Holtmann
2004-02-07 11:57       ` Andi Kleen
2004-02-07 16:57         ` Marcel Holtmann
2004-02-07 17:24           ` Andi Kleen
2004-02-11 18:55             ` Marcel Holtmann
2004-02-11 19:33               ` Andi Kleen
2004-02-11 20:47                 ` Marcel Holtmann
2004-02-11 19:55               ` Andi Kleen
2004-02-06 23:30 ` Marcel Holtmann
2004-02-06 23:34   ` David S. Miller
2004-02-06 23:46     ` Marcel Holtmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20040206050042.20a2b3b0.ak@suse.de \
    --to=ak@suse.de \
    --cc=bluez-devel@lists.sourceforge.net \
    --cc=marcel@holtmann.org \
    --cc=netdev@oss.sgi.com \
    --cc=viro@zenII.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).