Re: [patch 13/23] [PATCH] [NETFILTER] ctnetlink: Fix oops when noICMP ID info in message

[Krzysztof Oledzki]

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 <ole@xxxxxx>
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> Signed-off-by: Harald Welte <laforge@xxxxxxxxxxxxx>
> Signed-off-by: Chris Wright <chrisw@xxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
> ---
> 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