-stable review patch. If anyone has any objections, please let us know.
------------------
This patch fixes an userspace triggered oops. If there is no ICMP_ID
info the reference to attr will be NULL.
Signed-off-by: Krzysztof Piotr Oledzki <[email protected]>
Signed-off-by: Pablo Neira Ayuso <[email protected]>
Signed-off-by: Harald Welte <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
net/ipv4/netfilter/ip_conntrack_proto_icmp.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
--- linux-2.6.14.2.orig/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
+++ linux-2.6.14.2/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
@@ -151,13 +151,13 @@ icmp_error_message(struct sk_buff *skb,
/* Not enough header? */
inside = skb_header_pointer(skb, skb->nh.iph->ihl*4, sizeof(_in), &_in);
if (inside == NULL)
- return NF_ACCEPT;
+ return -NF_ACCEPT;
/* Ignore ICMP's containing fragments (shouldn't happen) */
if (inside->ip.frag_off & htons(IP_OFFSET)) {
DEBUGP("icmp_error_track: fragment of proto %u\n",
inside->ip.protocol);
- return NF_ACCEPT;
+ return -NF_ACCEPT;
}
innerproto = ip_conntrack_proto_find_get(inside->ip.protocol);
@@ -166,7 +166,7 @@ icmp_error_message(struct sk_buff *skb,
if (!ip_ct_get_tuple(&inside->ip, skb, dataoff, &origtuple, innerproto)) {
DEBUGP("icmp_error: ! get_tuple p=%u", inside->ip.protocol);
ip_conntrack_proto_put(innerproto);
- return NF_ACCEPT;
+ return -NF_ACCEPT;
}
/* Ordinarily, we'd expect the inverted tupleproto, but it's
@@ -174,7 +174,7 @@ icmp_error_message(struct sk_buff *skb,
if (!ip_ct_invert_tuple(&innertuple, &origtuple, innerproto)) {
DEBUGP("icmp_error_track: Can't invert tuple\n");
ip_conntrack_proto_put(innerproto);
- return NF_ACCEPT;
+ return -NF_ACCEPT;
}
ip_conntrack_proto_put(innerproto);
@@ -190,7 +190,7 @@ icmp_error_message(struct sk_buff *skb,
if (!h) {
DEBUGP("icmp_error_track: no match\n");
- return NF_ACCEPT;
+ return -NF_ACCEPT;
}
/* Reverse direction from that found */
if (DIRECTION(h) != IP_CT_DIR_REPLY)
@@ -296,7 +296,8 @@ static int icmp_nfattr_to_tuple(struct n
struct ip_conntrack_tuple *tuple)
{
if (!tb[CTA_PROTO_ICMP_TYPE-1]
- || !tb[CTA_PROTO_ICMP_CODE-1])
+ || !tb[CTA_PROTO_ICMP_CODE-1]
+ || !tb[CTA_PROTO_ICMP_ID-1])
return -1;
tuple->dst.u.icmp.type =
--
On Tue, 22 Nov 2005, Chris Wright wrote:
> -stable review patch. If anyone has any objections, please let us know.
It seems we have two different patches here.
> ------------------
>
> This patch fixes an userspace triggered oops. If there is no ICMP_ID
> info the reference to attr will be NULL.
>
> Signed-off-by: Krzysztof Piotr Oledzki <[email protected]>
> Signed-off-by: Pablo Neira Ayuso <[email protected]>
> Signed-off-by: Harald Welte <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> net/ipv4/netfilter/ip_conntrack_proto_icmp.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
First - "stop tracking ICMP(v6) error at early point" by Yasuyuki Kozakai:
> --- linux-2.6.14.2.orig/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
> +++ linux-2.6.14.2/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
> @@ -151,13 +151,13 @@ icmp_error_message(struct sk_buff *skb,
> /* Not enough header? */
> inside = skb_header_pointer(skb, skb->nh.iph->ihl*4, sizeof(_in), &_in);
> if (inside == NULL)
> - return NF_ACCEPT;
> + return -NF_ACCEPT;
>
> /* Ignore ICMP's containing fragments (shouldn't happen) */
> if (inside->ip.frag_off & htons(IP_OFFSET)) {
> DEBUGP("icmp_error_track: fragment of proto %u\n",
> inside->ip.protocol);
> - return NF_ACCEPT;
> + return -NF_ACCEPT;
> }
>
> innerproto = ip_conntrack_proto_find_get(inside->ip.protocol);
> @@ -166,7 +166,7 @@ icmp_error_message(struct sk_buff *skb,
> if (!ip_ct_get_tuple(&inside->ip, skb, dataoff, &origtuple, innerproto)) {
> DEBUGP("icmp_error: ! get_tuple p=%u", inside->ip.protocol);
> ip_conntrack_proto_put(innerproto);
> - return NF_ACCEPT;
> + return -NF_ACCEPT;
> }
>
> /* Ordinarily, we'd expect the inverted tupleproto, but it's
> @@ -174,7 +174,7 @@ icmp_error_message(struct sk_buff *skb,
> if (!ip_ct_invert_tuple(&innertuple, &origtuple, innerproto)) {
> DEBUGP("icmp_error_track: Can't invert tuple\n");
> ip_conntrack_proto_put(innerproto);
> - return NF_ACCEPT;
> + return -NF_ACCEPT;
> }
> ip_conntrack_proto_put(innerproto);
>
> @@ -190,7 +190,7 @@ icmp_error_message(struct sk_buff *skb,
>
> if (!h) {
> DEBUGP("icmp_error_track: no match\n");
> - return NF_ACCEPT;
> + return -NF_ACCEPT;
> }
> /* Reverse direction from that found */
> if (DIRECTION(h) != IP_CT_DIR_REPLY)
Second - "Fix oops when no ICMP ID info in message":
> @@ -296,7 +296,8 @@ static int icmp_nfattr_to_tuple(struct n
> struct ip_conntrack_tuple *tuple)
> {
> if (!tb[CTA_PROTO_ICMP_TYPE-1]
> - || !tb[CTA_PROTO_ICMP_CODE-1])
> + || !tb[CTA_PROTO_ICMP_CODE-1]
> + || !tb[CTA_PROTO_ICMP_ID-1])
> return -1;
>
> tuple->dst.u.icmp.type =
>
Best regards,
Krzysztof Ol?dzki
* Krzysztof Oledzki ([email protected]) wrote:
> On Tue, 22 Nov 2005, Chris Wright wrote:
>
> >-stable review patch. If anyone has any objections, please let us know.
>
> It seems we have two different patches here.
Thanks, you are right. Harald, was that intentional?
thanks,
-chris
On Wed, Nov 23, 2005 at 12:31:55AM +0100, Krzysztof Oledzki wrote:
> On Tue, 22 Nov 2005, Chris Wright wrote:
>
> >-stable review patch. If anyone has any objections, please let us know.
>
> It seems we have two different patches here.
yes, it seems like two independent patches slipped into the one patch
that was submitted. I detected that error for mainline, but forgot that
the same patch was submitted for stable.
So the first part (as pointed out by Krzyzstof) is not a bugfix, but a
cosmetic fix.
I therefore request reverting this patch '13', and instead applying the version
below, the one that contains only the real fix (as indicated in the
changelog)
Sorry once again.
[NETFILTER] ctnetlink: Fix oops when no ICMP ID info in message
This patch fixes an userspace triggered oops. If there is no ICMP_ID
info the reference to attr will be NULL.
Signed-off-by: Krzysztof Piotr Oledzki <[email protected]>
Signed-off-by: Pablo Neira Ayuso <[email protected]>
Signed-off-by: Harald Welte <[email protected]>
---
commit 922474105255d7791128688c8e60bb27a8eadf1d
tree b072448bfe0b79058b03ed798a1145ad1a7c6397
parent 723cb15b48e5510094296a9fc240d69a3acae95c
author Krzysztof Piotr Oledzki <[email protected]> Tue, 15 Nov 2005 12:16:43 +0100
committer Harald Welte <[email protected]> Tue, 15 Nov 2005 12:16:43 +0100
net/ipv4/netfilter/ip_conntrack_proto_icmp.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/netfilter/ip_conntrack_proto_icmp.c b/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
+++ b/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
@@ -296,7 +296,8 @@ static int icmp_nfattr_to_tuple(struct n
struct ip_conntrack_tuple *tuple)
{
if (!tb[CTA_PROTO_ICMP_TYPE-1]
- || !tb[CTA_PROTO_ICMP_CODE-1])
+ || !tb[CTA_PROTO_ICMP_CODE-1]
+ || !tb[CTA_PROTO_ICMP_ID-1])
return -1;
tuple->dst.u.icmp.type =
--
- Harald Welte <[email protected]> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
* Harald Welte ([email protected]) wrote:
> On Wed, Nov 23, 2005 at 12:31:55AM +0100, Krzysztof Oledzki wrote:
> > On Tue, 22 Nov 2005, Chris Wright wrote:
> >
> > >-stable review patch. If anyone has any objections, please let us know.
> >
> > It seems we have two different patches here.
>
> yes, it seems like two independent patches slipped into the one patch
> that was submitted. I detected that error for mainline, but forgot that
> the same patch was submitted for stable.
>
> So the first part (as pointed out by Krzyzstof) is not a bugfix, but a
> cosmetic fix.
>
> I therefore request reverting this patch '13', and instead applying the version
> below, the one that contains only the real fix (as indicated in the
> changelog)
Thanks Harald, I dropped the old patch and replaced with this one.
thanks,
-chris