| Message ID | 20171023061559.GA25493@tejaswit-linux.qualcomm.com |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | netfilter: ipv4: Fix use-after-free in send_reset | expand |
On Mon, Oct 23, 2017 at 11:46:03AM +0530, Tejaswi Tanikella wrote: > niph is not updated after pskb_expand_head changes the skb head. > It still points to the freed data, which is then used to update > tot_len and checksum. This could cause use-after-free poison crash. > > Update niph, if ip_route_me_harder does not fail. > > Signed-off-by: Tejaswi Tanikella <tejaswit@codeaurora.org> > --- > net/ipv4/netfilter/nf_reject_ipv4.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c > index eeacbda..b2e0f90 100644 > --- a/net/ipv4/netfilter/nf_reject_ipv4.c > +++ b/net/ipv4/netfilter/nf_reject_ipv4.c > @@ -131,6 +131,8 @@ void nf_send_reset(struct net *net, struct sk_buff *oldskb, int hook) > > if (ip_route_me_harder(net, nskb, RTN_UNSPEC)) > goto free_nskb; > + else > + niph = ip_hdr(nskb); No need for else here, right? So we can just turn this into a one liner to inconditionally reload this pointer. > /* "Never happens" */ > if (nskb->len > dst_mtu(skb_dst(nskb))) > -- > 1.9.1 > -- 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 Wed, Nov 01, 2017 at 01:36:36AM +0100, Pablo Neira Ayuso wrote: > On Mon, Oct 23, 2017 at 11:46:03AM +0530, Tejaswi Tanikella wrote: > > niph is not updated after pskb_expand_head changes the skb head. > > It still points to the freed data, which is then used to update > > tot_len and checksum. This could cause use-after-free poison crash. > > > > Update niph, if ip_route_me_harder does not fail. > > > > Signed-off-by: Tejaswi Tanikella <tejaswit@codeaurora.org> > > --- > > net/ipv4/netfilter/nf_reject_ipv4.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c > > index eeacbda..b2e0f90 100644 > > --- a/net/ipv4/netfilter/nf_reject_ipv4.c > > +++ b/net/ipv4/netfilter/nf_reject_ipv4.c > > @@ -131,6 +131,8 @@ void nf_send_reset(struct net *net, struct sk_buff *oldskb, int hook) > > > > if (ip_route_me_harder(net, nskb, RTN_UNSPEC)) > > goto free_nskb; > > + else > > + niph = ip_hdr(nskb); > > No need for else here, right? So we can just turn this into a one > liner to inconditionally reload this pointer. I'm just going to remove the 'else' and push out this. -- 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/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c index eeacbda..b2e0f90 100644 --- a/net/ipv4/netfilter/nf_reject_ipv4.c +++ b/net/ipv4/netfilter/nf_reject_ipv4.c @@ -131,6 +131,8 @@ void nf_send_reset(struct net *net, struct sk_buff *oldskb, int hook) if (ip_route_me_harder(net, nskb, RTN_UNSPEC)) goto free_nskb; + else + niph = ip_hdr(nskb); /* "Never happens" */ if (nskb->len > dst_mtu(skb_dst(nskb)))
niph is not updated after pskb_expand_head changes the skb head. It still points to the freed data, which is then used to update tot_len and checksum. This could cause use-after-free poison crash. Update niph, if ip_route_me_harder does not fail. Signed-off-by: Tejaswi Tanikella <tejaswit@codeaurora.org> --- net/ipv4/netfilter/nf_reject_ipv4.c | 2 ++ 1 file changed, 2 insertions(+)