brcmsmac: don't leak kernel memory via printk()
Subsystems (3)
PATCH 1/1 brcmsmac: don't leak kernel memory via printk()
Debug code prints the fifo name via custom dev_warn() wrappers. The fifo_names array is only non-zero when debugging is manually enabled, which is all well and good. However, it's *not* good that this array uses zero-length arrays in the non-debug case, and so it doesn't actually have any memory allocated to it. This means that as far as we know, fifo_names[i] actually points to garbage memory.
I've seen this in my log:
[ 4601.205511] brcmsmac bcma0:1: wl0: brcms_c_d11hdrs_mac80211: �GeL txop exceeded phylen 137/256 dur 1602/1504
So let's give this array space enough to fill it with a NULL byte.
--- drivers/net/wireless/brcm80211/brcmsmac/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/brcm80211/brcmsmac/main.c +1 -1
@@ -316,7 +316,7 @@ static const u16 xmtfifo_sz[][NFIFO] = {static const char *const fifo_names[] = {"AC_BK" ,"AC_BE" ,"AC_VI" ,"AC_VO" ,"BCMC" ,"ATIM" };#else -static const char fifo_names[6 ][0 ]; +static const char fifo_names[6 ][1 ];#endif #ifdef DEBUG
Signature · 1 line
2.2.0
Quoted 7 lines · Brian Norris
Debug code prints the fifo name via custom
dev_warn()wrappers. Thefifo_namesarray is only non-zero when debugging is manually enabled, which is all well and good. However, it's *not* good that this array uses zero-length arrays in the non-debug case, and so it doesn't actually have any memory allocated to it. This means that as far as we know, fifo_names[i] actually points to garbage memory.
Quoted 6 lines
I've seen this in my log:
[ 4601.205511] brcmsmac bcma0:1: wl0: brcms_c_d11hdrs_mac80211: �GeL txop exceeded phylen 137/256 dur 1602/1504So let's give this array space enough to fill it with a NULL byte.
+ Acked-by: Arend van Spriel <[email protected]>
Quoted 2 lines
Signed-off-by Brian Norris <[email protected]>Cc Brett Rudley <[email protected]>
- Cc: Arend van Spriel <[email protected]>
Quoted 24 lines
Cc "Franky (Zhenhui) Lin" <[email protected]>Cc Hante Meuleman <[email protected]>Cc "John W. Linville" <[email protected]>--- drivers/net/wireless/brcm80211/brcmsmac/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/wireless/brcm80211/brcmsmac/main.c +1
-1@@ -316,7 +316,7 @@ static const u16 xmtfifo_sz[][NFIFO] = {static const char *const fifo_names[] = {"AC_BK" ,"AC_BE" ,"AC_VI" ,"AC_VO" ,"BCMC" ,"ATIM" };#else -static const char fifo_names[6 ][0 ]; +static const char fifo_names[6 ][1 ];#endif #ifdef DEBUG
+ others [1]
Quoted 13 lines · Brian Norris
Debug code prints the fifo name via custom
dev_warn()wrappers. Thefifo_namesarray is only non-zero when debugging is manually enabled, which is all well and good. However, it's *not* good that this array uses zero-length arrays in the non-debug case, and so it doesn't actually have any memory allocated to it. This means that as far as we know, fifo_names[i] actually points to garbage memory.I've seen this in my log:
[ 4601.205511] brcmsmac bcma0:1: wl0: brcms_c_d11hdrs_mac80211: �GeL txop exceeded phylen 137/256 dur 1602/1504So let's give this array space enough to fill it with a NULL byte.
Quoted 9 lines
Signed-off-by Brian Norris <[email protected]>Cc Brett Rudley <[email protected]>Cc Arend van Spriel <[email protected]>Cc "Franky (Zhenhui) Lin" <[email protected]>Cc Hante Meuleman <[email protected]>Cc "John W. Linville" <[email protected]>
BTW, I guess this qualifies as a security hole, albeit a small one. Should this be CC: [email protected]?
Quoted 9 lines
--- drivers/net/wireless/brcm80211/brcmsmac/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/wireless/brcm80211/brcmsmac/main.c +0
-0
drivers/net/wireless/brcm80211/brcmsmac/main.c static const u16 xmtfifo_sz[][NFIFO] = { · line 322
Quoted 8 lines
drivers/net/wireless/brcm80211/brcmsmac/main.c +1
-1@@ -316,7 +316,7 @@ static const u16 xmtfifo_sz[][NFIFO] = {static const char *const fifo_names[] = {"AC_BK" ,"AC_BE" ,"AC_VI" ,"AC_VO" ,"BCMC" ,"ATIM" };#else -static const char fifo_names[6 ][0 ]; +static const char fifo_names[6 ][1 ];#endif #ifdef DEBUG
Brian
Quoted 27 lines · Brian Norris
+ others [1]
Quoted 13 lines · Brian Norris
Debug code prints the fifo name via custom
dev_warn()wrappers. Thefifo_namesarray is only non-zero when debugging is manually enabled, which is all well and good. However, it's *not* good that this array uses zero-length arrays in the non-debug case, and so it doesn't actually have any memory allocated to it. This means that as far as we know, fifo_names[i] actually points to garbage memory.I've seen this in my log:
[ 4601.205511] brcmsmac bcma0:1: wl0: brcms_c_d11hdrs_mac80211: �GeL txop exceeded phylen 137/256 dur 1602/1504So let's give this array space enough to fill it with a NULL byte.
Quoted 9 lines
Signed-off-by Brian Norris<[email protected]>Cc Brett Rudley<[email protected]>Cc Arend van Spriel<[email protected]>Cc "Franky (Zhenhui) Lin"<[email protected]>Cc Hante Meuleman<[email protected]>Cc "John W. Linville"<[email protected]>
Quoted 2 lines
BTW, I guess this qualifies as a security hole, albeit a small one. Should this be CC: [email protected]?
I have no strong opinion on this, but I guess. Feel free to do so.
Regards,
Arend
Quoted 21 lines
Quoted 9 lines
--- drivers/net/wireless/brcm80211/brcmsmac/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/wireless/brcm80211/brcmsmac/main.c +0
-0drivers/net/wireless/brcm80211/brcmsmac/main.c static const u16 xmtfifo_sz[][NFIFO] = { · line 322
Quoted 8 lines
drivers/net/wireless/brcm80211/brcmsmac/main.c +1
-1@@ -316,7 +316,7 @@ static const u16 xmtfifo_sz[][NFIFO] = {static const char *const fifo_names[] = {"AC_BK" ,"AC_BE" ,"AC_VI" ,"AC_VO" ,"BCMC" ,"ATIM" };#else -static const char fifo_names[6 ][0 ]; +static const char fifo_names[6 ][1 ];#endif #ifdef DEBUG Brian