[PATCH] net: Remove a noisy printk

[PATCH] net: Remove a noisy printk

[Adam Jackson]

From: Dave Jones <davej@redhat.com>

These messages are trivial to trigger when running stress tests
like isic, and add no real value.

Signed-off-by: Dave Jones <davej@redhat.com>
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index 4a7c352..0e86d0d 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -147,8 +147,6 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
 	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk("ipt_hook: happy cracking.\n");
 		return NF_ACCEPT;
 	}
 	return nf_conntrack_in(dev_net(out), PF_INET, hooknum, skb);
-- 
1.6.0.3

[PATCH] ACPI: Silence blacklist warnings.

[Adam Jackson]

If you have a machine that matches these warnings, there's almost
certainly nothing you can do about it, so don't punish quiet boot.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/acpi/blacklist.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
index 09c6980..786376e 100644
--- a/drivers/acpi/blacklist.c
+++ b/drivers/acpi/blacklist.c
@@ -81,18 +81,18 @@ static int __init blacklist_by_year(void)
 	int year = dmi_get_year(DMI_BIOS_DATE);
 	/* Doesn't exist? Likely an old system */
 	if (year == -1) {
-		printk(KERN_ERR PREFIX "no DMI BIOS year, "
+		printk(KERN_INFO PREFIX "no DMI BIOS year, "
 			"acpi=force is required to enable ACPI\n" );
 		return 1;
 	}
 	/* 0? Likely a buggy new BIOS */
 	if (year == 0) {
-		printk(KERN_ERR PREFIX "DMI BIOS year==0, "
+		printk(KERN_INFO PREFIX "DMI BIOS year==0, "
 			"assuming ACPI-capable machine\n" );
 		return 0;
 	}
 	if (year < CONFIG_ACPI_BLACKLIST_YEAR) {
-		printk(KERN_ERR PREFIX "BIOS age (%d) fails cutoff (%d), "
+		printk(KERN_INFO PREFIX "BIOS age (%d) fails cutoff (%d), "
 		       "acpi=force is required to enable ACPI\n",
 		       year, CONFIG_ACPI_BLACKLIST_YEAR);
 		return 1;
-- 
1.6.0.3

[PATCH] x86: Don't print error for lack of i8042 chip

[Adam Jackson]

From: Peter Jones <pjones@redhat.com>

Some systems, such as EFI-based Apple systems, won't necessarily have an
i8042 to initialize.  We shouldn't be printing an error message in this
case, since not detecting the chip is the correct behavior.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/input/serio/i8042.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 170f71e..4f3e632 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -701,10 +701,8 @@ static int __devinit i8042_check_aux(void)
 
 static int i8042_controller_check(void)
 {
-	if (i8042_flush() == I8042_BUFFER_SIZE) {
-		printk(KERN_ERR "i8042.c: No controller found.\n");
+	if (i8042_flush() == I8042_BUFFER_SIZE)
 		return -ENODEV;
-	}
 
 	return 0;
 }
-- 
1.6.0.3

[PATCH] PCI: Don't carp about BAR allocation failures in quiet boot

[Adam Jackson]

These are easy to trigger (more or less harmlessly) with multiple video
cards, since the ROM BAR will typically not be given any space by the
BIOS bridge setup.  No reason to punish quiet boot for this.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 arch/x86/pci/i386.c     |    4 ++--
 drivers/pci/setup-res.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 844df0c..43d9783 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -129,7 +129,7 @@ static void __init pcibios_allocate_bus_resources(struct list_head *bus_list)
 				pr = pci_find_parent_resource(dev, r);
 				if (!r->start || !pr ||
 				    request_resource(pr, r) < 0) {
-					dev_err(&dev->dev, "BAR %d: can't allocate resource\n", idx);
+					dev_info(&dev->dev, "BAR %d: can't allocate resource\n", idx);
 					/*
 					 * Something is wrong with the region.
 					 * Invalidate the resource to prevent
@@ -170,7 +170,7 @@ static void __init pcibios_allocate_resources(int pass)
 					r->flags, disabled, pass);
 				pr = pci_find_parent_resource(dev, r);
 				if (!pr || request_resource(pr, r) < 0) {
-					dev_err(&dev->dev, "BAR %d: can't allocate resource\n", idx);
+					dev_info(&dev->dev, "BAR %d: can't allocate resource\n", idx);
 					/* We'll assign a new address later */
 					r->end -= r->start;
 					r->start = 0;
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 2dbd96c..4e37563 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -134,7 +134,7 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
 
 	align = resource_alignment(res);
 	if (!align) {
-		dev_err(&dev->dev, "BAR %d: can't allocate resource (bogus "
+		dev_info(&dev->dev, "BAR %d: can't allocate resource (bogus "
 			"alignment) %pR flags %#lx\n",
 			resno, res, res->flags);
 		return -EINVAL;
@@ -157,7 +157,7 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
 	}
 
 	if (ret) {
-		dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
+		dev_info(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
 			resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
 	} else {
 		res->flags &= ~IORESOURCE_STARTALIGN;
-- 
1.6.0.3

Re: [PATCH] net: Remove a noisy printk

[Sam Ravnborg]

On Thu, Dec 11, 2008 at 05:13:42PM -0500, Adam Jackson wrote:
> From: Dave Jones <davej@redhat.com>
> 
> These messages are trivial to trigger when running stress tests
> like isic, and add no real value.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>
> Signed-off-by: Adam Jackson <ajax@redhat.com>

I would suggest to send this to netdev.

	Sam

Re: [PATCH] net: Remove a noisy printk

[David Miller]

From: Adam Jackson <ajax@redhat.com>
Date: Thu, 11 Dec 2008 17:13:42 -0500

> From: Dave Jones <davej@redhat.com>
> 
> These messages are trivial to trigger when running stress tests
> like isic, and add no real value.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>
> Signed-off-by: Adam Jackson <ajax@redhat.com>

If you don't send this to the netfilter developers nor the networking
developers, noboby knowledgable can even look at this patch.

CC:'d

> ---
>  net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> index 4a7c352..0e86d0d 100644
> --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> @@ -147,8 +147,6 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
>  	/* root is playing with raw sockets. */
>  	if (skb->len < sizeof(struct iphdr) ||
>  	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
> -		if (net_ratelimit())
> -			printk("ipt_hook: happy cracking.\n");
>  		return NF_ACCEPT;
>  	}
>  	return nf_conntrack_in(dev_net(out), PF_INET, hooknum, skb);
> -- 
> 1.6.0.3

Re: [PATCH] net: Remove a noisy printk

[Jan Engelhardt]

On Friday 2008-12-12 05:32, David Miller wrote:
>From: Adam Jackson <ajax@redhat.com>
>Date: Thu, 11 Dec 2008 17:13:42 -0500
>
>> From: Dave Jones <davej@redhat.com>
>> 
>> These messages are trivial to trigger when running stress tests
>> like isic, and add no real value.
>> 
>> Signed-off-by: Dave Jones <davej@redhat.com>
>> Signed-off-by: Adam Jackson <ajax@redhat.com>
>
>If you don't send this to the netfilter developers nor the networking
>developers, noboby knowledgable can even look at this patch.

(For the archive, it's not the Internation Student Identity Card)
http://www.packetfactory.net/Projects/ISIC/ :

“ISIC is a suite of utilities to exercise the stability of an IP
Stack and its component stacks (TCP, UDP, ICMP et. al.) It generates
piles of pseudo random packets of the target protocol.”

>> @@ -147,8 +147,6 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
>>  	/* root is playing with raw sockets. */
>>  	if (skb->len < sizeof(struct iphdr) ||
>>  	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
>> -		if (net_ratelimit())
>> -			printk("ipt_hook: happy cracking.\n");
>>  		return NF_ACCEPT;
>>  	}
>>  	return nf_conntrack_in(dev_net(out), PF_INET, hooknum, skb);

I think this change is ok.

Re: [PATCH] net: Remove a noisy printk

[Jozsef Kadlecsik]

On Sat, 13 Dec 2008, Jan Engelhardt wrote:

> On Friday 2008-12-12 05:32, David Miller wrote:
> >From: Adam Jackson <ajax@redhat.com>
> >Date: Thu, 11 Dec 2008 17:13:42 -0500
> >
> >> From: Dave Jones <davej@redhat.com>
> >> 
> >> These messages are trivial to trigger when running stress tests
> >> like isic, and add no real value.
> >> 
> >> Signed-off-by: Dave Jones <davej@redhat.com>
> >> Signed-off-by: Adam Jackson <ajax@redhat.com>
> >
> >If you don't send this to the netfilter developers nor the networking
> >developers, noboby knowledgable can even look at this patch.
> 
> (For the archive, it's not the Internation Student Identity Card)
> http://www.packetfactory.net/Projects/ISIC/ :
> 
> ?ISIC is a suite of utilities to exercise the stability of an IP
> Stack and its component stacks (TCP, UDP, ICMP et. al.) It generates
> piles of pseudo random packets of the target protocol.?
> 
> >> @@ -147,8 +147,6 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
> >>  	/* root is playing with raw sockets. */
> >>  	if (skb->len < sizeof(struct iphdr) ||
> >>  	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
> >> -		if (net_ratelimit())
> >> -			printk("ipt_hook: happy cracking.\n");
> >>  		return NF_ACCEPT;
> >>  	}
> >>  	return nf_conntrack_in(dev_net(out), PF_INET, hooknum, skb);
> 
> I think this change is ok.

In a >normal< system one usually does not use raw sockets. So if a root 
process do use raw socket, at least netfilter sends a notification and 
there's a chance that someone take notice it by checking the kernel logs.

Yes, if the machine is compromized then the logs can be tampered, if 
netfilter is compiled as a module, the module can be replaced by another 
one, and so on. A careful cracker can take care of all the alarms, trip 
wires. But should we remove them due to nuisances on >test< systems?

Rather make it a kernel compile option but do not remove.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] net: Remove a noisy printk

[Jan Engelhardt]

On Sunday 2008-12-14 18:09, Jozsef Kadlecsik wrote:
>> 
>> >> @@ -147,8 +147,6 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
>> >>  	/* root is playing with raw sockets. */
>> >>  	if (skb->len < sizeof(struct iphdr) ||
>> >>  	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
>> >> -		if (net_ratelimit())
>> >> -			printk("ipt_hook: happy cracking.\n");
>> >>  		return NF_ACCEPT;
>> >>  	}
>> >>  	return nf_conntrack_in(dev_net(out), PF_INET, hooknum, skb);
>> 
>> I think this change is ok.
>
>In a >normal< system one usually does not use raw sockets. So if a root 
>process do use raw socket, at least netfilter sends a notification and 
>there's a chance that someone take notice it by checking the kernel logs.
>[...]
>But should we remove them due to nuisances on >test< systems?
>
>Rather make it a kernel compile option but do not remove.

This warning is in the conntrack calling code. Iff you play with
raw sockets and do something wrong, the generic network code
should barf IMHO, not nf_conntrack, and not [nf_conntrack_ipv4 only].

Re: [PATCH] net: Remove a noisy printk

[Dave Jones]

On Sun, Dec 14, 2008 at 06:09:17PM +0100, Jozsef Kadlecsik wrote:

 > > >> @@ -147,8 +147,6 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
 > > >>  	/* root is playing with raw sockets. */
 > > >>  	if (skb->len < sizeof(struct iphdr) ||
 > > >>  	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
 > > >> -		if (net_ratelimit())
 > > >> -			printk("ipt_hook: happy cracking.\n");
 > > >>  		return NF_ACCEPT;
 > > >>  	}
 > > >>  	return nf_conntrack_in(dev_net(out), PF_INET, hooknum, skb);
 > > 
 > > I think this change is ok.
 > 
 > In a >normal< system one usually does not use raw sockets. So if a root 
 > process do use raw socket, at least netfilter sends a notification and 
 > there's a chance that someone take notice it by checking the kernel logs.

'normal' systems are irrelevant here. This message is triggerable remotely.
Even though it's ratelimited, anyone can flood another boxes logs by
sending enough packets.

The message is also utterly useless. What kind of action would you take
to a few gigabytes of "ipt_hook: happy cracking.\n" ?
There's no IP address logged, or any other useful information.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] net: Remove a noisy printk

[Jozsef Kadlecsik]

On Sun, 14 Dec 2008, Jan Engelhardt wrote:

> On Sunday 2008-12-14 18:09, Jozsef Kadlecsik wrote:
> >> 
> >> >> @@ -147,8 +147,6 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
> >> >>  	/* root is playing with raw sockets. */
> >> >>  	if (skb->len < sizeof(struct iphdr) ||
> >> >>  	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
> >> >> -		if (net_ratelimit())
> >> >> -			printk("ipt_hook: happy cracking.\n");
> >> >>  		return NF_ACCEPT;
> >> >>  	}
> >> >>  	return nf_conntrack_in(dev_net(out), PF_INET, hooknum, skb);
> >> 
> >> I think this change is ok.
> >
> >In a >normal< system one usually does not use raw sockets. So if a root 
> >process do use raw socket, at least netfilter sends a notification and 
> >there's a chance that someone take notice it by checking the kernel logs.
> >[...]
> >But should we remove them due to nuisances on >test< systems?
> >
> >Rather make it a kernel compile option but do not remove.
> 
> This warning is in the conntrack calling code. Iff you play with
> raw sockets and do something wrong, the generic network code
> should barf IMHO, not nf_conntrack, and not [nf_conntrack_ipv4 only].

It is not about doing something wrong at using raw sockets - it's about 
using raw sockets.

I'm not quite convinced the generic network code should warn about raw 
sockets. I believe it belongs to the security-related subsystems - 
netfilter and (or) the security frameworks. [But as netfilter is much more 
widely used, the 'or' is just theoretical.)

But back to netfilter: this is more than strange. The logging is already 
removed from iptable_filter|mangle.c - and left in 
ip6table_filter|mangle.c. The only place in the IPv4 path where the 
checking happens is in nf_conntrack_l3proto_ipv4.c. IPv6 conntrack does
it as well, but uses another message text:

        if (skb->len < sizeof(struct ipv6hdr)) {
                if (net_ratelimit())
                        printk("ipv6_conntrack_local: packet too short\n");

[ISIC should check IPv6 too ;-).]

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] net: Remove a noisy printk

[Patrick McHardy]

Jozsef Kadlecsik wrote:
> On Sun, 14 Dec 2008, Jan Engelhardt wrote:
> 
>>> In a >normal< system one usually does not use raw sockets. So if a root 
>>> process do use raw socket, at least netfilter sends a notification and 
>>> there's a chance that someone take notice it by checking the kernel logs.
>>> [...]
>>> But should we remove them due to nuisances on >test< systems?
>>>
>>> Rather make it a kernel compile option but do not remove.
>> This warning is in the conntrack calling code. Iff you play with
>> raw sockets and do something wrong, the generic network code
>> should barf IMHO, not nf_conntrack, and not [nf_conntrack_ipv4 only].
> 
> It is not about doing something wrong at using raw sockets - it's about 
> using raw sockets.
> 
> I'm not quite convinced the generic network code should warn about raw 
> sockets. I believe it belongs to the security-related subsystems - 
> netfilter and (or) the security frameworks. [But as netfilter is much more 
> widely used, the 'or' is just theoretical.)

I agree that it doesn't belong to the generic networking code.
But the way its handled in netfilter is far from perfect as well.
Currently multiple modules will spam the ringbuffer repeatedly,
but offer no possibility to change anything in the behaviour of
how these packets are treated. Unfortunately we can't handle this
in the ruleset (which is exactly the reason why we're spamming
the ringbuffer), so how about we add a module option controlling
how to treat those packets and remove the printk?

In the case of conntrack I would even argue that the message
makes no sense at all since tracking doesn't matter as long as
the state isn't used. And for that the table code can warn or
offer controls.

Re: [PATCH] net: Remove a noisy printk

[Jozsef Kadlecsik]

On Mon, 15 Dec 2008, Patrick McHardy wrote:

> Jozsef Kadlecsik wrote:
> > On Sun, 14 Dec 2008, Jan Engelhardt wrote:
> > 
> > > > In a >normal< system one usually does not use raw sockets. So if a root
> > > > process do use raw socket, at least netfilter sends a notification and
> > > > there's a chance that someone take notice it by checking the kernel
> > > > logs.
> > > > [...]
> > > > But should we remove them due to nuisances on >test< systems?
> > > > 
> > > > Rather make it a kernel compile option but do not remove.
> > > This warning is in the conntrack calling code. Iff you play with
> > > raw sockets and do something wrong, the generic network code
> > > should barf IMHO, not nf_conntrack, and not [nf_conntrack_ipv4 only].
> > 
> > It is not about doing something wrong at using raw sockets - it's about
> > using raw sockets.
> > 
> > I'm not quite convinced the generic network code should warn about raw
> > sockets. I believe it belongs to the security-related subsystems - netfilter
> > and (or) the security frameworks. [But as netfilter is much more widely
> > used, the 'or' is just theoretical.)
> 
> I agree that it doesn't belong to the generic networking code.
> But the way its handled in netfilter is far from perfect as well.
> Currently multiple modules will spam the ringbuffer repeatedly,
> but offer no possibility to change anything in the behaviour of
> how these packets are treated. Unfortunately we can't handle this
> in the ruleset (which is exactly the reason why we're spamming
> the ringbuffer), so how about we add a module option controlling
> how to treat those packets and remove the printk?

How about this: let the printk be removed from conntrack and the mangle 
table but put (back) into the filter table with a module option, which 
controls the behaviour (drop/accept & log/nolog)?

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] net: Remove a noisy printk

[Patrick McHardy]

Jozsef Kadlecsik wrote:
> On Mon, 15 Dec 2008, Patrick McHardy wrote:
> 
>> I agree that it doesn't belong to the generic networking code.
>> But the way its handled in netfilter is far from perfect as well.
>> Currently multiple modules will spam the ringbuffer repeatedly,
>> but offer no possibility to change anything in the behaviour of
>> how these packets are treated. Unfortunately we can't handle this
>> in the ruleset (which is exactly the reason why we're spamming
>> the ringbuffer), so how about we add a module option controlling
>> how to treat those packets and remove the printk?
> 
> How about this: let the printk be removed from conntrack and the mangle 
> table but put (back) into the filter table with a module option, which 
> controls the behaviour (drop/accept & log/nolog)?

Sounds fine to me. We can't log it in the usual way though
(ipt_LOG/nfnetlink_log) and spamming the ringbuffer should
really be a last resort, so I'd prefer to limit it to print
the message exactly once.

Re: [PATCH] net: Remove a noisy printk

[Jozsef Kadlecsik]

On Sun, 14 Dec 2008, Dave Jones wrote:

> On Sun, Dec 14, 2008 at 06:09:17PM +0100, Jozsef Kadlecsik wrote:
> 
>  > > >> @@ -147,8 +147,6 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
>  > > >>  	/* root is playing with raw sockets. */
>  > > >>  	if (skb->len < sizeof(struct iphdr) ||
>  > > >>  	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
>  > > >> -		if (net_ratelimit())
>  > > >> -			printk("ipt_hook: happy cracking.\n");
>  > > >>  		return NF_ACCEPT;
>  > > >>  	}
>  > > >>  	return nf_conntrack_in(dev_net(out), PF_INET, hooknum, skb);
>  > > 
>  > > I think this change is ok.
>  > 
>  > In a >normal< system one usually does not use raw sockets. So if a root 
>  > process do use raw socket, at least netfilter sends a notification and 
>  > there's a chance that someone take notice it by checking the kernel logs.
> 
> 'normal' systems are irrelevant here. This message is triggerable remotely.
> Even though it's ratelimited, anyone can flood another boxes logs by
> sending enough packets.

The packets in question are shorter than the IP header. How would those be 
delivered to the host? On the LAN it might be possible to forge such 
packets with proper HW address and be delivered. But wouldn't the network 
card or the stack itself throw away the packets??
 
> The message is also utterly useless. What kind of action would you take
> to a few gigabytes of "ipt_hook: happy cracking.\n" ?
> There's no IP address logged, or any other useful information.

As the packet is shorter than the IP header, what could be logged, besides 
the fact that something worth to investigate is happening?

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] net: Remove a noisy printk

[Jan Engelhardt]

On Sunday 2008-12-14 21:03, Dave Jones wrote:
> > > >> @@ -147,8 +147,6 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
> > > >>  	/* root is playing with raw sockets. */
> > > >>  	if (skb->len < sizeof(struct iphdr) ||
> > > >>  	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
> > > >> -		if (net_ratelimit())
> > > >> -			printk("ipt_hook: happy cracking.\n");
> > > >>  		return NF_ACCEPT;
> > > >>  	}
> > > >>  	return nf_conntrack_in(dev_net(out), PF_INET, hooknum, skb);
> > > 
> > > I think this change is ok.
> > 
> > In a >normal< system one usually does not use raw sockets. So if
> > a root process do use raw socket, at least netfilter sends a
> > notification and there's a chance that someone take notice it by
> > checking the kernel logs.
>
>'normal' systems are irrelevant here. This message is triggerable remotely.
>Even though it's ratelimited, anyone can flood another boxes logs by
>sending enough packets.
>
>The message is also utterly useless. What kind of action would you take
>to a few gigabytes of "ipt_hook: happy cracking.\n" ?
>There's no IP address logged, or any other useful information.


Here is a patch that attempts silence both the fraction
that wants to keep the printk and those to get rid of it.
It trips up on the bloatmeters, though.

(No, this patch is not serious, but should give you some
thinking as to why the printk is in there at all.)


diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index 4a7c352..ac72770 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -27,6 +27,9 @@
 #include <net/netfilter/nf_nat_helper.h>
 #include <net/netfilter/ipv4/nf_defrag_ipv4.h>
 
+static unsigned int happy_cracking;
+module_param(happy_cracking, bool, S_IRUGO | S_IWUSR);
+
 int (*nf_nat_seq_adjust_hook)(struct sk_buff *skb,
 			      struct nf_conn *ct,
 			      enum ip_conntrack_info ctinfo);
@@ -147,7 +150,7 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
 	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
+		if (happy_cracking && net_ratelimit())
 			printk("ipt_hook: happy cracking.\n");
 		return NF_ACCEPT;
 	}

Re: [PATCH] net: Remove a noisy printk

[Jan Engelhardt]

On Tuesday 2008-12-16 20:59, Jozsef Kadlecsik wrote:
>
>The packets in question are shorter than the IP header. How would those be 
>delivered to the host? On the LAN it might be possible to forge such 
>packets with proper HW address and be delivered. But wouldn't the network 
>card or the stack itself throw away the packets??

Yes.

>> The message is also utterly useless. What kind of action would you take
>> to a few gigabytes of "ipt_hook: happy cracking.\n" ?
>> There's no IP address logged, or any other useful information.
>
>As the packet is shorter than the IP header, what could be logged, besides 
>the fact that something worth to investigate is happening?

Find the offending process? printk current->pid.

'current' should be available in the output path,
but don't quote me on that.

Re: [PATCH] net: Remove a noisy printk

[Jozsef Kadlecsik]

On Tue, 16 Dec 2008, Jan Engelhardt wrote:

> Here is a patch that attempts silence both the fraction
> that wants to keep the printk and those to get rid of it.
> It trips up on the bloatmeters, though.

Based on your patch, here is another one: the printk is removed from 
everywhere except the filter tables where it's controlled by the module 
parameter. The checking against short packets was missing from 
ip6table_raw.c, so it's added as well.

diff --git a/net/ipv4/netfilter/iptable_filter.c b/net/ipv4/netfilter/iptable_filter.c
index 1ea677d..9527e2a 100644
--- a/net/ipv4/netfilter/iptable_filter.c
+++ b/net/ipv4/netfilter/iptable_filter.c
@@ -19,6 +19,10 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Netfilter Core Team <coreteam@netfilter.org>");
 MODULE_DESCRIPTION("iptables filter table");
 
+/* Default log short RAW packets */
+static unsigned int happy_cracking = 1;
+module_param(happy_cracking, bool, 0400);
+
 #define FILTER_VALID_HOOKS ((1 << NF_INET_LOCAL_IN) | \
 			    (1 << NF_INET_FORWARD) | \
 			    (1 << NF_INET_LOCAL_OUT))
@@ -94,7 +98,8 @@ ipt_local_out_hook(unsigned int hook,
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
 	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
+		if (happy_cracking && net_ratelimit())
+			/* FIXME: log process pid */
 			printk("iptable_filter: ignoring short SOCK_RAW "
 			       "packet.\n");
 		return NF_ACCEPT;
diff --git a/net/ipv4/netfilter/iptable_mangle.c b/net/ipv4/netfilter/iptable_mangle.c
index da59182..773d6ed 100644
--- a/net/ipv4/netfilter/iptable_mangle.c
+++ b/net/ipv4/netfilter/iptable_mangle.c
@@ -132,12 +132,8 @@ ipt_local_hook(unsigned int hook,
 
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr)
-	    || ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk("iptable_mangle: ignoring short SOCK_RAW "
-			       "packet.\n");
+	    || ip_hdrlen(skb) < sizeof(struct iphdr))
 		return NF_ACCEPT;
-	}
 
 	/* Save things which could affect route */
 	mark = skb->mark;
diff --git a/net/ipv4/netfilter/iptable_raw.c b/net/ipv4/netfilter/iptable_raw.c
index fddce77..71547fa 100644
--- a/net/ipv4/netfilter/iptable_raw.c
+++ b/net/ipv4/netfilter/iptable_raw.c
@@ -65,12 +65,8 @@ ipt_local_hook(unsigned int hook,
 {
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
-	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk("iptable_raw: ignoring short SOCK_RAW "
-			       "packet.\n");
+	    ip_hdrlen(skb) < sizeof(struct iphdr))
 		return NF_ACCEPT;
-	}
 	return ipt_do_table(skb, hook, in, out,
 			    nf_local_out_net(in, out)->ipv4.iptable_raw);
 }
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index 7eb0b61..d20c0a0 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -185,11 +185,8 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
 {
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
-	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk("ipt_hook: happy cracking.\n");
+	    ip_hdrlen(skb) < sizeof(struct iphdr))
 		return NF_ACCEPT;
-	}
 	return nf_conntrack_in(PF_INET, hooknum, skb);
 }
 
