2001-11-28 21:38:55

by Rolf Fokkens

[permalink] [raw]
Subject: [PATCH] vanilla 2.4.15 iptables/REDIRECT kernel oops

On Wednesday 28 November 2001 07:52, Stephan von Krawczynski wrote:
> The safe patch would look like this:
>
...
>
> Can you check out?
>
I took your patch, and added an obvious change to ip_output.c as well. It's
below. The patch seems to work fine on my PC, will try tomorrow on the server
that had the problems initially.

Still don't understand why skb->sk suddenly is NULL after nf_hook_slow. Maybe
the gurus finally pay attention to this (after all, there's a patch now!!)
and look into this.

Rolf

diff -ruN linux/include/net/ip.h.orig linux/include/net/ip.h
--- linux/include/net/ip.h.orig? Wed Nov 28 20:40:33 2001
+++ linux/include/net/ip.h?????? Wed Nov 28 20:47:20 2001
@@?-181,9 +181,9 @@
?static inline
?int ip_dont_fragment(struct sock *sk, struct dst_entry *dst)
?{
-??????? return (sk->protinfo.af_inet.pmtudisc == IP_PMTUDISC_DO ||
-??????? ??????? (sk->protinfo.af_inet.pmtudisc == IP_PMTUDISC_WANT &&
-??????? ??????? !(dst->mxlock&(1<<RTAX_MTU))));
+??????? return (sk && (sk->protinfo.af_inet.pmtudisc == IP_PMTUDISC_DO ||
+??????? ??????? ? ? ?(sk->protinfo.af_inet.pmtudisc == IP_PMTUDISC_WANT &&
+??????? ??????? ? ? ?!(dst->mxlock&(1<<RTAX_MTU)))));
?}
?
?extern void __ip_select_ident(struct iphdr *iph, struct dst_entry *dst);
diff -ruN linux/net/ipv4/ip_output.c.orig linux/net/ipv4/ip_output.c
--- linux/net/ipv4/ip_output.c.orig????? Wed Nov 28 20:51:02 2001
+++ linux/net/ipv4/ip_output.c?? Wed Nov 28 20:52:31 2001
@@?-315,7 +315,9 @@
???????? /* Add an IP checksum. */
???????? ip_send_check(iph);
?
-??????? skb->priority = sk->priority;
+??????? if (sk)
+??????? ??????? skb->priority = sk->priority;
+
???????? return skb->dst->output(skb);
?
?fragment:

> Can any ip-stack guru comment?
>
> Regards,
> Stephan
>
> PS: ip_select_ident looks clean in terms of sk==NULL, BTW.
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


2001-11-30 18:40:41

by Rolf Fokkens

[permalink] [raw]
Subject: Re: [PATCH] vanilla 2.4.15 iptables/REDIRECT kernel oops

On Friday 30 November 2001 06:13, Stephan von Krawczynski wrote:
> we have a problem: I talked to David Miller (one of the network
> maintainers), and he told me, that sk _must_ be non NULL in this code. He
> states netfilter is broken.
> So our patch won't make it. We have to investigate the cause of sk=NULL.
> Can you describe a possible test-setup?

I think I know what may have caused the problem: I patch from Rusty Russell
that I forgot about. I use RPM to build kernels, and I forgot to deactivate
this patch. So it wasn't a vanilla kernel.

Stuped me!

Sorry about the confusion. I'll forward you the patch, just for your
information:

Rolf

On Mon, 29 Oct 2001 21:31:57 -0800 (PST)
"David S. Miller" <[email protected]> wrote:

>? ? From: Rusty Russell <[email protected]>
>? ? Date: Tue, 30 Oct 2001 15:28:12 +1100
>? ?
>? ? should the NAT layer be doing skb_unshare() before altering the packet?
>?
>?I think it should.


Agreed. ?The 2.2 masq code didn't do this, and hence the "don't tcpdump on
masq host"
recommendation.

Please try this patch (compiles at least),
Rusty.

diff -urN -I \$.*\$ --exclude TAGS -X
/home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal
linux-2.4.13-official/net/ipv4/netfilter/ip_fw_compat.c
working-2.4.13-nfunshare/net/ipv4/netfilter/ip_fw_compat.c
--- linux-2.4.13-official/net/ipv4/netfilter/ip_fw_compat.c????? Sat Apr 28
07:15:01 2001
+++ working-2.4.13-nfunshare/net/ipv4/netfilter/ip_fw_compat.c?? Wed Oct 31
17:05:53 2001
@@?-78,11 +78,19 @@
?{
???????? int ret = FW_BLOCK;
???????? u_int16_t redirpt;
+??????? struct sk_buff *nskb;
?
???????? /* Assume worse case: any hook could change packet */
???????? (*pskb)->nfcache |= NFC_UNKNOWN | NFC_ALTERED;
???????? if ((*pskb)->ip_summed == CHECKSUM_HW)
???????? ??????? (*pskb)->ip_summed = CHECKSUM_NONE;
+
+??????? /* Firewall rules can alter TOS: raw socket may have clone of
+ ? ? ? ? ? skb: don't disturb it --RR */
+??????? nskb = skb_unshare(*pskb, GFP_ATOMIC);
+??????? if (!nskb)
+??????? ??????? return NF_DROP;
+??????? *pskb = nskb;
?
???????? switch (hooknum) {
???????? case NF_IP_PRE_ROUTING:
diff -urN -I \$.*\$ --exclude TAGS -X
/home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal
linux-2.4.13-official/net/ipv4/netfilter/ip_nat_core.c
working-2.4.13-nfunshare/net/ipv4/netfilter/ip_nat_core.c
--- linux-2.4.13-official/net/ipv4/netfilter/ip_nat_core.c?????? Thu May 17
03:31:27 2001
+++ working-2.4.13-nfunshare/net/ipv4/netfilter/ip_nat_core.c??? Wed Oct 31
16:52:06 2001
@@?-734,6 +734,15 @@
???????? ? synchronize_bh()) can vanish. */
???????? READ_LOCK(&ip_nat_lock);
???????? for (i = 0; i < info->num_manips; i++) {
+??????? ??????? struct sk_buff *nskb;
+??????? ??????? /* raw socket may have clone of skb: don't disturb it --RR */
+??????? ??????? nskb = skb_unshare(*pskb, GFP_ATOMIC);
+??????? ??????? if (!nskb) {
+??????? ??????? ??????? READ_UNLOCK(&ip_nat_lock);
+??????? ??????? ??????? return NF_DROP;
+??????? ??????? }
+??????? ??????? *pskb = nskb;
+
???????? ??????? if (info->manips[i].direction == dir
???????? ??????? ? ?&& info->manips[i].hooknum == hooknum) {
???????? ??????? ??????? DEBUGP("Mangling %p: %s to %u.%u.%u.%u %u\n",
diff -urN -I \$.*\$ --exclude TAGS -X
/home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal
linux-2.4.13-official/net/ipv4/netfilter/ipt_TCPMSS.c
working-2.4.13-nfunshare/net/ipv4/netfilter/ipt_TCPMSS.c
--- linux-2.4.13-official/net/ipv4/netfilter/ipt_TCPMSS.c??????? Mon Oct ?1
05:26:08 2001
+++ working-2.4.13-nfunshare/net/ipv4/netfilter/ipt_TCPMSS.c???? Wed Oct 31
17:00:42 2001
@@?-48,6 +48,13 @@
???????? u_int16_t tcplen, newtotlen, oldval, newmss;
???????? unsigned int i;
???????? u_int8_t *opt;
+??????? struct sk_buff *nskb;
+
+??????? /* raw socket may have clone of skb: don't disturb it --RR */
+??????? nskb = skb_unshare(*pskb, GFP_ATOMIC);
+??????? if (!nskb)
+??????? ??????? return NF_DROP;
+??????? *pskb = nskb;
?
???????? tcplen = (*pskb)->len - iph->ihl*4;
?
diff -urN -I \$.*\$ --exclude TAGS -X
/home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal
linux-2.4.13-official/net/ipv4/netfilter/ipt_TOS.c
working-2.4.13-nfunshare/net/ipv4/netfilter/ipt_TOS.c
--- linux-2.4.13-official/net/ipv4/netfilter/ipt_TOS.c?? Mon Oct ?1 05:26:08
2001
+++ working-2.4.13-nfunshare/net/ipv4/netfilter/ipt_TOS.c??????? Wed Oct 31
17:03:11 2001
@@?-19,7 +19,14 @@
???????? const struct ipt_tos_target_info *tosinfo = targinfo;
?
???????? if ((iph->tos & IPTOS_TOS_MASK) != tosinfo->tos) {
+??????? ??????? struct sk_buff *nskb;
???????? ??????? u_int16_t diffs[2];
+
+??????? ??????? /* raw socket may have clone of skb: don't disturb it --RR */
+??????? ??????? nskb = skb_unshare(*pskb, GFP_ATOMIC);
+??????? ??????? if (!nskb)
+??????? ??????? ??????? return NF_DROP;
+??????? ??????? *pskb = nskb;
?
???????? ??????? diffs[0] = htons(iph->tos) ^ 0xFFFF;
???????? ??????? iph->tos = (iph->tos & IPTOS_PREC_MASK) | tosinfo->tos;