Re: BUG and WARN kernel log levels
On Wed, 2016-08-17 at 14:19 -0700, Kees Cook wrote: > On Wed, Aug 17, 2016 at 9:36 AM, Joe Percheswrote: [] > > And here I submitted patches: > > https://lkml.org/lkml/2010/10/30/176 > Ah, I see some of this series landed. I see commits scattered in the > tree, but not as many as you sent, I think. What still remains? No idea. There wasn't any enthusiasm to apply the asm-generic patch so I didn't push it.
Re: BUG and WARN kernel log levels
On Wed, 2016-08-17 at 14:19 -0700, Kees Cook wrote: > On Wed, Aug 17, 2016 at 9:36 AM, Joe Perches wrote: [] > > And here I submitted patches: > > https://lkml.org/lkml/2010/10/30/176 > Ah, I see some of this series landed. I see commits scattered in the > tree, but not as many as you sent, I think. What still remains? No idea. There wasn't any enthusiasm to apply the asm-generic patch so I didn't push it.
Re: BUG and WARN kernel log levels
On Wed, Aug 17, 2016 at 9:36 AM, Joe Percheswrote: > On Mon, 2016-08-15 at 12:00 -0700, Joe Perches wrote: >> On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: >> > >> > Hi, >> > >> > So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: >> > >> > #ifndef HAVE_ARCH_BUG >> > #define BUG() do { \ >> >printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, >> > __func__); \ >> > >> > Seems like it should have one? >> > >> > Also, I think we might want to examine WARN() a bit... it doesn't have >> > a log level either, but only a fraction of callers set one: >> > >> > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | >> > wc -l >> > 2735 >> > >> > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc >> > -l >> > 77 >> > >> > If I'm reading checkpatch.pl correctly, it doesn't warn about missing >> > log levels on WARN calls, but I think it should. >> > >> > How do you think is best to clean this up? >> > >> > Mainly, I'd like to add a format string to BUG, or introduce a new >> > BUGish call that takes a format... >> I once suggested something similar awhile ago. >> https://lkml.org/lkml/2008/7/8/261 > > And here I submitted patches: > https://lkml.org/lkml/2010/10/30/176 Ah, I see some of this series landed. I see commits scattered in the tree, but not as many as you sent, I think. What still remains? -Kees -- Kees Cook Nexus Security
Re: BUG and WARN kernel log levels
On Wed, Aug 17, 2016 at 9:36 AM, Joe Perches wrote: > On Mon, 2016-08-15 at 12:00 -0700, Joe Perches wrote: >> On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: >> > >> > Hi, >> > >> > So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: >> > >> > #ifndef HAVE_ARCH_BUG >> > #define BUG() do { \ >> >printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, >> > __func__); \ >> > >> > Seems like it should have one? >> > >> > Also, I think we might want to examine WARN() a bit... it doesn't have >> > a log level either, but only a fraction of callers set one: >> > >> > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | >> > wc -l >> > 2735 >> > >> > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc >> > -l >> > 77 >> > >> > If I'm reading checkpatch.pl correctly, it doesn't warn about missing >> > log levels on WARN calls, but I think it should. >> > >> > How do you think is best to clean this up? >> > >> > Mainly, I'd like to add a format string to BUG, or introduce a new >> > BUGish call that takes a format... >> I once suggested something similar awhile ago. >> https://lkml.org/lkml/2008/7/8/261 > > And here I submitted patches: > https://lkml.org/lkml/2010/10/30/176 Ah, I see some of this series landed. I see commits scattered in the tree, but not as many as you sent, I think. What still remains? -Kees -- Kees Cook Nexus Security
Re: BUG and WARN kernel log levels
On Mon, 2016-08-15 at 12:00 -0700, Joe Perches wrote: > On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: > > > > Hi, > > > > So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: > > > > #ifndef HAVE_ARCH_BUG > > #define BUG() do { \ > > printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, > > __func__); \ > > > > Seems like it should have one? > > > > Also, I think we might want to examine WARN() a bit... it doesn't have > > a log level either, but only a fraction of callers set one: > > > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | > > wc -l > > 2735 > > > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc -l > > 77 > > > > If I'm reading checkpatch.pl correctly, it doesn't warn about missing > > log levels on WARN calls, but I think it should. > > > > How do you think is best to clean this up? > > > > Mainly, I'd like to add a format string to BUG, or introduce a new > > BUGish call that takes a format... > I once suggested something similar awhile ago. > https://lkml.org/lkml/2008/7/8/261 And here I submitted patches: https://lkml.org/lkml/2010/10/30/176
Re: BUG and WARN kernel log levels
On Mon, 2016-08-15 at 12:00 -0700, Joe Perches wrote: > On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: > > > > Hi, > > > > So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: > > > > #ifndef HAVE_ARCH_BUG > > #define BUG() do { \ > > printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, > > __func__); \ > > > > Seems like it should have one? > > > > Also, I think we might want to examine WARN() a bit... it doesn't have > > a log level either, but only a fraction of callers set one: > > > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | > > wc -l > > 2735 > > > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc -l > > 77 > > > > If I'm reading checkpatch.pl correctly, it doesn't warn about missing > > log levels on WARN calls, but I think it should. > > > > How do you think is best to clean this up? > > > > Mainly, I'd like to add a format string to BUG, or introduce a new > > BUGish call that takes a format... > I once suggested something similar awhile ago. > https://lkml.org/lkml/2008/7/8/261 And here I submitted patches: https://lkml.org/lkml/2010/10/30/176
Re: BUG and WARN kernel log levels
On Mon, 2016-08-15 at 13:28 -0700, Kees Cook wrote: > On Mon, Aug 15, 2016 at 12:00 PM, Joe Percheswrote: > > On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: > > > So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: > > > > > > #ifndef HAVE_ARCH_BUG > > > #define BUG() do { \ > > > printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, > > > __func__); \ > > > > > > Seems like it should have one? > > > > > > Also, I think we might want to examine WARN() a bit... it doesn't have > > > a log level either, but only a fraction of callers set one: > > > > > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | > > > wc -l > > > 2735 > > > > > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc > > > -l > > > 77 > > > > > > If I'm reading checkpatch.pl correctly, it doesn't warn about missing > > > log levels on WARN calls, but I think it should. > > > > > > How do you think is best to clean this up? > > > > > > Mainly, I'd like to add a format string to BUG, or introduce a new > > > BUGish call that takes a format... > > I once suggested something similar awhile ago. > > https://lkml.org/lkml/2008/7/8/261 > > > > I think it's best to remove any KERN_ from the use of > > all the WARN variants and add it to the WARN definitions. > Yeah, that's what I was thinking too. It does mean that the format > needs to be a const char string, though. No it doesn't mean that. Prefixing KERN_WARNING to the const char[] types and using KERN_WARNING "%pV" could work for the non const char[] types Or maybe use the same KERN_WARNING "%pV" for simpler code. > > Same with BUG. > Yeah, though for full effect, it needs to be plumbed into each > architecture's BUG handler. Mainly what I don't like is that if I do > this on x86: > > pr_err("eek, terrible thing\n"); > BUG(); > > I get a dmesg that read: > > eek, terrible thing > === cut here === > ...traceback, etc > > I'd like the "eek" part to be inside the "cut here". And I'd like BUGs > to be able to be more verbose. Maybe add BUG_MSG/BUG_ON_MSG or some such.
Re: BUG and WARN kernel log levels
On Mon, 2016-08-15 at 13:28 -0700, Kees Cook wrote: > On Mon, Aug 15, 2016 at 12:00 PM, Joe Perches wrote: > > On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: > > > So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: > > > > > > #ifndef HAVE_ARCH_BUG > > > #define BUG() do { \ > > > printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, > > > __func__); \ > > > > > > Seems like it should have one? > > > > > > Also, I think we might want to examine WARN() a bit... it doesn't have > > > a log level either, but only a fraction of callers set one: > > > > > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | > > > wc -l > > > 2735 > > > > > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc > > > -l > > > 77 > > > > > > If I'm reading checkpatch.pl correctly, it doesn't warn about missing > > > log levels on WARN calls, but I think it should. > > > > > > How do you think is best to clean this up? > > > > > > Mainly, I'd like to add a format string to BUG, or introduce a new > > > BUGish call that takes a format... > > I once suggested something similar awhile ago. > > https://lkml.org/lkml/2008/7/8/261 > > > > I think it's best to remove any KERN_ from the use of > > all the WARN variants and add it to the WARN definitions. > Yeah, that's what I was thinking too. It does mean that the format > needs to be a const char string, though. No it doesn't mean that. Prefixing KERN_WARNING to the const char[] types and using KERN_WARNING "%pV" could work for the non const char[] types Or maybe use the same KERN_WARNING "%pV" for simpler code. > > Same with BUG. > Yeah, though for full effect, it needs to be plumbed into each > architecture's BUG handler. Mainly what I don't like is that if I do > this on x86: > > pr_err("eek, terrible thing\n"); > BUG(); > > I get a dmesg that read: > > eek, terrible thing > === cut here === > ...traceback, etc > > I'd like the "eek" part to be inside the "cut here". And I'd like BUGs > to be able to be more verbose. Maybe add BUG_MSG/BUG_ON_MSG or some such.
Re: BUG and WARN kernel log levels
On Mon, Aug 15, 2016 at 12:00 PM, Joe Percheswrote: > On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: >> Hi, >> >> So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: >> >> #ifndef HAVE_ARCH_BUG >> #define BUG() do { \ >>printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, >> __func__); \ >> >> Seems like it should have one? >> >> Also, I think we might want to examine WARN() a bit... it doesn't have >> a log level either, but only a fraction of callers set one: >> >> $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | wc >> -l >> 2735 >> >> $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc -l >> 77 >> >> If I'm reading checkpatch.pl correctly, it doesn't warn about missing >> log levels on WARN calls, but I think it should. >> >> How do you think is best to clean this up? >> >> Mainly, I'd like to add a format string to BUG, or introduce a new >> BUGish call that takes a format... > > I once suggested something similar awhile ago. > https://lkml.org/lkml/2008/7/8/261 > > I think it's best to remove any KERN_ from the use of > all the WARN variants and add it to the WARN definitions. Yeah, that's what I was thinking too. It does mean that the format needs to be a const char string, though. I haven't checked all of them but most seem to be. It's an easy fix to move them to "%s" as needed, though. > Same with BUG. Yeah, though for full effect, it needs to be plumbed into each architecture's BUG handler. Mainly what I don't like is that if I do this on x86: pr_err("eek, terrible thing\n"); BUG(); I get a dmesg that read: eek, terrible thing === cut here === ...traceback, etc I'd like the "eek" part to be inside the "cut here". And I'd like BUGs to be able to be more verbose. -Kees -- Kees Cook Nexus Security
Re: BUG and WARN kernel log levels
On Mon, Aug 15, 2016 at 12:00 PM, Joe Perches wrote: > On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: >> Hi, >> >> So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: >> >> #ifndef HAVE_ARCH_BUG >> #define BUG() do { \ >>printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, >> __func__); \ >> >> Seems like it should have one? >> >> Also, I think we might want to examine WARN() a bit... it doesn't have >> a log level either, but only a fraction of callers set one: >> >> $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | wc >> -l >> 2735 >> >> $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc -l >> 77 >> >> If I'm reading checkpatch.pl correctly, it doesn't warn about missing >> log levels on WARN calls, but I think it should. >> >> How do you think is best to clean this up? >> >> Mainly, I'd like to add a format string to BUG, or introduce a new >> BUGish call that takes a format... > > I once suggested something similar awhile ago. > https://lkml.org/lkml/2008/7/8/261 > > I think it's best to remove any KERN_ from the use of > all the WARN variants and add it to the WARN definitions. Yeah, that's what I was thinking too. It does mean that the format needs to be a const char string, though. I haven't checked all of them but most seem to be. It's an easy fix to move them to "%s" as needed, though. > Same with BUG. Yeah, though for full effect, it needs to be plumbed into each architecture's BUG handler. Mainly what I don't like is that if I do this on x86: pr_err("eek, terrible thing\n"); BUG(); I get a dmesg that read: eek, terrible thing === cut here === ...traceback, etc I'd like the "eek" part to be inside the "cut here". And I'd like BUGs to be able to be more verbose. -Kees -- Kees Cook Nexus Security
Re: BUG and WARN kernel log levels
On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: > Hi, > > So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: > > #ifndef HAVE_ARCH_BUG > #define BUG() do { \ > printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); > \ > > Seems like it should have one? > > Also, I think we might want to examine WARN() a bit... it doesn't have > a log level either, but only a fraction of callers set one: > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | wc > -l > 2735 > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc -l > 77 > > If I'm reading checkpatch.pl correctly, it doesn't warn about missing > log levels on WARN calls, but I think it should. > > How do you think is best to clean this up? > > Mainly, I'd like to add a format string to BUG, or introduce a new > BUGish call that takes a format... I once suggested something similar awhile ago. https://lkml.org/lkml/2008/7/8/261 I think it's best to remove any KERN_ from the use of all the WARN variants and add it to the WARN definitions. Same with BUG.
Re: BUG and WARN kernel log levels
On Mon, 2016-08-15 at 11:53 -0700, Kees Cook wrote: > Hi, > > So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: > > #ifndef HAVE_ARCH_BUG > #define BUG() do { \ > printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); > \ > > Seems like it should have one? > > Also, I think we might want to examine WARN() a bit... it doesn't have > a log level either, but only a fraction of callers set one: > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | wc > -l > 2735 > > $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc -l > 77 > > If I'm reading checkpatch.pl correctly, it doesn't warn about missing > log levels on WARN calls, but I think it should. > > How do you think is best to clean this up? > > Mainly, I'd like to add a format string to BUG, or introduce a new > BUGish call that takes a format... I once suggested something similar awhile ago. https://lkml.org/lkml/2008/7/8/261 I think it's best to remove any KERN_ from the use of all the WARN variants and add it to the WARN definitions. Same with BUG.
BUG and WARN kernel log levels
Hi, So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: #ifndef HAVE_ARCH_BUG #define BUG() do { \ printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ Seems like it should have one? Also, I think we might want to examine WARN() a bit... it doesn't have a log level either, but only a fraction of callers set one: $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | wc -l 2735 $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc -l 77 If I'm reading checkpatch.pl correctly, it doesn't warn about missing log levels on WARN calls, but I think it should. How do you think is best to clean this up? Mainly, I'd like to add a format string to BUG, or introduce a new BUGish call that takes a format... -Kees -- Kees Cook Nexus Security
BUG and WARN kernel log levels
Hi, So, I noticed that asm-gemeric/bug.h defines BUG() without a log level: #ifndef HAVE_ARCH_BUG #define BUG() do { \ printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ Seems like it should have one? Also, I think we might want to examine WARN() a bit... it doesn't have a log level either, but only a fraction of callers set one: $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep -v KERN_ | wc -l 2735 $ git grep -E 'WARN(_TAINT|)(_RATELIMIT|_ONCE|)\([^\)]' | grep KERN_ | wc -l 77 If I'm reading checkpatch.pl correctly, it doesn't warn about missing log levels on WARN calls, but I think it should. How do you think is best to clean this up? Mainly, I'd like to add a format string to BUG, or introduce a new BUGish call that takes a format... -Kees -- Kees Cook Nexus Security