2008-12-11 22:13:59

by Adam Jackson

[permalink] [raw]
Subject: [PATCH] net: Remove a noisy printk

From: Dave Jones <[email protected]>

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

Signed-off-by: Dave Jones <[email protected]>
Signed-off-by: Adam Jackson <[email protected]>
---
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


2008-12-11 22:14:23

by Adam Jackson

[permalink] [raw]
Subject: [PATCH] ACPI: Silence blacklist warnings.

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 <[email protected]>
---
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

2008-12-11 22:14:41

by Adam Jackson

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

From: Peter Jones <[email protected]>

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 <[email protected]>
---
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

2008-12-11 22:15:02

by Adam Jackson

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

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 <[email protected]>
---
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

2008-12-11 22:25:39

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

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

I would suggest to send this to netdev.

Sam

2008-12-12 04:32:54

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

From: Adam Jackson <[email protected]>
Date: Thu, 11 Dec 2008 17:13:42 -0500

> From: Dave Jones <[email protected]>
>
> These messages are trivial to trigger when running stress tests
> like isic, and add no real value.
>
> Signed-off-by: Dave Jones <[email protected]>
> Signed-off-by: Adam Jackson <[email protected]>

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

2008-12-13 22:13:31

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk


On Friday 2008-12-12 05:32, David Miller wrote:
>From: Adam Jackson <[email protected]>
>Date: Thu, 11 Dec 2008 17:13:42 -0500
>
>> From: Dave Jones <[email protected]>
>>
>> These messages are trivial to trigger when running stress tests
>> like isic, and add no real value.
>>
>> Signed-off-by: Dave Jones <[email protected]>
>> Signed-off-by: Adam Jackson <[email protected]>
>
>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.

2008-12-14 17:42:25

by Jozsef Kadlecsik

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

On Sat, 13 Dec 2008, Jan Engelhardt wrote:

> On Friday 2008-12-12 05:32, David Miller wrote:
> >From: Adam Jackson <[email protected]>
> >Date: Thu, 11 Dec 2008 17:13:42 -0500
> >
> >> From: Dave Jones <[email protected]>
> >>
> >> These messages are trivial to trigger when running stress tests
> >> like isic, and add no real value.
> >>
> >> Signed-off-by: Dave Jones <[email protected]>
> >> Signed-off-by: Adam Jackson <[email protected]>
> >
> >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 : [email protected], [email protected]
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

2008-12-14 18:07:15

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk


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].

2008-12-14 20:15:36

by Jozsef Kadlecsik

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

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 : [email protected], [email protected]
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

2008-12-15 12:23:29

by Patrick McHardy

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

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.

2008-12-15 13:25:25

by Jozsef Kadlecsik

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

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 : [email protected], [email protected]
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

2008-12-15 13:33:16

by Patrick McHardy

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

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.

2008-12-16 19:43:18

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

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

2008-12-16 19:59:26

by Jozsef Kadlecsik

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

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 : [email protected], [email protected]
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

2008-12-16 20:00:32

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk


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;
}

2008-12-16 20:03:29

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk


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.

2008-12-19 01:22:21

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

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.

2008-12-17 08:26:42

by Jozsef Kadlecsik

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

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 <[email protected]>");
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 <[email protected]>");
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 : [email protected], [email protected]
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

2009-01-12 05:03:23

by Patrick McHardy

[permalink] [raw]
Subject: Re: [PATCH] net: Remove a noisy printk

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.


Attachments:
01.diff (3.52 kB)