netdev - Re: [PATCH] tipc: Disable preemption when calling send

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5a668c12-4038-1f68-3e17-edcdc66e9e96@gmail.com>
Date:   Fri, 22 May 2020 14:45:55 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
        Jon Maloy <jmaloy@...hat.com>,
        Ying Xue <ying.xue@...driver.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH] tipc: Disable preemption when calling send_msg method.



On 5/20/20 4:45 AM, Tetsuo Handa wrote:
> syzbot is reporting that tipc_udp_send_msg() is calling dst_cache_get()
> without preemption disabled [1]. Since b->media->send_msg() is called
> with RCU lock already held, we can also disable preemption at that point.
> 
> [1] https://syzkaller.appspot.com/bug?id=dc6352b92862eb79373fe03fdf9af5928753e057
> 
> Reported-by: syzbot+1a68504d96cd17b33a05@...kaller.appspotmail.com
> Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> ---
>  net/tipc/bearer.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index 34ca7b789eba..e5cf91665881 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -516,6 +516,7 @@ void tipc_bearer_xmit_skb(struct net *net, u32 bearer_id,
>  	struct tipc_msg *hdr = buf_msg(skb);
>  	struct tipc_bearer *b;
>  
> +	preempt_disable();
>  	rcu_read_lock();
>  	b = bearer_get(net, bearer_id);
>  	if (likely(b && (test_bit(0, &b->up) || msg_is_reset(hdr)))) {
> @@ -528,6 +529,7 @@ void tipc_bearer_xmit_skb(struct net *net, u32 bearer_id,
>  		kfree_skb(skb);
>  	}
>  	rcu_read_unlock();
> +	preempt_enable();
>  }
>  
>  /* tipc_bearer_xmit() -send buffer to destination over bearer
> @@ -543,6 +545,7 @@ void tipc_bearer_xmit(struct net *net, u32 bearer_id,
>  	if (skb_queue_empty(xmitq))
>  		return;
>  
> +	preempt_disable();
>  	rcu_read_lock();
>  	b = bearer_get(net, bearer_id);
>  	if (unlikely(!b))
> @@ -560,6 +563,7 @@ void tipc_bearer_xmit(struct net *net, u32 bearer_id,
>  		}
>  	}
>  	rcu_read_unlock();
> +	preempt_enable();
>  }
>  
>  /* tipc_bearer_bc_xmit() - broadcast buffers to all destinations
> @@ -574,6 +578,7 @@ void tipc_bearer_bc_xmit(struct net *net, u32 bearer_id,
>  	struct sk_buff *skb, *tmp;
>  	struct tipc_msg *hdr;
>  
> +	preempt_disable();
>  	rcu_read_lock();
>  	b = bearer_get(net, bearer_id);
>  	if (unlikely(!b || !test_bit(0, &b->up)))
> @@ -591,6 +596,7 @@ void tipc_bearer_bc_xmit(struct net *net, u32 bearer_id,
>  			b->media->send_msg(net, skb, b, dst);
>  	}
>  	rcu_read_unlock();
> +	preempt_enable();
>  }
>  
>  /**
> 


This is wrong, see my patch instead.

https://patchwork.ozlabs.org/project/netdev/patch/20200521182958.163436-1-edumazet@google.com/ 

Thanks.