diff mbox series

[net,3/3] ipv6: prevent possible UAF in ip6_xmit()

Message ID 20240819134827.2989452-4-edumazet@google.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ipv6: fix possible UAF in output paths | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 16
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 16 this patch: 16
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 16 this patch: 16
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 15 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 3 this patch: 3
netdev/source_inline success Was 0 now: 0
netdev/contest fail net-next-2024-08-20--00-00 (tests: 712)

Commit Message

Eric Dumazet Aug. 19, 2024, 1:48 p.m. UTC 
If skb_expand_head() returns NULL, skb has been freed
and the associated dst/idev could also have been freed.

We must use rcu_read_lock() to prevent a possible UAF.

Fixes: 0c9f227bee11 ("ipv6: use skb_expand_head in ip6_xmit")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Vasily Averin <vvs@virtuozzo.com>
---
 net/ipv6/ip6_output.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

David Ahern Aug. 19, 2024, 3:21 p.m. UTC | #1 
On 8/19/24 7:48 AM, Eric Dumazet wrote:
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 1b9ebee7308f02a626c766de1794e6b114ae8554..519690514b2d1520a311adbcfaa8c6a69b1e85d3 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -287,11 +287,15 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
>  		head_room += opt->opt_nflen + opt->opt_flen;
>  
>  	if (unlikely(head_room > skb_headroom(skb))) {
> +		/* Make sure idev stays alive */
> +		rcu_read_lock();
>  		skb = skb_expand_head(skb, head_room);
>  		if (!skb) {
> +			rcu_read_unlock();
>  			IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS);

rcu_read_unlock after INC_STATS

>  			return -ENOBUFS;
>  		}
> +		rcu_read_unlock();
>  	}
>  
>  	if (opt) {
Eric Dumazet Aug. 19, 2024, 3:22 p.m. UTC | #2 
On Mon, Aug 19, 2024 at 5:21 PM David Ahern <dsahern@kernel.org> wrote:
>
> On 8/19/24 7:48 AM, Eric Dumazet wrote:
> > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> > index 1b9ebee7308f02a626c766de1794e6b114ae8554..519690514b2d1520a311adbcfaa8c6a69b1e85d3 100644
> > --- a/net/ipv6/ip6_output.c
> > +++ b/net/ipv6/ip6_output.c
> > @@ -287,11 +287,15 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
> >               head_room += opt->opt_nflen + opt->opt_flen;
> >
> >       if (unlikely(head_room > skb_headroom(skb))) {
> > +             /* Make sure idev stays alive */
> > +             rcu_read_lock();
> >               skb = skb_expand_head(skb, head_room);
> >               if (!skb) {
> > +                     rcu_read_unlock();
> >                       IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS);
>
> rcu_read_unlock after INC_STATS

Indeed, thanks for noticing.
diff mbox series

Patch

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 1b9ebee7308f02a626c766de1794e6b114ae8554..519690514b2d1520a311adbcfaa8c6a69b1e85d3 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -287,11 +287,15 @@  int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
 		head_room += opt->opt_nflen + opt->opt_flen;
 
 	if (unlikely(head_room > skb_headroom(skb))) {
+		/* Make sure idev stays alive */
+		rcu_read_lock();
 		skb = skb_expand_head(skb, head_room);
 		if (!skb) {
+			rcu_read_unlock();
 			IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS);
 			return -ENOBUFS;
 		}
+		rcu_read_unlock();
 	}
 
 	if (opt) {