| Message ID | 1360660958-29494-1-git-send-email-fw@strlen.de |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Feb 12, 2013 at 10:22:38AM +0100, Florian Westphal wrote: > Userspace can cause kernel panic by not specifying orig/reply > tuple: kernel will create a tuple with random stack values. > > Problem is that tuple.dst.dir will be random, too, which > causes nf_ct_tuplehash_to_ctrack() to return garbage. good catch. > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > net/netfilter/nf_conntrack_netlink.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c > index 7bbfb3d..e592281 100644 > --- a/net/netfilter/nf_conntrack_netlink.c > +++ b/net/netfilter/nf_conntrack_netlink.c > @@ -1596,6 +1596,9 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb, > if (nlh->nlmsg_flags & NLM_F_CREATE) { > enum ip_conntrack_events events; > > + if (!cda[CTA_TUPLE_ORIG] || > + !cda[CTA_TUPLE_REPLY]) > + return -EINVAL; I think that needs to be && instead of ||. No need to resend the patch, I'll fix it and apply. Thanks Florian. > ct = ctnetlink_create_conntrack(net, zone, cda, &otuple, > &rtuple, u3); > if (IS_ERR(ct)) > -- > 1.7.8.6 > > -- > To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Tue, Feb 12, 2013 at 10:22:38AM +0100, Florian Westphal wrote: > > Userspace can cause kernel panic by not specifying orig/reply > > tuple: kernel will create a tuple with random stack values. > > > > Problem is that tuple.dst.dir will be random, too, which > > causes nf_ct_tuplehash_to_ctrack() to return garbage. [..] > > diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c > > index 7bbfb3d..e592281 100644 > > --- a/net/netfilter/nf_conntrack_netlink.c > > +++ b/net/netfilter/nf_conntrack_netlink.c > > @@ -1596,6 +1596,9 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb, > > if (nlh->nlmsg_flags & NLM_F_CREATE) { > > enum ip_conntrack_events events; > > > > + if (!cda[CTA_TUPLE_ORIG] || > > + !cda[CTA_TUPLE_REPLY]) > > + return -EINVAL; > > I think that needs to be && instead of ||. Uh, maybe I'm too low on caffeine, but when a new conntrack is to be created, shouldn't userspace have to specify _both_ ORIG and REPLY? -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 12, 2013 at 01:43:28PM +0100, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > On Tue, Feb 12, 2013 at 10:22:38AM +0100, Florian Westphal wrote: > > > Userspace can cause kernel panic by not specifying orig/reply > > > tuple: kernel will create a tuple with random stack values. > > > > > > Problem is that tuple.dst.dir will be random, too, which > > > causes nf_ct_tuplehash_to_ctrack() to return garbage. > > [..] > > > > diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c > > > index 7bbfb3d..e592281 100644 > > > --- a/net/netfilter/nf_conntrack_netlink.c > > > +++ b/net/netfilter/nf_conntrack_netlink.c > > > @@ -1596,6 +1596,9 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb, > > > if (nlh->nlmsg_flags & NLM_F_CREATE) { > > > enum ip_conntrack_events events; > > > > > > + if (!cda[CTA_TUPLE_ORIG] || > > > + !cda[CTA_TUPLE_REPLY]) > > > + return -EINVAL; > > > > I think that needs to be && instead of ||. > > Uh, maybe I'm too low on caffeine, but when a new conntrack > is to be created, shouldn't userspace have to specify > _both_ ORIG and REPLY? You're right, it's me who is low on caffeine. Sorry. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 12, 2013 at 10:22:38AM +0100, Florian Westphal wrote: > Userspace can cause kernel panic by not specifying orig/reply > tuple: kernel will create a tuple with random stack values. > > Problem is that tuple.dst.dir will be random, too, which > causes nf_ct_tuplehash_to_ctrack() to return garbage. Applied, thanks Florian. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index 7bbfb3d..e592281 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -1596,6 +1596,9 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb, if (nlh->nlmsg_flags & NLM_F_CREATE) { enum ip_conntrack_events events; + if (!cda[CTA_TUPLE_ORIG] || + !cda[CTA_TUPLE_REPLY]) + return -EINVAL; ct = ctnetlink_create_conntrack(net, zone, cda, &otuple, &rtuple, u3); if (IS_ERR(ct))
Userspace can cause kernel panic by not specifying orig/reply tuple: kernel will create a tuple with random stack values. Problem is that tuple.dst.dir will be random, too, which causes nf_ct_tuplehash_to_ctrack() to return garbage. Signed-off-by: Florian Westphal <fw@strlen.de> --- net/netfilter/nf_conntrack_netlink.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)