* [Bluez-devel] Re: some bluetooth fixes [not found] <20040206050042.20a2b3b0.ak@suse.de> @ 2004-02-06 14:58 ` Marcel Holtmann [not found] ` <20040207032428.56ffbebc.ak@suse.de> 2004-02-06 23:30 ` Marcel Holtmann 1 sibling, 1 reply; 10+ messages in thread From: Marcel Holtmann @ 2004-02-06 14:58 UTC (permalink / raw) To: Andi Kleen; +Cc: bluez-devel, netdev, viro Hi Andi, > While reading bluetooth code I noticed some bugs and potential overflows. > > Also I commented one ref-counting bug. > > Patch against 2.6.2. thanks for looking at it. Same applies to 2.4, right? > 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) { The same should apply to net/bluetooth/bnep/sock.c > 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) { Why should hdev go away? > 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); Why 2*PAGE_SIZE in this case? What is different? Regards Marcel ------------------------------------------------------- The SF.Net email is sponsored by EclipseCon 2004 Premiere Conference on Open Tools Development and Integration See the breadth of Eclipse activity. February 3-5 in Anaheim, CA. http://www.eclipsecon.org/osdn _______________________________________________ Bluez-devel mailing list Bluez-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bluez-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20040207032428.56ffbebc.ak@suse.de>]
* [Bluez-devel] Re: some bluetooth fixes [not found] ` <20040207032428.56ffbebc.ak@suse.de> @ 2004-02-07 11:13 ` Marcel Holtmann [not found] ` <20040207125723.391a1fcd.ak@suse.de> 0 siblings, 1 reply; 10+ messages in thread From: Marcel Holtmann @ 2004-02-07 11:13 UTC (permalink / raw) To: Andi Kleen; +Cc: bluez-devel, netdev, viro Hi Andi, > > > 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) { > > > > Why should hdev go away? > > Because the skbuff is queued, but there is no reference count to keep the device around. > I wasn't 100% sure on that, so I just commented it. Feel free to remove if you think > it's correct. The queue itself is part of the hdev structure and the only call that let hdev go away is hci_unregister_dev and this clears the queue. So I don't see a problem here. > > > 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); > > > > Why 2*PAGE_SIZE in this case? What is different? > > It's just an arbitary number. Mainly to stop overflow attacks on > user controlled values. e.g. user can pass UINT_MAX for conn_num. > The kmalloc would overflow and succeed. But a later loop running > through the values could do wrong things on the too small buffer. > The code seems to not be vunerable to this, but only by luck. > > Also in general it's good practice to stop user controlled kmalloc > at a reasonable size. I check this. Maybe we have more of them. What do you propose as max size value for kmalloc? 2*PAGE_SIZE or 4*PAGE_SIZE? Regards Marcel ------------------------------------------------------- The SF.Net email is sponsored by EclipseCon 2004 Premiere Conference on Open Tools Development and Integration See the breadth of Eclipse activity. February 3-5 in Anaheim, CA. http://www.eclipsecon.org/osdn _______________________________________________ Bluez-devel mailing list Bluez-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bluez-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20040207125723.391a1fcd.ak@suse.de>]
* [Bluez-devel] Re: some bluetooth fixes [not found] ` <20040207125723.391a1fcd.ak@suse.de> @ 2004-02-07 16:57 ` Marcel Holtmann 2004-02-07 17:24 ` Andi Kleen 0 siblings, 1 reply; 10+ messages in thread From: Marcel Holtmann @ 2004-02-07 16:57 UTC (permalink / raw) To: Andi Kleen; +Cc: bluez-devel, netdev, viro [-- Attachment #1: Type: text/plain, Size: 587 bytes --] Hi Andi, > > I check this. Maybe we have more of them. What do you propose as max > > size value for kmalloc? 2*PAGE_SIZE or 4*PAGE_SIZE? > > What better fits the intended use case. I don't know how many objects are expected > here. Smaller is better probably. I now looked carefully through your patch and changed and added some parts to better fit into. I also fixed another RFCOMM refcount bug. Please review it, before I send it to Dave. > If you want to handle more objects this way you should use seq_file instead. The general plan is to move over to sysfs. Regards Marcel [-- Attachment #2: patch --] [-- Type: text/x-patch, Size: 4372 bytes --] diff -Nru a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c --- a/net/bluetooth/hci_conn.c Sat Feb 7 17:52:09 2004 +++ b/net/bluetooth/hci_conn.c Sat Feb 7 17:52:09 2004 @@ -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; - - size = req.conn_num * sizeof(struct hci_conn_info) + sizeof(req); + size = sizeof(req) + req.conn_num * sizeof(*ci); - if (verify_area(VERIFY_WRITE, (void *)arg, size)) - return -EFAULT; + if (size > PAGE_SIZE * 2) + return -EINVAL; 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); @@ -381,20 +384,21 @@ (ci + n)->out = c->out; (ci + n)->state = c->state; (ci + n)->link_mode = c->link_mode; - n++; + if (++n >= req.conn_num) + break; } hci_dev_unlock_bh(hdev); cl->dev_id = hdev->id; cl->conn_num = n; - size = n * sizeof(struct hci_conn_info) + sizeof(req); + size = sizeof(req) + n * sizeof(*ci); hci_dev_put(hdev); - copy_to_user((void *) arg, cl, size); + err = copy_to_user((void *) arg, cl, size); kfree(cl); - return 0; + return err ? -EFAULT : 0; } int hci_get_conn_info(struct hci_dev *hdev, unsigned long arg) @@ -407,9 +411,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 +426,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 -Nru a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c --- a/net/bluetooth/hci_core.c Sat Feb 7 17:52:09 2004 +++ b/net/bluetooth/hci_core.c Sat Feb 7 17:52:09 2004 @@ -716,7 +716,7 @@ struct hci_dev_list_req *dl; struct hci_dev_req *dr; struct list_head *p; - int n = 0, size; + int n = 0, size, err; __u16 dev_num; if (get_user(dev_num, (__u16 *) arg)) @@ -724,14 +724,15 @@ if (!dev_num) return -EINVAL; - - size = dev_num * sizeof(*dr) + sizeof(*dl); - if (verify_area(VERIFY_WRITE, (void *) arg, size)) - return -EFAULT; + size = sizeof(*dl) + dev_num * sizeof(*dr); + + if (size > PAGE_SIZE * 2) + return -EINVAL; if (!(dl = kmalloc(size, GFP_KERNEL))) return -ENOMEM; + dr = dl->dev_req; read_lock_bh(&hci_dev_list_lock); @@ -746,12 +747,12 @@ read_unlock_bh(&hci_dev_list_lock); dl->dev_num = n; - size = n * sizeof(*dr) + sizeof(*dl); + size = sizeof(*dl) + n * sizeof(*dr); - 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) diff -Nru a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c --- a/net/bluetooth/rfcomm/tty.c Sat Feb 7 17:52:09 2004 +++ b/net/bluetooth/rfcomm/tty.c Sat Feb 7 17:52:09 2004 @@ -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 > PAGE_SIZE * 4) + return -EINVAL; if (!(dl = kmalloc(size, GFP_KERNEL))) return -ENOMEM; @@ -389,9 +389,10 @@ 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); kfree(dl); - return 0; + + return err ? -EFAULT : 0; } static int rfcomm_get_dev_info(unsigned long arg) @@ -563,8 +564,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); @@ -588,6 +591,9 @@ } set_current_state(TASK_RUNNING); remove_wait_queue(&dev->wait, &wait); + + if (err < 0) + rfcomm_dev_put(dev); return err; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: some bluetooth fixes 2004-02-07 16:57 ` Marcel Holtmann @ 2004-02-07 17:24 ` Andi Kleen 2004-02-11 18:55 ` [Bluez-devel] " Marcel Holtmann 0 siblings, 1 reply; 10+ messages in thread From: Andi Kleen @ 2004-02-07 17:24 UTC (permalink / raw) To: Marcel Holtmann; +Cc: Andi Kleen, bluez-devel, netdev, viro On Sat, Feb 07, 2004 at 05:57:48PM +0100, Marcel Holtmann wrote: > I now looked carefully through your patch and changed and added some > parts to better fit into. I also fixed another RFCOMM refcount bug. > Please review it, before I send it to Dave. Doing size checks after the multiply is too late - they could have already overflowed. You have to check the raw value from the user. -Andi ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bluez-devel] Re: some bluetooth fixes 2004-02-07 17:24 ` Andi Kleen @ 2004-02-11 18:55 ` Marcel Holtmann 2004-02-14 23:25 ` Andi Kleen 0 siblings, 1 reply; 10+ messages in thread From: Marcel Holtmann @ 2004-02-11 18:55 UTC (permalink / raw) To: Andi Kleen; +Cc: BlueZ Mailing List, netdev, viro [-- Attachment #1: Type: text/plain, Size: 190 bytes --] Hi Andi, > Doing size checks after the multiply is too late - they could > have already overflowed. You have to check the raw value from the user. new patch is attached. Regards Marcel [-- Attachment #2: patch --] [-- Type: text/x-patch, Size: 4511 bytes --] diff -Nru a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c --- a/net/bluetooth/hci_conn.c Wed Feb 11 19:39:20 2004 +++ b/net/bluetooth/hci_conn.c Wed Feb 11 19:39:20 2004 @@ -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; - - size = req.conn_num * sizeof(struct hci_conn_info) + sizeof(req); + if (req.conn_num * sizeof(*ci) > PAGE_SIZE * 2) + return -EINVAL; - if (verify_area(VERIFY_WRITE, (void *)arg, size)) - return -EFAULT; + size = sizeof(req) + req.conn_num * sizeof(*ci); 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); @@ -381,20 +384,21 @@ (ci + n)->out = c->out; (ci + n)->state = c->state; (ci + n)->link_mode = c->link_mode; - n++; + if (++n >= req.conn_num) + break; } hci_dev_unlock_bh(hdev); cl->dev_id = hdev->id; cl->conn_num = n; - size = n * sizeof(struct hci_conn_info) + sizeof(req); + size = sizeof(req) + n * sizeof(*ci); hci_dev_put(hdev); - copy_to_user((void *) arg, cl, size); + err = copy_to_user((void *) arg, cl, size); kfree(cl); - return 0; + return err ? -EFAULT : 0; } int hci_get_conn_info(struct hci_dev *hdev, unsigned long arg) @@ -407,9 +411,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 +426,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 -Nru a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c --- a/net/bluetooth/hci_core.c Wed Feb 11 19:39:20 2004 +++ b/net/bluetooth/hci_core.c Wed Feb 11 19:39:20 2004 @@ -716,7 +716,7 @@ struct hci_dev_list_req *dl; struct hci_dev_req *dr; struct list_head *p; - int n = 0, size; + int n = 0, size, err; __u16 dev_num; if (get_user(dev_num, (__u16 *) arg)) @@ -724,14 +724,15 @@ if (!dev_num) return -EINVAL; - - size = dev_num * sizeof(*dr) + sizeof(*dl); - if (verify_area(VERIFY_WRITE, (void *) arg, size)) - return -EFAULT; + if (dev_num * sizeof(*dr) > PAGE_SIZE * 2) + return -EINVAL; + + size = sizeof(*dl) + dev_num * sizeof(*dr); if (!(dl = kmalloc(size, GFP_KERNEL))) return -ENOMEM; + dr = dl->dev_req; read_lock_bh(&hci_dev_list_lock); @@ -746,12 +747,12 @@ read_unlock_bh(&hci_dev_list_lock); dl->dev_num = n; - size = n * sizeof(*dr) + sizeof(*dl); + size = sizeof(*dl) + n * sizeof(*dr); - 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) diff -Nru a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c --- a/net/bluetooth/rfcomm/tty.c Wed Feb 11 19:39:20 2004 +++ b/net/bluetooth/rfcomm/tty.c Wed Feb 11 19:39:20 2004 @@ -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(""); @@ -360,10 +360,10 @@ if (!dev_num) return -EINVAL; - size = sizeof(*dl) + dev_num * sizeof(*di); + if (dev_num * sizeof(*di) > PAGE_SIZE * 4) + return -EINVAL; - if (verify_area(VERIFY_WRITE, (void *)arg, size)) - return -EFAULT; + size = sizeof(*dl) + dev_num * sizeof(*di); if (!(dl = kmalloc(size, GFP_KERNEL))) return -ENOMEM; @@ -389,9 +389,10 @@ 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); kfree(dl); - return 0; + + return err ? -EFAULT : 0; } static int rfcomm_get_dev_info(unsigned long arg) @@ -563,8 +564,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); @@ -588,6 +591,9 @@ } set_current_state(TASK_RUNNING); remove_wait_queue(&dev->wait, &wait); + + if (err < 0) + rfcomm_dev_put(dev); return err; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: some bluetooth fixes 2004-02-11 18:55 ` [Bluez-devel] " Marcel Holtmann @ 2004-02-14 23:25 ` Andi Kleen 2004-02-11 20:47 ` [Bluez-devel] " Marcel Holtmann 0 siblings, 1 reply; 10+ messages in thread From: Andi Kleen @ 2004-02-14 23:25 UTC (permalink / raw) To: Marcel Holtmann; +Cc: bluez-devel, netdev, viro On Wed, 11 Feb 2004 19:55:43 +0100 Marcel Holtmann <marcel@holtmann.org> wrote: > Hi Andi, > > > Doing size checks after the multiply is too late - they could > > have already overflowed. You have to check the raw value from the user. > > new patch is attached. + if (req.conn_num * sizeof(*ci) > PAGE_SIZE * 2) + return -EINVAL; This can still overflow. It should be if (req.conn_num > (PAGE_SIZE * 2)/sizeof(*ci)) return -EINVAL -Andi ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bluez-devel] Re: some bluetooth fixes 2004-02-14 23:25 ` Andi Kleen @ 2004-02-11 20:47 ` Marcel Holtmann 0 siblings, 0 replies; 10+ messages in thread From: Marcel Holtmann @ 2004-02-11 20:47 UTC (permalink / raw) To: Andi Kleen; +Cc: BlueZ Mailing List, netdev, viro [-- Attachment #1: Type: text/plain, Size: 346 bytes --] Hi Andi, > + if (req.conn_num * sizeof(*ci) > PAGE_SIZE * 2) > + return -EINVAL; > > This can still overflow. It should be > > if (req.conn_num > (PAGE_SIZE * 2)/sizeof(*ci)) > return -EINVAL thanks for reviewing the patch again. The fixed version is only attached for control. It goes out to Dave in the next minutes. Regards Marcel [-- Attachment #2: patch --] [-- Type: text/plain, Size: 4516 bytes --] ===== net/bluetooth/hci_conn.c 1.9 vs edited ===== --- 1.9/net/bluetooth/hci_conn.c Tue Jan 13 03:50:09 2004 +++ edited/net/bluetooth/hci_conn.c Wed Feb 11 21:00:52 2004 @@ -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; - - size = req.conn_num * sizeof(struct hci_conn_info) + sizeof(req); + if (!req.conn_num || req.conn_num > (PAGE_SIZE * 2) / sizeof(*ci)) + return -EINVAL; - if (verify_area(VERIFY_WRITE, (void *)arg, size)) - return -EFAULT; + size = sizeof(req) + req.conn_num * sizeof(*ci); 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); @@ -381,20 +384,21 @@ (ci + n)->out = c->out; (ci + n)->state = c->state; (ci + n)->link_mode = c->link_mode; - n++; + if (++n >= req.conn_num) + break; } hci_dev_unlock_bh(hdev); cl->dev_id = hdev->id; cl->conn_num = n; - size = n * sizeof(struct hci_conn_info) + sizeof(req); + size = sizeof(req) + n * sizeof(*ci); hci_dev_put(hdev); - copy_to_user((void *) arg, cl, size); + err = copy_to_user((void *) arg, cl, size); kfree(cl); - return 0; + return err ? -EFAULT : 0; } int hci_get_conn_info(struct hci_dev *hdev, unsigned long arg) @@ -407,9 +411,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 +426,5 @@ if (!conn) return -ENOENT; - copy_to_user(ptr, &ci, sizeof(ci)); - return 0; + return copy_to_user(ptr, &ci, sizeof(ci)) ? -EFAULT : 0; } ===== net/bluetooth/hci_core.c 1.22 vs edited ===== --- 1.22/net/bluetooth/hci_core.c Thu Feb 5 13:07:36 2004 +++ edited/net/bluetooth/hci_core.c Wed Feb 11 20:59:03 2004 @@ -716,22 +716,20 @@ struct hci_dev_list_req *dl; struct hci_dev_req *dr; struct list_head *p; - int n = 0, size; + int n = 0, size, err; __u16 dev_num; if (get_user(dev_num, (__u16 *) arg)) return -EFAULT; - if (!dev_num) + if (!dev_num || dev_num > (PAGE_SIZE * 2) / sizeof(*dr)) return -EINVAL; - - size = dev_num * sizeof(*dr) + sizeof(*dl); - if (verify_area(VERIFY_WRITE, (void *) arg, size)) - return -EFAULT; + size = sizeof(*dl) + dev_num * sizeof(*dr); if (!(dl = kmalloc(size, GFP_KERNEL))) return -ENOMEM; + dr = dl->dev_req; read_lock_bh(&hci_dev_list_lock); @@ -746,12 +744,12 @@ read_unlock_bh(&hci_dev_list_lock); dl->dev_num = n; - size = n * sizeof(*dr) + sizeof(*dl); + size = sizeof(*dl) + n * sizeof(*dr); - 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) ===== net/bluetooth/rfcomm/tty.c 1.28 vs edited ===== --- 1.28/net/bluetooth/rfcomm/tty.c Thu Aug 7 02:24:51 2003 +++ edited/net/bluetooth/rfcomm/tty.c Wed Feb 11 21:01:25 2004 @@ -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(""); @@ -357,14 +357,11 @@ if (get_user(dev_num, (u16 *) arg)) return -EFAULT; - if (!dev_num) + if (!dev_num || dev_num > (PAGE_SIZE * 4) / sizeof(*di)) return -EINVAL; size = sizeof(*dl) + dev_num * sizeof(*di); - if (verify_area(VERIFY_WRITE, (void *)arg, size)) - return -EFAULT; - if (!(dl = kmalloc(size, GFP_KERNEL))) return -ENOMEM; @@ -389,9 +386,10 @@ 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); kfree(dl); - return 0; + + return err ? -EFAULT : 0; } static int rfcomm_get_dev_info(unsigned long arg) @@ -563,8 +561,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); @@ -588,6 +588,9 @@ } set_current_state(TASK_RUNNING); remove_wait_queue(&dev->wait, &wait); + + if (err < 0) + rfcomm_dev_put(dev); return err; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bluez-devel] Re: some bluetooth fixes [not found] <20040206050042.20a2b3b0.ak@suse.de> 2004-02-06 14:58 ` [Bluez-devel] Re: some bluetooth fixes Marcel Holtmann @ 2004-02-06 23:30 ` Marcel Holtmann 2004-02-06 23:34 ` David S. Miller 1 sibling, 1 reply; 10+ messages in thread From: Marcel Holtmann @ 2004-02-06 23:30 UTC (permalink / raw) To: Andi Kleen; +Cc: bluez-devel, netdev, viro Hi 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 > @@ -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; this part is wrong, because it is not an error case. It is success and the refcount must stay increased. Regards Marcel ------------------------------------------------------- The SF.Net email is sponsored by EclipseCon 2004 Premiere Conference on Open Tools Development and Integration See the breadth of Eclipse activity. February 3-5 in Anaheim, CA. http://www.eclipsecon.org/osdn _______________________________________________ Bluez-devel mailing list Bluez-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bluez-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: some bluetooth fixes 2004-02-06 23:30 ` Marcel Holtmann @ 2004-02-06 23:34 ` David S. Miller 2004-02-06 23:46 ` [Bluez-devel] " Marcel Holtmann 0 siblings, 1 reply; 10+ messages in thread From: David S. Miller @ 2004-02-06 23:34 UTC (permalink / raw) To: Marcel Holtmann; +Cc: ak, bluez-devel, netdev, viro Marcelo/Andi, I assume you guys will work things out and send me a final patch? Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bluez-devel] Re: some bluetooth fixes 2004-02-06 23:34 ` David S. Miller @ 2004-02-06 23:46 ` Marcel Holtmann 0 siblings, 0 replies; 10+ messages in thread From: Marcel Holtmann @ 2004-02-06 23:46 UTC (permalink / raw) To: David S. Miller; +Cc: ak, BlueZ Mailing List, netdev, viro Hi Dave, > Marcelo/Andi, I assume you guys will work things out and send > me a final patch? I already splitted Andi's patch into smaller parts and I wait for the answers to my questions. I also have some other fixes in the queue, so you will get a number of patches ;) Regards Marcel ------------------------------------------------------- The SF.Net email is sponsored by EclipseCon 2004 Premiere Conference on Open Tools Development and Integration See the breadth of Eclipse activity. February 3-5 in Anaheim, CA. http://www.eclipsecon.org/osdn _______________________________________________ Bluez-devel mailing list Bluez-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bluez-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2004-02-14 23:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20040206050042.20a2b3b0.ak@suse.de>
2004-02-06 14:58 ` [Bluez-devel] Re: some bluetooth fixes Marcel Holtmann
[not found] ` <20040207032428.56ffbebc.ak@suse.de>
2004-02-07 11:13 ` Marcel Holtmann
[not found] ` <20040207125723.391a1fcd.ak@suse.de>
2004-02-07 16:57 ` Marcel Holtmann
2004-02-07 17:24 ` Andi Kleen
2004-02-11 18:55 ` [Bluez-devel] " Marcel Holtmann
2004-02-14 23:25 ` Andi Kleen
2004-02-11 20:47 ` [Bluez-devel] " Marcel Holtmann
2004-02-06 23:30 ` Marcel Holtmann
2004-02-06 23:34 ` David S. Miller
2004-02-06 23:46 ` [Bluez-devel] " Marcel Holtmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox