Re: [PATCH stable 4.4 net] net: rds: Fix NULL ptr use in rds_tcp_kill_sock

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Sep 18, 2019 at 04:37:33PM +0800, Mao Wenan wrote:
> After the commit c4e97b06cfdc ("net: rds: force to destroy
> connection if t_sock is NULL in rds_tcp_kill_sock()."),
> it introduced null-ptr-deref in rds_tcp_kill_sock as below:
> 
> BUG: KASAN: null-ptr-deref on address 0000000000000020
> Read of size 8 by task kworker/u16:10/910
> CPU: 3 PID: 910 Comm: kworker/u16:10 Not tainted 4.4.178+ #3
> Hardware name: linux,dummy-virt (DT)
> Workqueue: netns cleanup_net
> Call trace:
> [<ffffff90080abb50>] dump_backtrace+0x0/0x618
> [<ffffff90080ac1a0>] show_stack+0x38/0x60
> [<ffffff9008c42b78>] dump_stack+0x1a8/0x230
> [<ffffff90085d469c>] kasan_report_error+0xc8c/0xfc0
> [<ffffff90085d54a4>] kasan_report+0x94/0xd8
> [<ffffff90085d1b28>] __asan_load8+0x88/0x150
> [<ffffff9009c9cc2c>] rds_tcp_dev_event+0x734/0xb48
> [<ffffff90081eacb0>] raw_notifier_call_chain+0x150/0x1e8
> [<ffffff900973fec0>] call_netdevice_notifiers_info+0x90/0x110
> [<ffffff9009764874>] netdev_run_todo+0x2f4/0xb08
> [<ffffff9009796d34>] rtnl_unlock+0x2c/0x48
> [<ffffff9009756484>] default_device_exit_batch+0x444/0x528
> [<ffffff9009720498>] ops_exit_list+0x1c0/0x240
> [<ffffff9009724a80>] cleanup_net+0x738/0xbf8
> [<ffffff90081ca6cc>] process_one_work+0x96c/0x13e0
> [<ffffff90081cf370>] worker_thread+0x7e0/0x1910
> [<ffffff90081e7174>] kthread+0x304/0x390
> [<ffffff9008094280>] ret_from_fork+0x10/0x50
> 
> If the first loop add the tc->t_sock = NULL to the tmp_list,
> 1). list_for_each_entry_safe(tc, _tc, &rds_tcp_conn_list, t_tcp_node)
> 
> then the second loop is to find connections to destroy, tc->t_sock
> might equal NULL, and tc->t_sock->sk happens null-ptr-deref.
> 2). list_for_each_entry_safe(tc, _tc, &tmp_list, t_tcp_node)
> 
> Fixes: c4e97b06cfdc ("net: rds: force to destroy connection if t_sock is NULL in rds_tcp_kill_sock().")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  net/rds/tcp.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Why is this not needed upstream as well?

4.9.y?  4.14.y?  anything else?

thanks,

greg k-h