[PATCH] netlink: downgrade warning about leftover bytes to debug level

[PATCH] netlink: downgrade warning about leftover bytes to debug level

[Michal Schmidt]

Any process is able to send netlink messages with leftover bytes, so
downgrade the warning message to pr_debug in order to avoid possible
kernel log spam.

Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
 lib/nlattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/nlattr.c b/lib/nlattr.c
index fc67547..fbadde6 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -201,8 +201,8 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
 	}
 
 	if (unlikely(rem > 0))
-		printk(KERN_WARNING "netlink: %d bytes leftover after parsing "
-		       "attributes.\n", rem);
+		pr_debug("netlink: %d bytes leftover after parsing attributes.\n",
+			 rem);
 
 	err = 0;
 errout:
-- 
1.9.3

Re: [PATCH] netlink: downgrade warning about leftover bytes to debug level

[Nicolas Dichtel]

Le 27/05/2014 15:16, Michal Schmidt a écrit :
> Any process is able to send netlink messages with leftover bytes, so
> downgrade the warning message to pr_debug in order to avoid possible
> kernel log spam.
Yes, but that's not a normal case. I think that the level warning is good,
there is something wrong with this kind of messages.

Re: [PATCH] netlink: downgrade warning about leftover bytes to debug level

[Michal Schmidt]

On 05/28/2014 09:42 AM, Nicolas Dichtel wrote:
> Le 27/05/2014 15:16, Michal Schmidt a écrit :
>> Any process is able to send netlink messages with leftover bytes, so
>> downgrade the warning message to pr_debug in order to avoid possible
>> kernel log spam.
> Yes, but that's not a normal case. I think that the level warning is good,
> there is something wrong with this kind of messages.

As an alternative we could at least guard it with net_ratelimit().

But still, the message indicates a userspace bug. It can be useful
for a developer of a netlink library, but not for many more.
I think it's odd to let unprivileged userspace spam the dmesg,
even if rate-limited. The kernel does not emit warning messages when
userspace is buggy in other ways, like calling syscalls with invalid
arguments.

Regards,
Michal

Re: [PATCH] netlink: downgrade warning about leftover bytes to debug level

[Nicolas Dichtel]

Le 28/05/2014 11:18, Michal Schmidt a écrit :
> On 05/28/2014 09:42 AM, Nicolas Dichtel wrote:
>> Le 27/05/2014 15:16, Michal Schmidt a écrit :
>>> Any process is able to send netlink messages with leftover bytes, so
>>> downgrade the warning message to pr_debug in order to avoid possible
>>> kernel log spam.
>> Yes, but that's not a normal case. I think that the level warning is good,
>> there is something wrong with this kind of messages.
>
> As an alternative we could at least guard it with net_ratelimit().
I would prefer this solution.

>
> But still, the message indicates a userspace bug. It can be useful
> for a developer of a netlink library, but not for many more.
Or to let you know why a userspace tool does not work ;-)

> I think it's odd to let unprivileged userspace spam the dmesg,
> even if rate-limited. The kernel does not emit warning messages when
> userspace is buggy in other ways, like calling syscalls with invalid
> arguments.
If it's really a problem (this message is here since 2.6.15 and nobody
complains), maybe a level between warning and debug may be acceptable.


Regards,
Nicolas

[PATCH] netlink: rate-limit leftover bytes warning and print process name

[Michal Schmidt]

Any process is able to send netlink messages with leftover bytes.
Make the warning rate-limited to prevent too much log spam.

The warning is supposed to help find userspace bugs, so print the
triggering command name to implicate the buggy program.

Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
 lib/nlattr.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/nlattr.c b/lib/nlattr.c
index fc67547..31047c4 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -201,8 +201,9 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
 	}
 
 	if (unlikely(rem > 0))
-		printk(KERN_WARNING "netlink: %d bytes leftover after parsing "
-		       "attributes.\n", rem);
+		printk_ratelimited(KERN_WARNING
+				   "netlink: %d bytes leftover after parsing attributes in process `%s'.\n",
+				   rem, current->comm);
 
 	err = 0;
 errout:
-- 
1.9.3

Re: [PATCH] netlink: rate-limit leftover bytes warning and print process name

[Sergei Shtylyov]

Hello.

On 05/30/2014 04:57 PM, Michal Schmidt wrote:

> Any process is able to send netlink messages with leftover bytes.
> Make the warning rate-limited to prevent too much log spam.

> The warning is supposed to help find userspace bugs, so print the
> triggering command name to implicate the buggy program.

> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
>   lib/nlattr.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index fc67547..31047c4 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -201,8 +201,9 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
>   	}
>
>   	if (unlikely(rem > 0))
> -		printk(KERN_WARNING "netlink: %d bytes leftover after parsing "
> -		       "attributes.\n", rem);
> +		printk_ratelimited(KERN_WARNING

    Better pr_warn_ratelimited().

> +				   "netlink: %d bytes leftover after parsing attributes in process `%s'.\n",
> +				   rem, current->comm);

WBR, Sergei

[PATCH v2] netlink: rate-limit leftover bytes warning and print process name

[Michal Schmidt]

Any process is able to send netlink messages with leftover bytes.
Make the warning rate-limited to prevent too much log spam.

The warning is supposed to help find userspace bugs, so print the
triggering command name to implicate the buggy program.

[v2: Use pr_warn_ratelimited instead of printk_ratelimited.]

Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
 lib/nlattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/nlattr.c b/lib/nlattr.c
index fc67547..10ad042 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -201,8 +201,8 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
 	}
 
 	if (unlikely(rem > 0))
-		printk(KERN_WARNING "netlink: %d bytes leftover after parsing "
-		       "attributes.\n", rem);
+		pr_warn_ratelimited("netlink: %d bytes leftover after parsing attributes in process `%s'.\n",
+				    rem, current->comm);
 
 	err = 0;
 errout:
-- 
1.9.3

Re: [PATCH v2] netlink: rate-limit leftover bytes warning and print process name

[David Miller]

From: Michal Schmidt <mschmidt@redhat.com>
Date: Mon,  2 Jun 2014 18:25:02 +0200

> Any process is able to send netlink messages with leftover bytes.
> Make the warning rate-limited to prevent too much log spam.
> 
> The warning is supposed to help find userspace bugs, so print the
> triggering command name to implicate the buggy program.
> 
> [v2: Use pr_warn_ratelimited instead of printk_ratelimited.]
> 
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>

Applied, thanks Michal.

Re: [PATCH] netlink: rate-limit leftover bytes warning and print process name

[David Miller]

From: Michal Schmidt <mschmidt@redhat.com>
Date: Fri, 30 May 2014 14:57:27 +0200

> Any process is able to send netlink messages with leftover bytes.
> Make the warning rate-limited to prevent too much log spam.
> 
> The warning is supposed to help find userspace bugs, so print the
> triggering command name to implicate the buggy program.
> 
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>

Please use pr_warn_ratelimited() as suggested by Sergei.

Thanks.