diff --git a/net/ipv6/netfilter/ip6table_filter.c b/net/ipv6/netfilter/ip6table_filter.c
index 55a2c29..a74b0e6 100644
--- a/net/ipv6/netfilter/ip6table_filter.c
+++ b/net/ipv6/netfilter/ip6table_filter.c
@@ -17,6 +17,10 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Netfilter Core Team <coreteam@netfilter.org>");
 MODULE_DESCRIPTION("ip6tables filter table");
 
+/* Default log short RAW packets */
+static unsigned int happy_cracking = 1;
+module_param(happy_cracking, bool, 0400);
+
 #define FILTER_VALID_HOOKS ((1 << NF_INET_LOCAL_IN) | \
 			    (1 << NF_INET_FORWARD) | \
 			    (1 << NF_INET_LOCAL_OUT))
@@ -89,15 +93,14 @@ ip6t_local_out_hook(unsigned int hook,
 		   const struct net_device *out,
 		   int (*okfn)(struct sk_buff *))
 {
-#if 0
 	/* root is playing with raw sockets. */
-	if (skb->len < sizeof(struct iphdr)
-	    || ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk("ip6t_hook: happy cracking.\n");
+	if (skb->len < sizeof(struct ipv6hdr)) {
+		if (happy_cracking && net_ratelimit())
+			/* FIXME: log process pid */
+			printk("ip6table_filter: ignoring short SOCK_RAW "
+			       "packet.\n");
 		return NF_ACCEPT;
 	}
-#endif
 
 	return ip6t_do_table(skb, hook, in, out,
 			     nf_local_out_net(in, out)->ipv6.ip6table_filter);
diff --git a/net/ipv6/netfilter/ip6table_mangle.c b/net/ipv6/netfilter/ip6table_mangle.c
index f405cea..5c93909 100644
--- a/net/ipv6/netfilter/ip6table_mangle.c
+++ b/net/ipv6/netfilter/ip6table_mangle.c
@@ -89,15 +89,9 @@ ip6t_local_hook(unsigned int hook,
 	u_int8_t hop_limit;
 	u_int32_t flowlabel, mark;
 
-#if 0
 	/* root is playing with raw sockets. */
-	if (skb->len < sizeof(struct iphdr)
-	    || ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk("ip6t_hook: happy cracking.\n");
+	if (skb->len < sizeof(struct ipv6hdr))
 		return NF_ACCEPT;
-	}
-#endif
 
 	/* save source/dest address, mark, hoplimit, flowlabel, priority,  */
 	memcpy(&saddr, &ipv6_hdr(skb)->saddr, sizeof(saddr));
diff --git a/net/ipv6/netfilter/ip6table_raw.c b/net/ipv6/netfilter/ip6table_raw.c
index 92b9107..4e24ff9 100644
--- a/net/ipv6/netfilter/ip6table_raw.c
+++ b/net/ipv6/netfilter/ip6table_raw.c
@@ -54,6 +54,19 @@ ip6t_hook(unsigned int hook,
 	return ip6t_do_table(skb, hook, in, out, init_net.ipv6.ip6table_raw);
 }
 
+static unsigned int
+ip6t_local_hook(unsigned int hook,
+		struct sk_buff *skb,
+		const struct net_device *in,
+		const struct net_device *out,
+		int (*okfn)(struct sk_buff *))
+{
+	/* root is playing with raw sockets. */
+	if (skb->len < sizeof(struct ipv6hdr))
+		return NF_ACCEPT;
+	return ip6t_do_table(skb, hook, in, out, init_net.ipv6.ip6table_raw);
+}
+
 static struct nf_hook_ops ip6t_ops[] __read_mostly = {
 	{
 	  .hook = ip6t_hook,
@@ -63,7 +76,7 @@ static struct nf_hook_ops ip6t_ops[] __read_mostly = {
 	  .owner = THIS_MODULE,
 	},
 	{
-	  .hook = ip6t_hook,
+	  .hook = ip6t_local_hook,
 	  .pf = PF_INET6,
 	  .hooknum = NF_INET_LOCAL_OUT,
 	  .priority = NF_IP6_PRI_FIRST,
diff --git a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
index 85050c0..462360e 100644
--- a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
@@ -245,11 +245,8 @@ static unsigned int ipv6_conntrack_local(unsigned int hooknum,
 					 int (*okfn)(struct sk_buff *))
 {
 	/* root is playing with raw sockets. */
-	if (skb->len < sizeof(struct ipv6hdr)) {
-		if (net_ratelimit())
-			printk("ipv6_conntrack_local: packet too short\n");
+	if (skb->len < sizeof(struct ipv6hdr))
 		return NF_ACCEPT;
-	}
 	return ipv6_conntrack_in(hooknum, skb, in, out, okfn);
 }
 

Bests regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] net: Remove a noisy printk

[Rusty Russell]

On Monday 15 December 2008 06:33:53 Dave Jones wrote:
> On Sun, Dec 14, 2008 at 06:09:17PM +0100, Jozsef Kadlecsik wrote:
>  > In a >normal< system one usually does not use raw sockets. So if a root 
>  > process do use raw socket, at least netfilter sends a notification and 
>  > there's a chance that someone take notice it by checking the kernel logs.
> 
> 'normal' systems are irrelevant here. This message is triggerable remotely.

I don't think it can be.  This is for truncated locally-generated outgoing
packets, which can only happen when root is playing with raw sockets.

As you can probably tell, I was the one who wrote this printk :)  IMHO,
one reasonable complaint is sufficient to have it removed, so just remove
it.  If anyone thinks it's valuable, put a static counter < 5 around it
and add pid/comm info.

Cheers,
Rusty.

Re: [PATCH] net: Remove a noisy printk

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 954 bytes --]

Rusty Russell wrote:
> On Monday 15 December 2008 06:33:53 Dave Jones wrote:
>> On Sun, Dec 14, 2008 at 06:09:17PM +0100, Jozsef Kadlecsik wrote:
>>  > In a >normal< system one usually does not use raw sockets. So if a root 
>>  > process do use raw socket, at least netfilter sends a notification and 
>>  > there's a chance that someone take notice it by checking the kernel logs.
>>
>> 'normal' systems are irrelevant here. This message is triggerable remotely.
> 
> I don't think it can be.  This is for truncated locally-generated outgoing
> packets, which can only happen when root is playing with raw sockets.

Yes, it can only be triggered locally by root.

> As you can probably tell, I was the one who wrote this printk :)  IMHO,
> one reasonable complaint is sufficient to have it removed, so just remove
> it.  If anyone thinks it's valuable, put a static counter < 5 around it
> and add pid/comm info.

I've queued this patch to remove it.


[-- Attachment #2: 01.diff --]
[-- Type: text/x-patch, Size: 3605 bytes --]

commit ea1926cabd0076846119a7e10f29070907fc296c
Author: Patrick McHardy <kaber@trash.net>
Date:   Mon Jan 12 06:01:48 2009 +0100

    netfilter: remove "happy cracking" message
    
    Don't spam logs for locally generated short packets. these can only
    be generated by root.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/ipv4/netfilter/iptable_filter.c b/net/ipv4/netfilter/iptable_filter.c
index c922431..52cb693 100644
--- a/net/ipv4/netfilter/iptable_filter.c
+++ b/net/ipv4/netfilter/iptable_filter.c
@@ -93,13 +93,8 @@ ipt_local_out_hook(unsigned int hook,
 {
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
-	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk("iptable_filter: ignoring short SOCK_RAW "
-			       "packet.\n");
+	    ip_hdrlen(skb) < sizeof(struct iphdr))
 		return NF_ACCEPT;
-	}
-
 	return ipt_do_table(skb, hook, in, out,
 			    dev_net(out)->ipv4.iptable_filter);
 }
diff --git a/net/ipv4/netfilter/iptable_mangle.c b/net/ipv4/netfilter/iptable_mangle.c
index 69f2c42..3929d20 100644
--- a/net/ipv4/netfilter/iptable_mangle.c
+++ b/net/ipv4/netfilter/iptable_mangle.c
@@ -132,12 +132,8 @@ ipt_local_hook(unsigned int hook,
 
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr)
-	    || ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk("iptable_mangle: ignoring short SOCK_RAW "
-			       "packet.\n");
+	    || ip_hdrlen(skb) < sizeof(struct iphdr))
 		return NF_ACCEPT;
-	}
 
 	/* Save things which could affect route */
 	mark = skb->mark;
