Following is an attempt to backport fix of CVE-2024-44986 back to stable 5.4 and 5.10. 3 extra pre-requisite patches were required to introduce the skb_expand_head() function and use it in ip6_finish_output2() for the fix patch to be applicable.
Eric Dumazet (1): ipv6: fix possible UAF in ip6_finish_output2()
Vasily Averin (3): skbuff: introduce skb_expand_head() ipv6: use skb_expand_head in ip6_finish_output2 ipv6: use skb_expand_head in ip6_xmit
include/linux/skbuff.h | 1 + net/core/skbuff.c | 42 ++++++++++++++++++++++ net/ipv6/ip6_output.c | 82 ++++++++++++++++-------------------------- 3 files changed, 74 insertions(+), 51 deletions(-)
From: Vasily Averin vvs@virtuozzo.com
[ Upstream commit f1260ff15a71b8fc122b2c9abd8a7abffb6e0168 ]
Like skb_realloc_headroom(), new helper increases headroom of specified skb. Unlike skb_realloc_headroom(), it does not allocate a new skb if possible; copies skb->sk on new skb when as needed and frees original skb in case of failures.
This helps to simplify ip[6]_finish_output2() and a few other similar cases.
Signed-off-by: Vasily Averin vvs@virtuozzo.com Signed-off-by: David S. Miller davem@davemloft.net (cherry picked from commit f1260ff15a71b8fc122b2c9abd8a7abffb6e0168) Signed-off-by: Harshvardhan Jha harshvardhan.j.jha@oracle.com --- include/linux/skbuff.h | 1 + net/core/skbuff.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 31ae4b74d4352..3248e4aeec037 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1166,6 +1166,7 @@ static inline struct sk_buff *__pskb_copy(struct sk_buff *skb, int headroom, int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail, gfp_t gfp_mask); struct sk_buff *skb_realloc_headroom(struct sk_buff *skb, unsigned int headroom); +struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom); struct sk_buff *skb_copy_expand(const struct sk_buff *skb, int newheadroom, int newtailroom, gfp_t priority); int __must_check skb_to_sgvec_nomark(struct sk_buff *skb, struct scatterlist *sg, diff --git a/net/core/skbuff.c b/net/core/skbuff.c index b0c2d6f018003..fa3ea287d6ecc 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -1732,6 +1732,48 @@ struct sk_buff *skb_realloc_headroom(struct sk_buff *skb, unsigned int headroom) } EXPORT_SYMBOL(skb_realloc_headroom);
+/** + * skb_expand_head - reallocate header of &sk_buff + * @skb: buffer to reallocate + * @headroom: needed headroom + * + * Unlike skb_realloc_headroom, this one does not allocate a new skb + * if possible; copies skb->sk to new skb as needed + * and frees original skb in case of failures. + * + * It expect increased headroom and generates warning otherwise. + */ + +struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom) +{ + int delta = headroom - skb_headroom(skb); + + if (WARN_ONCE(delta <= 0, + "%s is expecting an increase in the headroom", __func__)) + return skb; + + /* pskb_expand_head() might crash, if skb is shared */ + if (skb_shared(skb)) { + struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); + + if (likely(nskb)) { + if (skb->sk) + skb_set_owner_w(nskb, skb->sk); + consume_skb(skb); + } else { + kfree_skb(skb); + } + skb = nskb; + } + if (skb && + pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) { + kfree_skb(skb); + skb = NULL; + } + return skb; +} +EXPORT_SYMBOL(skb_expand_head); + /** * skb_copy_expand - copy and expand sk_buff * @skb: buffer to copy
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: f1260ff15a71b8fc122b2c9abd8a7abffb6e0168
WARNING: Author mismatch between patch and upstream commit: Backport author: Harshvardhan Jha harshvardhan.j.jha@oracle.com Commit author: Vasily Averin vvs@virtuozzo.com
Status in newer kernel trees: 6.12.y | Present (exact SHA1) 6.6.y | Present (exact SHA1) 6.1.y | Present (exact SHA1) 5.15.y | Present (exact SHA1) 5.10.y | Not found
Note: The patch differs from the upstream commit: --- 1: f1260ff15a71 ! 1: ee7ccf9396c7 skbuff: introduce skb_expand_head() @@ Metadata ## Commit message ## skbuff: introduce skb_expand_head()
+ [ Upstream commit f1260ff15a71b8fc122b2c9abd8a7abffb6e0168 ] + Like skb_realloc_headroom(), new helper increases headroom of specified skb. Unlike skb_realloc_headroom(), it does not allocate a new skb if possible; copies skb->sk on new skb when as needed and frees original skb in case @@ Commit message
Signed-off-by: Vasily Averin vvs@virtuozzo.com Signed-off-by: David S. Miller davem@davemloft.net + (cherry picked from commit f1260ff15a71b8fc122b2c9abd8a7abffb6e0168) + Signed-off-by: Harshvardhan Jha harshvardhan.j.jha@oracle.com
## include/linux/skbuff.h ## @@ include/linux/skbuff.h: static inline struct sk_buff *__pskb_copy(struct sk_buff *skb, int headroom, ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-5.10.y | Success | Success | | stable/linux-5.4.y | Success | Success |
Hi,
On 26/12/24 6:51 AM, Sasha Levin wrote:
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: f1260ff15a71b8fc122b2c9abd8a7abffb6e0168
WARNING: Author mismatch between patch and upstream commit: Backport author: Harshvardhan Jha harshvardhan.j.jha@oracle.com Commit author: Vasily Averin vvs@virtuozzo.com
The only difference I see is that this patch has my signed off by? As seen below, the tests also pass. Can you please tell me what is needed to rectify this warning?
Status in newer kernel trees: 6.12.y | Present (exact SHA1) 6.6.y | Present (exact SHA1) 6.1.y | Present (exact SHA1) 5.15.y | Present (exact SHA1) 5.10.y | Not found
Note: The patch differs from the upstream commit:
1: f1260ff15a71 ! 1: ee7ccf9396c7 skbuff: introduce skb_expand_head() @@ Metadata ## Commit message ## skbuff: introduce skb_expand_head() + [ Upstream commit f1260ff15a71b8fc122b2c9abd8a7abffb6e0168 ] + Like skb_realloc_headroom(), new helper increases headroom of specified skb. Unlike skb_realloc_headroom(), it does not allocate a new skb if possible; copies skb->sk on new skb when as needed and frees original skb in case @@ Commit message Signed-off-by: Vasily Averin vvs@virtuozzo.com Signed-off-by: David S. Miller davem@davemloft.net + (cherry picked from commit f1260ff15a71b8fc122b2c9abd8a7abffb6e0168) + Signed-off-by: Harshvardhan Jha harshvardhan.j.jha@oracle.com ## include/linux/skbuff.h ## @@ include/linux/skbuff.h: static inline struct sk_buff *__pskb_copy(struct sk_buff *skb, int headroom,
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-5.10.y | Success | Success | | stable/linux-5.4.y | Success | Success |
Thanks & Regards, Harshvardhan
On Thu, Dec 26, 2024 at 06:14:10PM +0530, Harshvardhan Jha wrote:
Hi,
On 26/12/24 6:51 AM, Sasha Levin wrote:
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: f1260ff15a71b8fc122b2c9abd8a7abffb6e0168
WARNING: Author mismatch between patch and upstream commit: Backport author: Harshvardhan Jha harshvardhan.j.jha@oracle.com Commit author: Vasily Averin vvs@virtuozzo.com
The only difference I see is that this patch has my signed off by? As seen below, the tests also pass. Can you please tell me what is needed to rectify this warning?
Nothing to do in this case, this will just make our review easier.
Thanks!
From: Vasily Averin vvs@virtuozzo.com
[ Upstream commit e415ed3a4b8b246ee5e9d109ff5153efcf96b9f2 ]
Unlike skb_realloc_headroom, new helper skb_expand_head does not allocate a new skb if possible.
Additionally this patch replaces commonly used dereferencing with variables.
Signed-off-by: Vasily Averin vvs@virtuozzo.com Signed-off-by: David S. Miller davem@davemloft.net (cherry picked from commit e415ed3a4b8b246ee5e9d109ff5153efcf96b9f2) Signed-off-by: Harshvardhan Jha harshvardhan.j.jha@oracle.com --- net/ipv6/ip6_output.c | 51 ++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 35 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 26d8105981e96..7806963b4539e 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -60,46 +60,29 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff * { struct dst_entry *dst = skb_dst(skb); struct net_device *dev = dst->dev; + struct inet6_dev *idev = ip6_dst_idev(dst); unsigned int hh_len = LL_RESERVED_SPACE(dev); - int delta = hh_len - skb_headroom(skb); - const struct in6_addr *nexthop; + const struct in6_addr *daddr, *nexthop; + struct ipv6hdr *hdr; struct neighbour *neigh; int ret;
/* Be paranoid, rather than too clever. */ - if (unlikely(delta > 0) && dev->header_ops) { - /* pskb_expand_head() might crash, if skb is shared */ - if (skb_shared(skb)) { - struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); - - if (likely(nskb)) { - if (skb->sk) - skb_set_owner_w(nskb, skb->sk); - consume_skb(skb); - } else { - kfree_skb(skb); - } - skb = nskb; - } - if (skb && - pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) { - kfree_skb(skb); - skb = NULL; - } + if (unlikely(hh_len > skb_headroom(skb)) && dev->header_ops) { + skb = skb_expand_head(skb, hh_len); if (!skb) { - IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTDISCARDS); + IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS); return -ENOMEM; } }
- if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr)) { - struct inet6_dev *idev = ip6_dst_idev(skb_dst(skb)); - + hdr = ipv6_hdr(skb); + daddr = &hdr->daddr; + if (ipv6_addr_is_multicast(daddr)) { if (!(dev->flags & IFF_LOOPBACK) && sk_mc_loop(sk) && ((mroute6_is_socket(net, skb) && !(IP6CB(skb)->flags & IP6SKB_FORWARDED)) || - ipv6_chk_mcast_addr(dev, &ipv6_hdr(skb)->daddr, - &ipv6_hdr(skb)->saddr))) { + ipv6_chk_mcast_addr(dev, daddr, &hdr->saddr))) { struct sk_buff *newskb = skb_clone(skb, GFP_ATOMIC);
/* Do not check for IFF_ALLMULTI; multicast routing @@ -110,7 +93,7 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff * net, sk, newskb, NULL, newskb->dev, dev_loopback_xmit);
- if (ipv6_hdr(skb)->hop_limit == 0) { + if (hdr->hop_limit == 0) { IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS); kfree_skb(skb); @@ -119,9 +102,7 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff * }
IP6_UPD_PO_STATS(net, idev, IPSTATS_MIB_OUTMCAST, skb->len); - - if (IPV6_ADDR_MC_SCOPE(&ipv6_hdr(skb)->daddr) <= - IPV6_ADDR_SCOPE_NODELOCAL && + if (IPV6_ADDR_MC_SCOPE(daddr) <= IPV6_ADDR_SCOPE_NODELOCAL && !(dev->flags & IFF_LOOPBACK)) { kfree_skb(skb); return 0; @@ -136,10 +117,10 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff * }
rcu_read_lock_bh(); - nexthop = rt6_nexthop((struct rt6_info *)dst, &ipv6_hdr(skb)->daddr); - neigh = __ipv6_neigh_lookup_noref(dst->dev, nexthop); + nexthop = rt6_nexthop((struct rt6_info *)dst, daddr); + neigh = __ipv6_neigh_lookup_noref(dev, nexthop); if (unlikely(!neigh)) - neigh = __neigh_create(&nd_tbl, nexthop, dst->dev, false); + neigh = __neigh_create(&nd_tbl, nexthop, dev, false); if (!IS_ERR(neigh)) { sock_confirm_neigh(skb, neigh); ret = neigh_output(neigh, skb, false); @@ -148,7 +129,7 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff * } rcu_read_unlock_bh();
- IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTNOROUTES); + IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTNOROUTES); kfree_skb(skb); return -EINVAL; }
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: e415ed3a4b8b246ee5e9d109ff5153efcf96b9f2
WARNING: Author mismatch between patch and upstream commit: Backport author: Harshvardhan Jha harshvardhan.j.jha@oracle.com Commit author: Vasily Averin vvs@virtuozzo.com
Status in newer kernel trees: 6.12.y | Present (exact SHA1) 6.6.y | Present (exact SHA1) 6.1.y | Present (exact SHA1) 5.15.y | Present (exact SHA1) 5.10.y | Not found
Note: The patch differs from the upstream commit: --- 1: e415ed3a4b8b ! 1: 5d647c73654a ipv6: use skb_expand_head in ip6_finish_output2 @@ Metadata ## Commit message ## ipv6: use skb_expand_head in ip6_finish_output2
+ [ Upstream commit e415ed3a4b8b246ee5e9d109ff5153efcf96b9f2 ] + Unlike skb_realloc_headroom, new helper skb_expand_head does not allocate a new skb if possible.
@@ Commit message
Signed-off-by: Vasily Averin vvs@virtuozzo.com Signed-off-by: David S. Miller davem@davemloft.net + (cherry picked from commit e415ed3a4b8b246ee5e9d109ff5153efcf96b9f2) + Signed-off-by: Harshvardhan Jha harshvardhan.j.jha@oracle.com
## net/ipv6/ip6_output.c ## @@ net/ipv6/ip6_output.c: static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff * ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-5.10.y | Success | Success | | stable/linux-5.4.y | Success | Success |
From: Vasily Averin vvs@virtuozzo.com
[ Upstream commit 0c9f227bee11910a49e1d159abe102d06e3745d5 ]
Unlike skb_realloc_headroom, new helper skb_expand_head does not allocate a new skb if possible.
Additionally this patch replaces commonly used dereferencing with variables.
Signed-off-by: Vasily Averin vvs@virtuozzo.com Signed-off-by: David S. Miller davem@davemloft.net (cherry picked from commit 0c9f227bee11910a49e1d159abe102d06e3745d5) Signed-off-by: Harshvardhan Jha harshvardhan.j.jha@oracle.com --- net/ipv6/ip6_output.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 7806963b4539e..a8475848d0382 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -254,6 +254,8 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6, const struct ipv6_pinfo *np = inet6_sk(sk); struct in6_addr *first_hop = &fl6->daddr; struct dst_entry *dst = skb_dst(skb); + struct net_device *dev = dst->dev; + struct inet6_dev *idev = ip6_dst_idev(dst); unsigned int head_room; struct ipv6hdr *hdr; u8 proto = fl6->flowi6_proto; @@ -261,22 +263,16 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6, int hlimit = -1; u32 mtu;
- head_room = sizeof(struct ipv6hdr) + LL_RESERVED_SPACE(dst->dev); + head_room = sizeof(struct ipv6hdr) + LL_RESERVED_SPACE(dev); if (opt) head_room += opt->opt_nflen + opt->opt_flen;
- if (unlikely(skb_headroom(skb) < head_room)) { - struct sk_buff *skb2 = skb_realloc_headroom(skb, head_room); - if (!skb2) { - IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)), - IPSTATS_MIB_OUTDISCARDS); - kfree_skb(skb); + if (unlikely(head_room > skb_headroom(skb))) { + skb = skb_expand_head(skb, head_room); + if (!skb) { + IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS); return -ENOBUFS; } - if (skb->sk) - skb_set_owner_w(skb2, skb->sk); - consume_skb(skb); - skb = skb2; }
if (opt) { @@ -318,8 +314,7 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
mtu = dst_mtu(dst); if ((skb->len <= mtu) || skb->ignore_df || skb_is_gso(skb)) { - IP6_UPD_PO_STATS(net, ip6_dst_idev(skb_dst(skb)), - IPSTATS_MIB_OUT, skb->len); + IP6_UPD_PO_STATS(net, idev, IPSTATS_MIB_OUT, skb->len);
/* if egress device is enslaved to an L3 master device pass the * skb to its handler for processing @@ -332,17 +327,17 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6, * we promote our socket to non const */ return NF_HOOK(NFPROTO_IPV6, NF_INET_LOCAL_OUT, - net, (struct sock *)sk, skb, NULL, dst->dev, + net, (struct sock *)sk, skb, NULL, dev, dst_output); }
- skb->dev = dst->dev; + skb->dev = dev; /* ipv6_local_error() does not require socket lock, * we promote our socket to non const */ ipv6_local_error((struct sock *)sk, EMSGSIZE, fl6, mtu);
- IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)), IPSTATS_MIB_FRAGFAILS); + IP6_INC_STATS(net, idev, IPSTATS_MIB_FRAGFAILS); kfree_skb(skb); return -EMSGSIZE; }
[ Sasha's backport helper bot ]
Hi,
The upstream commit SHA1 provided is correct: 0c9f227bee11910a49e1d159abe102d06e3745d5
WARNING: Author mismatch between patch and upstream commit: Backport author: Harshvardhan Jha harshvardhan.j.jha@oracle.com Commit author: Vasily Averin vvs@virtuozzo.com
Status in newer kernel trees: 6.12.y | Present (exact SHA1) 6.6.y | Present (exact SHA1) 6.1.y | Present (exact SHA1) 5.15.y | Present (exact SHA1) 5.10.y | Not found
Note: The patch differs from the upstream commit: --- 1: 0c9f227bee11 ! 1: a38cbbbc21d7 ipv6: use skb_expand_head in ip6_xmit @@ Metadata ## Commit message ## ipv6: use skb_expand_head in ip6_xmit
+ [ Upstream commit 0c9f227bee11910a49e1d159abe102d06e3745d5 ] + Unlike skb_realloc_headroom, new helper skb_expand_head does not allocate a new skb if possible.
@@ Commit message
Signed-off-by: Vasily Averin vvs@virtuozzo.com Signed-off-by: David S. Miller davem@davemloft.net + (cherry picked from commit 0c9f227bee11910a49e1d159abe102d06e3745d5) + Signed-off-by: Harshvardhan Jha harshvardhan.j.jha@oracle.com
## net/ipv6/ip6_output.c ## @@ net/ipv6/ip6_output.c: int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6, ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-5.10.y | Success | Success | | stable/linux-5.4.y | Success | Success |
From: Eric Dumazet edumazet@google.com
[ Upstream commit e891b36de161fcd96f12ff83667473e5067b9037 ]
If skb_expand_head() returns NULL, skb has been freed and associated dst/idev could also have been freed.
We need to hold rcu_read_lock() to make sure the dst and associated idev are alive.
Fixes: 5796015fa968 ("ipv6: allocate enough headroom in ip6_finish_output2()") Signed-off-by: Eric Dumazet edumazet@google.com Cc: Vasily Averin vasily.averin@linux.dev Reviewed-by: David Ahern dsahern@kernel.org Link: https://patch.msgid.link/20240820160859.3786976-3-edumazet@google.com Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org (cherry picked from commit e891b36de161fcd96f12ff83667473e5067b9037) Signed-off-by: Harshvardhan Jha harshvardhan.j.jha@oracle.com --- net/ipv6/ip6_output.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index a8475848d0382..48f926157ef8c 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -69,11 +69,15 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *
/* Be paranoid, rather than too clever. */ if (unlikely(hh_len > skb_headroom(skb)) && dev->header_ops) { + /* Make sure idev stays alive */ + rcu_read_lock(); skb = skb_expand_head(skb, hh_len); if (!skb) { IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS); + rcu_read_unlock(); return -ENOMEM; } + rcu_read_unlock(); }
hdr = ipv6_hdr(skb);
[ Sasha's backport helper bot ]
Hi,
The claimed upstream commit SHA1 (e891b36de161fcd96f12ff83667473e5067b9037) was not found. However, I found a matching commit: da273b377ae0d9bd255281ed3c2adb228321687b
WARNING: Author mismatch between patch and found commit: Backport author: Harshvardhan Jha harshvardhan.j.jha@oracle.com Commit author: Eric Dumazet edumazet@google.com
Status in newer kernel trees: 6.12.y | Present (exact SHA1) 6.6.y | Present (different SHA1: 6ab6bf731354) 6.1.y | Present (different SHA1: 3574d28caf9a) 5.15.y | Present (different SHA1: e891b36de161) 5.10.y | Not found
Note: The patch differs from the upstream commit: --- Failed to apply patch cleanly, falling back to interdiff... ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-5.10.y | Success | Success | | stable/linux-5.4.y | Success | Success |
linux-stable-mirror@lists.linaro.org