Discussion:
[PATCH 2/4] carl9170: fix debugfs crashes
Christian Lamparter
2016-09-17 19:43:02 UTC
Permalink
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 <***@candelatech.com>
Reviewed-by: Nicolai Stange <***@gmail.com>
Signed-off-by: Christian Lamparter <***@gmail.com>
---
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;
--
2.9.3
Christian Lamparter
2016-09-17 19:43:04 UTC
Permalink
This patch fixes a crash that happens because b43legacy's
debugfs code expects file->f_op to be a pointer to its own
b43legacy_debugfs_fops struct. This is no longer the case
since commit 9fd4dcece43a
("debugfs: prevent access to possibly dead file_operations at file open")

Reviewed-by: Nicolai Stange <***@gmail.com>
Signed-off-by: Christian Lamparter <***@gmail.com>
---
drivers/net/wireless/broadcom/b43legacy/debugfs.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/debugfs.c b/drivers/net/wireless/broadcom/b43legacy/debugfs.c
index 090910e..82ef56e 100644
--- a/drivers/net/wireless/broadcom/b43legacy/debugfs.c
+++ b/drivers/net/wireless/broadcom/b43legacy/debugfs.c
@@ -221,7 +221,8 @@ static ssize_t b43legacy_debugfs_read(struct file *file, char __user *userbuf,
goto out_unlock;
}

- dfops = container_of(file->f_op, struct b43legacy_debugfs_fops, fops);
+ dfops = container_of(debugfs_real_fops(file),
+ struct b43legacy_debugfs_fops, fops);
if (!dfops->read) {
err = -ENOSYS;
goto out_unlock;
@@ -287,7 +288,8 @@ static ssize_t b43legacy_debugfs_write(struct file *file,
goto out_unlock;
}

- dfops = container_of(file->f_op, struct b43legacy_debugfs_fops, fops);
+ dfops = container_of(debugfs_real_fops(file),
+ struct b43legacy_debugfs_fops, fops);
if (!dfops->write) {
err = -ENOSYS;
goto out_unlock;
--
2.9.3
Christian Lamparter
2016-09-17 19:43:03 UTC
Permalink
This patch fixes a crash that happens because b43's
debugfs code expects file->f_op to be a pointer to
its own b43_debugfs_fops struct. This is no longer
the case since commit 9fd4dcece43a
("debugfs: prevent access to possibly dead file_operations at file open")

Reviewed-by: Nicolai Stange <***@gmail.com>
Signed-off-by: Christian Lamparter <***@gmail.com>
---
drivers/net/wireless/broadcom/b43/debugfs.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/debugfs.c b/drivers/net/wireless/broadcom/b43/debugfs.c
index b4bcd94..7704638 100644
--- a/drivers/net/wireless/broadcom/b43/debugfs.c
+++ b/drivers/net/wireless/broadcom/b43/debugfs.c
@@ -524,7 +524,8 @@ static ssize_t b43_debugfs_read(struct file *file, char __user *userbuf,
goto out_unlock;
}

- dfops = container_of(file->f_op, struct b43_debugfs_fops, fops);
+ dfops = container_of(debugfs_real_fops(file),
+ struct b43_debugfs_fops, fops);
if (!dfops->read) {
err = -ENOSYS;
goto out_unlock;
@@ -585,7 +586,8 @@ static ssize_t b43_debugfs_write(struct file *file,
goto out_unlock;
}

- dfops = container_of(file->f_op, struct b43_debugfs_fops, fops);
+ dfops = container_of(debugfs_real_fops(file),
+ struct b43_debugfs_fops, fops);
if (!dfops->write) {
err = -ENOSYS;
goto out_unlock;
--
2.9.3
Greg KH
2016-09-17 21:45:39 UTC
Permalink
Post by Christian Lamparter
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
Kalle Valo
2016-09-18 07:54:18 UTC
Permalink
Post by Greg KH
Post by Christian Lamparter
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?
--
Kalle Valo
Greg KH
2016-09-18 10:14:55 UTC
Permalink
Post by Kalle Valo
Post by Greg KH
Post by Christian Lamparter
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
Christian Lamparter
2016-09-18 12:49:33 UTC
Permalink
Post by Greg KH
Post by Kalle Valo
Post by Greg KH
Post by Christian Lamparter
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?
Post by Greg KH
Post by Kalle Valo
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
Greg KH
2016-09-18 16:44:08 UTC
Permalink
Post by Christian Lamparter
Post by Greg KH
Post by Kalle Valo
Post by Greg KH
Post by Christian Lamparter
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.
Post by Christian Lamparter
Post by Greg KH
Post by Kalle Valo
Via which tree should these go, Greg's or mine?
I'll take it if you ack it, as it's a debugfs issue.
"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
Christian Lamparter
2016-09-19 20:12:08 UTC
Permalink
Post by Greg KH
Post by Christian Lamparter
Post by Greg KH
Post by Kalle Valo
Post by Greg KH
Post by Christian Lamparter
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.
Post by Christian Lamparter
Post by Greg KH
Post by Kalle Valo
Via which tree should these go, Greg's or mine?
I'll take it if you ack it, as it's a debugfs issue.
"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
Greg KH
2016-09-20 06:50:53 UTC
Permalink
Post by Christian Lamparter
Post by Greg KH
Post by Christian Lamparter
Post by Greg KH
Post by Kalle Valo
Post by Greg KH
Post by Christian Lamparter
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.
Post by Christian Lamparter
Post by Greg KH
Post by Kalle Valo
Via which tree should these go, Greg's or mine?
I'll take it if you ack it, as it's a debugfs issue.
"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:"? :)
Kalle Valo
2016-09-18 16:57:57 UTC
Permalink
Post by Greg KH
Post by Kalle Valo
Post by Greg KH
Post by Christian Lamparter
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 <***@codeaurora.org>
--
Kalle Valo
Greg KH
2016-09-21 10:13:25 UTC
Permalink
Post by Christian Lamparter
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.
---
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
Christian Lamparter
2016-09-21 16:29:26 UTC
Permalink
Post by Greg KH
Post by Christian Lamparter
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.
---
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

Loading...