diff --git a/net/ipv4/netfilter/iptable_raw.c b/net/ipv4/netfilter/iptable_raw.c
index 8faebfe..7f65d18 100644
--- a/net/ipv4/netfilter/iptable_raw.c
+++ b/net/ipv4/netfilter/iptable_raw.c
@@ -65,12 +65,8 @@ ipt_local_hook(unsigned int hook,
 {
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
-	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk("iptable_raw: ignoring short SOCK_RAW "
-			       "packet.\n");
+	    ip_hdrlen(skb) < sizeof(struct iphdr))
 		return NF_ACCEPT;
-	}
 	return ipt_do_table(skb, hook, in, out,
 			    dev_net(out)->ipv4.iptable_raw);
 }
diff --git a/net/ipv4/netfilter/iptable_security.c b/net/ipv4/netfilter/iptable_security.c
index 36f3be3..a52a35f 100644
--- a/net/ipv4/netfilter/iptable_security.c
+++ b/net/ipv4/netfilter/iptable_security.c
@@ -96,12 +96,8 @@ ipt_local_out_hook(unsigned int hook,
 {
 	/* Somebody is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr)
-	    || ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk(KERN_INFO "iptable_security: ignoring short "
-			       "SOCK_RAW packet.\n");
+	    || ip_hdrlen(skb) < sizeof(struct iphdr))
 		return NF_ACCEPT;
-	}
 	return ipt_do_table(skb, hook, in, out,
 			    dev_net(out)->ipv4.iptable_security);
 }
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index b2141e1..4beb04f 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -145,11 +145,8 @@ static unsigned int ipv4_conntrack_local(unsigned int hooknum,
 {
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
-	    ip_hdrlen(skb) < sizeof(struct iphdr)) {
-		if (net_ratelimit())
-			printk("ipt_hook: happy cracking.\n");
+	    ip_hdrlen(skb) < sizeof(struct iphdr))
 		return NF_ACCEPT;
-	}
 	return nf_conntrack_in(dev_net(out), PF_INET, hooknum, skb);
 }