| Message ID | 20190306073304.GA22426@myunghoj-Precision-5530 |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | David Miller |
| Headers | show |
| Series | xfrm: Reset secpath in xfrm failure | expand |
On Wed, Mar 06, 2019 at 04:33:08PM +0900, Myungho Jung wrote: > In esp4_gro_receive() and esp6_gro_receive(), secpath can be allocated > without adding xfrm state to xvec. Then, sp->xvec[sp->len - 1] would > fail and result in dereferencing invalid pointer in esp4_gso_segment() > and esp6_gso_segment(). Reset secpath if xfrm function returns error. > > Reported-by: syzbot+b69368fd933c6c592f4c@syzkaller.appspotmail.com > Signed-off-by: Myungho Jung <mhjungk@gmail.com> The patch itself looks ok, but please add a 'Fixes' tag to the commit message. Thanks!
On Wed, Mar 06, 2019 at 12:35:43PM +0100, Steffen Klassert wrote: > On Wed, Mar 06, 2019 at 04:33:08PM +0900, Myungho Jung wrote: > > In esp4_gro_receive() and esp6_gro_receive(), secpath can be allocated > > without adding xfrm state to xvec. Then, sp->xvec[sp->len - 1] would > > fail and result in dereferencing invalid pointer in esp4_gso_segment() > > and esp6_gso_segment(). Reset secpath if xfrm function returns error. > > > > Reported-by: syzbot+b69368fd933c6c592f4c@syzkaller.appspotmail.com > > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > > The patch itself looks ok, but please add a 'Fixes' tag to > the commit message. > > Thanks! Hi Steffen, Thanks for reviewing the change. I'll add fixes tag and resubmit it. Thanks, Myungho
diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c index 8756e0e790d2..7329e40c73f6 100644 --- a/net/ipv4/esp4_offload.c +++ b/net/ipv4/esp4_offload.c @@ -51,14 +51,18 @@ static struct sk_buff *esp4_gro_receive(struct list_head *head, if (!sp) goto out; - if (sp->len == XFRM_MAX_DEPTH) + if (sp->len == XFRM_MAX_DEPTH) { + secpath_reset(skb); goto out; + } x = xfrm_state_lookup(dev_net(skb->dev), skb->mark, (xfrm_address_t *)&ip_hdr(skb)->daddr, spi, IPPROTO_ESP, AF_INET); - if (!x) + if (!x) { + secpath_reset(skb); goto out; + } sp->xvec[sp->len++] = x; sp->olen++; @@ -66,6 +70,7 @@ static struct sk_buff *esp4_gro_receive(struct list_head *head, xo = xfrm_offload(skb); if (!xo) { xfrm_state_put(x); + secpath_reset(skb); goto out; } } diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c index d46b4eb645c2..399c688d192e 100644 --- a/net/ipv6/esp6_offload.c +++ b/net/ipv6/esp6_offload.c @@ -73,14 +73,18 @@ static struct sk_buff *esp6_gro_receive(struct list_head *head, if (!sp) goto out; - if (sp->len == XFRM_MAX_DEPTH) + if (sp->len == XFRM_MAX_DEPTH) { + secpath_reset(skb); goto out; + } x = xfrm_state_lookup(dev_net(skb->dev), skb->mark, (xfrm_address_t *)&ipv6_hdr(skb)->daddr, spi, IPPROTO_ESP, AF_INET6); - if (!x) + if (!x) { + secpath_reset(skb); goto out; + } sp->xvec[sp->len++] = x; sp->olen++; @@ -88,6 +92,7 @@ static struct sk_buff *esp6_gro_receive(struct list_head *head, xo = xfrm_offload(skb); if (!xo) { xfrm_state_put(x); + secpath_reset(skb); goto out; } }
In esp4_gro_receive() and esp6_gro_receive(), secpath can be allocated without adding xfrm state to xvec. Then, sp->xvec[sp->len - 1] would fail and result in dereferencing invalid pointer in esp4_gso_segment() and esp6_gso_segment(). Reset secpath if xfrm function returns error. Reported-by: syzbot+b69368fd933c6c592f4c@syzkaller.appspotmail.com Signed-off-by: Myungho Jung <mhjungk@gmail.com> --- net/ipv4/esp4_offload.c | 9 +++++++-- net/ipv6/esp6_offload.c | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-)