netdev - Re: [PATCH net] net: bpfilter: use get_pid_task instead of pid

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181016155122.ct5gbwdtb3bhpvko@ast-mbp.dhcp.thefacebook.com>
Date:   Tue, 16 Oct 2018 08:51:24 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Taehee Yoo <ap420073@...il.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, daniel@...earbox.net,
        ast@...nel.org
Subject: Re: [PATCH net] net: bpfilter: use get_pid_task instead of pid_task

On Wed, Oct 17, 2018 at 12:35:10AM +0900, Taehee Yoo wrote:
> pid_task() dereferences rcu protected tasks array.
> But there is no rcu_read_lock() in shutdown_umh() routine so that
> rcu_read_lock() is needed.
> get_pid_task() is wrapper function of pid_task. it holds rcu_read_lock()
> then calls pid_task(). if task isn't NULL, it increases reference count
> of task.
> 
> test commands:
>    %modprobe bpfilter
>    %modprobe -rv bpfilter
> 
> splat looks like:
> [15102.030932] =============================
> [15102.030957] WARNING: suspicious RCU usage
> [15102.030985] 4.19.0-rc7+ #21 Not tainted
> [15102.031010] -----------------------------
> [15102.031038] kernel/pid.c:330 suspicious rcu_dereference_check() usage!
> [15102.031063]
> 	       other info that might help us debug this:
> 
> [15102.031332]
> 	       rcu_scheduler_active = 2, debug_locks = 1
> [15102.031363] 1 lock held by modprobe/1570:
> [15102.031389]  #0: 00000000580ef2b0 (bpfilter_lock){+.+.}, at: stop_umh+0x13/0x52 [bpfilter]
> [15102.031552]
>                stack backtrace:
> [15102.031583] CPU: 1 PID: 1570 Comm: modprobe Not tainted 4.19.0-rc7+ #21
> [15102.031607] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/08/2015
> [15102.031628] Call Trace:
> [15102.031676]  dump_stack+0xc9/0x16b
> [15102.031723]  ? show_regs_print_info+0x5/0x5
> [15102.031801]  ? lockdep_rcu_suspicious+0x117/0x160
> [15102.031855]  pid_task+0x134/0x160
> [15102.031900]  ? find_vpid+0xf0/0xf0
> [15102.032017]  shutdown_umh.constprop.1+0x1e/0x53 [bpfilter]
> [15102.032055]  stop_umh+0x46/0x52 [bpfilter]
> [15102.032092]  __x64_sys_delete_module+0x47e/0x570
> [ ... ]
> 
> Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
> Signed-off-by: Taehee Yoo <ap420073@...il.com>

thanks a lot for the fix
Acked-by: Alexei Starovoitov <ast@...nel.org>