2010-03-06 11:15:52

by Dan Carpenter

[permalink] [raw]
Subject: [patch] bluetooth: debugfs changes use too much stack

The original code would break with a 4K stack.

Signed-off-by: Dan Carpenter <[email protected]>
---
This was compile tested only. Sorry about that.

diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 1a79a6c..835758f 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -417,9 +417,11 @@ static ssize_t inquiry_cache_read(struct file *file, char __user *userbuf,
struct hci_dev *hdev = file->private_data;
struct inquiry_cache *cache = &hdev->inq_cache;
struct inquiry_entry *e;
- char buf[4096];
+ char *buf;
int n = 0;
+ ssize_t ret;

+ buf = kmalloc(4096, GFP_KERNEL);
hci_dev_lock_bh(hdev);

for (e = cache->list; e; e = e->next) {
@@ -437,7 +439,10 @@ static ssize_t inquiry_cache_read(struct file *file, char __user *userbuf,

hci_dev_unlock_bh(hdev);

- return simple_read_from_buffer(userbuf, count, ppos, buf, n);
+ ret = simple_read_from_buffer(userbuf, count, ppos, buf, n);
+ kfree(buf);
+
+ return ret;
}

static const struct file_operations inquiry_cache_fops = {


2010-03-06 17:49:43

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [patch] bluetooth: debugfs changes use too much stack

Hi Dan,

> > Error handling?
> >
> > > The original code would break with a 4K stack.
> > >
> > > Signed-off-by: Dan Carpenter <[email protected]>
> > > ---
> > > This was compile tested only. Sorry about that.
> > >
> > > diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> > > index 1a79a6c..835758f 100644
> > > --- a/net/bluetooth/hci_sysfs.c
> > > +++ b/net/bluetooth/hci_sysfs.c
> > > @@ -417,9 +417,11 @@ static ssize_t inquiry_cache_read(struct file *file, char __user *userbuf,
> > > struct hci_dev *hdev = file->private_data;
> > > struct inquiry_cache *cache = &hdev->inq_cache;
> > > struct inquiry_entry *e;
> > > - char buf[4096];
> > > + char *buf;
> > > int n = 0;
> > > + ssize_t ret;
> > >
> > > + buf = kmalloc(4096, GFP_KERNEL);
> >
> > Could this kmalloc not fail?
>
> Grr... I'm really sorry about that.
>
> I will send an updated patch tomorrow.

please don't since we fixed this already in the net-2.6 tree.

Regards

Marcel



2010-03-06 15:30:30

by Dan Carpenter

[permalink] [raw]
Subject: Re: [patch] bluetooth: debugfs changes use too much stack

On Sat, Mar 06, 2010 at 03:40:31PM +0100, Jan Ceuleers wrote:
> Dan Carpenter wrote:
>
> Error handling?
>
> > The original code would break with a 4K stack.
> >
> > Signed-off-by: Dan Carpenter <[email protected]>
> > ---
> > This was compile tested only. Sorry about that.
> >
> > diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> > index 1a79a6c..835758f 100644
> > --- a/net/bluetooth/hci_sysfs.c
> > +++ b/net/bluetooth/hci_sysfs.c
> > @@ -417,9 +417,11 @@ static ssize_t inquiry_cache_read(struct file *file, char __user *userbuf,
> > struct hci_dev *hdev = file->private_data;
> > struct inquiry_cache *cache = &hdev->inq_cache;
> > struct inquiry_entry *e;
> > - char buf[4096];
> > + char *buf;
> > int n = 0;
> > + ssize_t ret;
> >
> > + buf = kmalloc(4096, GFP_KERNEL);
>
> Could this kmalloc not fail?

Grr... I'm really sorry about that.

I will send an updated patch tomorrow.

regards,
dan carpenter

2010-03-06 14:40:31

by Jan Ceuleers

[permalink] [raw]
Subject: Re: [patch] bluetooth: debugfs changes use too much stack

Dan Carpenter wrote:

Error handling?

> The original code would break with a 4K stack.
>
> Signed-off-by: Dan Carpenter <[email protected]>
> ---
> This was compile tested only. Sorry about that.
>
> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 1a79a6c..835758f 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -417,9 +417,11 @@ static ssize_t inquiry_cache_read(struct file *file, char __user *userbuf,
> struct hci_dev *hdev = file->private_data;
> struct inquiry_cache *cache = &hdev->inq_cache;
> struct inquiry_entry *e;
> - char buf[4096];
> + char *buf;
> int n = 0;
> + ssize_t ret;
>
> + buf = kmalloc(4096, GFP_KERNEL);

Could this kmalloc not fail?