| Message ID | ccf228acb7af8ee3bbd82f72f28ebc068f37cb8e.1474140477.git.chunkeey@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote: > Ben Greear reported: > > I see lots of instability as soon as I load up the carl9710 NIC. > > My application is going to be poking at it's debugfs files... > > > > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0 > > [carl9170] at addr ffff8801bc1208b0 > > Read of size 8 by task btserver/5888 > > ======================================================================= > > BUG kmalloc-256 (Tainted: G W ): kasan: bad access detected > > ----------------------------------------------------------------------- > > > > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772 > >... > > This breakage was caused by the introduction of intermediate > fops in debugfs by commit 9fd4dcece43a > ("debugfs: prevent access to possibly dead file_operations at file open") Because of this, these should all be backported to 4.7-stable, and 4.8-stable, right? thanks, greg k-h
Greg KH <gregkh@linuxfoundation.org> writes: > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote: >> Ben Greear reported: >> > I see lots of instability as soon as I load up the carl9710 NIC. >> > My application is going to be poking at it's debugfs files... >> > >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0 >> > [carl9170] at addr ffff8801bc1208b0 >> > Read of size 8 by task btserver/5888 >> > ======================================================================= >> > BUG kmalloc-256 (Tainted: G W ): kasan: bad access detected >> > ----------------------------------------------------------------------- >> > >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772 >> >... >> >> This breakage was caused by the introduction of intermediate >> fops in debugfs by commit 9fd4dcece43a >> ("debugfs: prevent access to possibly dead file_operations at file open") > > Because of this, these should all be backported to 4.7-stable, and > 4.8-stable, right? Via which tree should these go, Greg's or mine?
On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote: > Greg KH <gregkh@linuxfoundation.org> writes: > > > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote: > >> Ben Greear reported: > >> > I see lots of instability as soon as I load up the carl9710 NIC. > >> > My application is going to be poking at it's debugfs files... > >> > > >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0 > >> > [carl9170] at addr ffff8801bc1208b0 > >> > Read of size 8 by task btserver/5888 > >> > ======================================================================= > >> > BUG kmalloc-256 (Tainted: G W ): kasan: bad access detected > >> > ----------------------------------------------------------------------- > >> > > >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772 > >> >... > >> > >> This breakage was caused by the introduction of intermediate > >> fops in debugfs by commit 9fd4dcece43a > >> ("debugfs: prevent access to possibly dead file_operations at file open") > > > > Because of this, these should all be backported to 4.7-stable, and > > 4.8-stable, right? > > Via which tree should these go, Greg's or mine? I'll take it if you ack it, as it's a debugfs issue. thanks, greg k-h
On Sunday, September 18, 2016 12:14:55 PM CEST Greg KH wrote: > On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote: > > Greg KH <gregkh@linuxfoundation.org> writes: > > > > > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote: > > >> Ben Greear reported: > > >> > I see lots of instability as soon as I load up the carl9710 NIC. > > >> > My application is going to be poking at it's debugfs files... > > >> > > > >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0 > > >> > [carl9170] at addr ffff8801bc1208b0 > > >> > Read of size 8 by task btserver/5888 > > >> > ======================================================================= > > >> > BUG kmalloc-256 (Tainted: G W ): kasan: bad access detected > > >> > ----------------------------------------------------------------------- > > >> > > > >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772 > > >> >... > > >> > > >> This breakage was caused by the introduction of intermediate > > >> fops in debugfs by commit 9fd4dcece43a > > >> ("debugfs: prevent access to possibly dead file_operations at file open") > > > > > > Because of this, these should all be backported to 4.7-stable, and > > > 4.8-stable, right? Ok, only b43legacy has debugfs enabled by default. For b43 and carl9170 debugfs support is usually disabled. Greg, would you take these four patches "as is" for -stable or do you want a "minimal version" which just replaces the dfops = container_of(file->f_op, ... with dfops = container_of(file->f_path.dentry->d_fsdata, ... in the three drivers for -stable? > > Via which tree should these go, Greg's or mine? > > I'll take it if you ack it, as it's a debugfs issue. For carl9170: Ben Greear has reported: "I have verified this fixes my problem in the 4.7 kernel." But this was with a preliminary/minimal version so I didn't add the tested-by tag. As for b43, I'll see if I have a working b43 in my collection somewhere to confirm the issue and the fix. Question is, do you want to wait or not? Regards, Christian
On Sun, Sep 18, 2016 at 02:49:33PM +0200, Christian Lamparter wrote: > On Sunday, September 18, 2016 12:14:55 PM CEST Greg KH wrote: > > On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote: > > > Greg KH <gregkh@linuxfoundation.org> writes: > > > > > > > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote: > > > >> Ben Greear reported: > > > >> > I see lots of instability as soon as I load up the carl9710 NIC. > > > >> > My application is going to be poking at it's debugfs files... > > > >> > > > > >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0 > > > >> > [carl9170] at addr ffff8801bc1208b0 > > > >> > Read of size 8 by task btserver/5888 > > > >> > ======================================================================= > > > >> > BUG kmalloc-256 (Tainted: G W ): kasan: bad access detected > > > >> > ----------------------------------------------------------------------- > > > >> > > > > >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772 > > > >> >... > > > >> > > > >> This breakage was caused by the introduction of intermediate > > > >> fops in debugfs by commit 9fd4dcece43a > > > >> ("debugfs: prevent access to possibly dead file_operations at file open") > > > > > > > > Because of this, these should all be backported to 4.7-stable, and > > > > 4.8-stable, right? > Ok, only b43legacy has debugfs enabled by default. For b43 and carl9170 > debugfs support is usually disabled. > > Greg, would you take these four patches "as is" for -stable > or do you want a "minimal version" which just replaces the > > dfops = container_of(file->f_op, ... > > with > > dfops = container_of(file->f_path.dentry->d_fsdata, ... > > in the three drivers for -stable? No, I'll take this as is, we want things to remain as close as possible to Linus's tree. When we are not, is when things break. > > > Via which tree should these go, Greg's or mine? > > > > I'll take it if you ack it, as it's a debugfs issue. > For carl9170: Ben Greear has reported: > "I have verified this fixes my problem in the 4.7 kernel." > > But this was with a preliminary/minimal version so I didn't > add the tested-by tag. > > As for b43, I'll see if I have a working b43 in my collection > somewhere to confirm the issue and the fix. Question is, do > you want to wait or not? I'll queue these up this week, no rush. thanks, greg k-h
Greg KH <gregkh@linuxfoundation.org> writes: > On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote: >> Greg KH <gregkh@linuxfoundation.org> writes: >> >> > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote: >> >> Ben Greear reported: >> >> > I see lots of instability as soon as I load up the carl9710 NIC. >> >> > My application is going to be poking at it's debugfs files... >> >> > >> >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0 >> >> > [carl9170] at addr ffff8801bc1208b0 >> >> > Read of size 8 by task btserver/5888 >> >> > ======================================================================= >> >> > BUG kmalloc-256 (Tainted: G W ): kasan: bad access detected >> >> > ----------------------------------------------------------------------- >> >> > >> >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772 >> >> >... >> >> >> >> This breakage was caused by the introduction of intermediate >> >> fops in debugfs by commit 9fd4dcece43a >> >> ("debugfs: prevent access to possibly dead file_operations at file open") >> > >> > Because of this, these should all be backported to 4.7-stable, and >> > 4.8-stable, right? >> >> Via which tree should these go, Greg's or mine? > > I'll take it if you ack it, as it's a debugfs issue. Good, thanks. The wireless patches look good to me so: Acked-by: Kalle Valo <kvalo@codeaurora.org>
On Sunday, September 18, 2016 6:44:08 PM CEST Greg KH wrote: > On Sun, Sep 18, 2016 at 02:49:33PM +0200, Christian Lamparter wrote: > > On Sunday, September 18, 2016 12:14:55 PM CEST Greg KH wrote: > > > On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote: > > > > Greg KH <gregkh@linuxfoundation.org> writes: > > > > > > > > > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote: > > > > >> Ben Greear reported: > > > > >> > I see lots of instability as soon as I load up the carl9710 NIC. > > > > >> > My application is going to be poking at it's debugfs files... > > > > >> > > > > > >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0 > > > > >> > [carl9170] at addr ffff8801bc1208b0 > > > > >> > Read of size 8 by task btserver/5888 > > > > >> > ======================================================================= > > > > >> > BUG kmalloc-256 (Tainted: G W ): kasan: bad access detected > > > > >> > ----------------------------------------------------------------------- > > > > >> > > > > > >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772 > > > > >> >... > > > > >> > > > > >> This breakage was caused by the introduction of intermediate > > > > >> fops in debugfs by commit 9fd4dcece43a > > > > >> ("debugfs: prevent access to possibly dead file_operations at file open") > > > > > > > > > > Because of this, these should all be backported to 4.7-stable, and > > > > > 4.8-stable, right? > > Ok, only b43legacy has debugfs enabled by default. For b43 and carl9170 > > debugfs support is usually disabled. > > > > Greg, would you take these four patches "as is" for -stable > > or do you want a "minimal version" which just replaces the > > > > dfops = container_of(file->f_op, ... > > > > with > > > > dfops = container_of(file->f_path.dentry->d_fsdata, ... > > > > in the three drivers for -stable? > > No, I'll take this as is, we want things to remain as close as possible > to Linus's tree. When we are not, is when things break. > > > > > Via which tree should these go, Greg's or mine? > > > > > > I'll take it if you ack it, as it's a debugfs issue. > > For carl9170: Ben Greear has reported: > > "I have verified this fixes my problem in the 4.7 kernel." > > > > But this was with a preliminary/minimal version so I didn't > > add the tested-by tag. > > > > As for b43, I'll see if I have a working b43 in my collection > > somewhere to confirm the issue and the fix. Question is, do > > you want to wait or not? > > I'll queue these up this week, no rush. I was able to sucessfully test the b43 patch on my iBook G4's BCM4306. Thanks, Christian
On Mon, Sep 19, 2016 at 10:12:08PM +0200, Christian Lamparter wrote: > On Sunday, September 18, 2016 6:44:08 PM CEST Greg KH wrote: > > On Sun, Sep 18, 2016 at 02:49:33PM +0200, Christian Lamparter wrote: > > > On Sunday, September 18, 2016 12:14:55 PM CEST Greg KH wrote: > > > > On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote: > > > > > Greg KH <gregkh@linuxfoundation.org> writes: > > > > > > > > > > > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote: > > > > > >> Ben Greear reported: > > > > > >> > I see lots of instability as soon as I load up the carl9710 NIC. > > > > > >> > My application is going to be poking at it's debugfs files... > > > > > >> > > > > > > >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0 > > > > > >> > [carl9170] at addr ffff8801bc1208b0 > > > > > >> > Read of size 8 by task btserver/5888 > > > > > >> > ======================================================================= > > > > > >> > BUG kmalloc-256 (Tainted: G W ): kasan: bad access detected > > > > > >> > ----------------------------------------------------------------------- > > > > > >> > > > > > > >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772 > > > > > >> >... > > > > > >> > > > > > >> This breakage was caused by the introduction of intermediate > > > > > >> fops in debugfs by commit 9fd4dcece43a > > > > > >> ("debugfs: prevent access to possibly dead file_operations at file open") > > > > > > > > > > > > Because of this, these should all be backported to 4.7-stable, and > > > > > > 4.8-stable, right? > > > Ok, only b43legacy has debugfs enabled by default. For b43 and carl9170 > > > debugfs support is usually disabled. > > > > > > Greg, would you take these four patches "as is" for -stable > > > or do you want a "minimal version" which just replaces the > > > > > > dfops = container_of(file->f_op, ... > > > > > > with > > > > > > dfops = container_of(file->f_path.dentry->d_fsdata, ... > > > > > > in the three drivers for -stable? > > > > No, I'll take this as is, we want things to remain as close as possible > > to Linus's tree. When we are not, is when things break. > > > > > > > Via which tree should these go, Greg's or mine? > > > > > > > > I'll take it if you ack it, as it's a debugfs issue. > > > For carl9170: Ben Greear has reported: > > > "I have verified this fixes my problem in the 4.7 kernel." > > > > > > But this was with a preliminary/minimal version so I didn't > > > add the tested-by tag. > > > > > > As for b43, I'll see if I have a working b43 in my collection > > > somewhere to confirm the issue and the fix. Question is, do > > > you want to wait or not? > > > > I'll queue these up this week, no rush. > > I was able to sucessfully test the b43 patch on my iBook G4's BCM4306. So is that a "Tested-by:"? :)
On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote: > Ben Greear reported: > > I see lots of instability as soon as I load up the carl9710 NIC. > > My application is going to be poking at it's debugfs files... > > > > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0 > > [carl9170] at addr ffff8801bc1208b0 > > Read of size 8 by task btserver/5888 > > ======================================================================= > > BUG kmalloc-256 (Tainted: G W ): kasan: bad access detected > > ----------------------------------------------------------------------- > > > > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772 > >... > > This breakage was caused by the introduction of intermediate > fops in debugfs by commit 9fd4dcece43a > ("debugfs: prevent access to possibly dead file_operations at file open") > > Thankfully, the original/real fops are still available in d_fsdata. > > Reported-by: Ben Greear <greearb@candelatech.com> > Reviewed-by: Nicolai Stange <nicstange@gmail.com> > Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > Acked-by: Kalle Valo <kvalo@codeaurora.org> > Cc: stable <stable@vger.kernel.org> # 4.7+ > --- > drivers/net/wireless/ath/carl9170/debug.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ath/carl9170/debug.c b/drivers/net/wireless/ath/carl9170/debug.c > index 01a0919..ad7ffd5 100644 > --- a/drivers/net/wireless/ath/carl9170/debug.c > +++ b/drivers/net/wireless/ath/carl9170/debug.c > @@ -75,7 +75,7 @@ static ssize_t carl9170_debugfs_read(struct file *file, char __user *userbuf, > > if (!ar) > return -ENODEV; > - dfops = container_of(file->f_path.dentry->d_fsdata, > + dfops = container_of(debugfs_real_fops(file), > struct carl9170_debugfs_fops, fops); > > if (!dfops->read) > @@ -128,7 +128,7 @@ static ssize_t carl9170_debugfs_write(struct file *file, > > if (!ar) > return -ENODEV; > - dfops = container_of(file->f_path.dentry->d_fsdata, > + dfops = container_of(debugfs_real_fops(file), > struct carl9170_debugfs_fops, fops); > if (!dfops->write) > return -ENOSYS; What tree is this against? I can't apply it to 4.8-rc5, or 4.8-rc7, are you sure it is still needed? thanks, greg k-h
On Wednesday, September 21, 2016 12:13:25 PM CEST Greg KH wrote: > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote: > > Ben Greear reported: > > > I see lots of instability as soon as I load up the carl9710 NIC. > > > My application is going to be poking at it's debugfs files... > > > > > > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0 > > > [carl9170] at addr ffff8801bc1208b0 > > > Read of size 8 by task btserver/5888 > > > ======================================================================= > > > BUG kmalloc-256 (Tainted: G W ): kasan: bad access detected > > > ----------------------------------------------------------------------- > > > > > > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772 > > >... > > > > This breakage was caused by the introduction of intermediate > > fops in debugfs by commit 9fd4dcece43a > > ("debugfs: prevent access to possibly dead file_operations at file open") > > > > Thankfully, the original/real fops are still available in d_fsdata. > > > > Reported-by: Ben Greear <greearb@candelatech.com> > > Reviewed-by: Nicolai Stange <nicstange@gmail.com> > > Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > > Acked-by: Kalle Valo <kvalo@codeaurora.org> > > Cc: stable <stable@vger.kernel.org> # 4.7+ > > --- > > drivers/net/wireless/ath/carl9170/debug.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/wireless/ath/carl9170/debug.c b/drivers/net/wireless/ath/carl9170/debug.c > > index 01a0919..ad7ffd5 100644 > > --- a/drivers/net/wireless/ath/carl9170/debug.c > > +++ b/drivers/net/wireless/ath/carl9170/debug.c > > @@ -75,7 +75,7 @@ static ssize_t carl9170_debugfs_read(struct file *file, char __user *userbuf, > > > > if (!ar) > > return -ENODEV; > > - dfops = container_of(file->f_path.dentry->d_fsdata, > > + dfops = container_of(debugfs_real_fops(file), > > struct carl9170_debugfs_fops, fops); > > > > if (!dfops->read) > > @@ -128,7 +128,7 @@ static ssize_t carl9170_debugfs_write(struct file *file, > > > > if (!ar) > > return -ENODEV; > > - dfops = container_of(file->f_path.dentry->d_fsdata, > > + dfops = container_of(debugfs_real_fops(file), > > struct carl9170_debugfs_fops, fops); > > if (!dfops->write) > > return -ENOSYS; > > What tree is this against? I can't apply it to 4.8-rc5, or 4.8-rc7, are > you sure it is still needed? --- Yes, the patch is needed. That said I screwed this patch up and as a result it is faulty. I'll send out v2 shortly Thanks, Christian
diff --git a/drivers/net/wireless/ath/carl9170/debug.c b/drivers/net/wireless/ath/carl9170/debug.c index 01a0919..ad7ffd5 100644 --- a/drivers/net/wireless/ath/carl9170/debug.c +++ b/drivers/net/wireless/ath/carl9170/debug.c @@ -75,7 +75,7 @@ static ssize_t carl9170_debugfs_read(struct file *file, char __user *userbuf, if (!ar) return -ENODEV; - dfops = container_of(file->f_path.dentry->d_fsdata, + dfops = container_of(debugfs_real_fops(file), struct carl9170_debugfs_fops, fops); if (!dfops->read) @@ -128,7 +128,7 @@ static ssize_t carl9170_debugfs_write(struct file *file, if (!ar) return -ENODEV; - dfops = container_of(file->f_path.dentry->d_fsdata, + dfops = container_of(debugfs_real_fops(file), struct carl9170_debugfs_fops, fops); if (!dfops->write) return -ENOSYS;