* [PATCH bpf] bpf: fix bpf_prog_array_copy_to_user() issues
@ 2018-02-02 23:14 Alexei Starovoitov
2018-02-03 1:25 ` Daniel Borkmann
0 siblings, 1 reply; 2+ messages in thread
From: Alexei Starovoitov @ 2018-02-02 23:14 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, kernel-team
1. move copy_to_user out of rcu section to fix the following issue:
./include/linux/rcupdate.h:302 Illegal context switch in RCU read-side critical section!
stack backtrace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
lockdep_rcu_suspicious+0x123/0x170 kernel/locking/lockdep.c:4592
rcu_preempt_sleep_check include/linux/rcupdate.h:301 [inline]
___might_sleep+0x385/0x470 kernel/sched/core.c:6079
__might_sleep+0x95/0x190 kernel/sched/core.c:6067
__might_fault+0xab/0x1d0 mm/memory.c:4532
_copy_to_user+0x2c/0xc0 lib/usercopy.c:25
copy_to_user include/linux/uaccess.h:155 [inline]
bpf_prog_array_copy_to_user+0x217/0x4d0 kernel/bpf/core.c:1587
bpf_prog_array_copy_info+0x17b/0x1c0 kernel/bpf/core.c:1685
perf_event_query_prog_array+0x196/0x280 kernel/trace/bpf_trace.c:877
_perf_ioctl kernel/events/core.c:4737 [inline]
perf_ioctl+0x3e1/0x1480 kernel/events/core.c:4757
2. move *prog under rcu, since it's not ok to dereference it afterwards
3. in a rare case of prog array being swapped between bpf_prog_array_length()
and bpf_prog_array_copy_to_user() calls make sure to copy zeros to user space,
so the user doesn't walk over uninited prog_ids while kernel reported
uattr->query.prog_cnt > 0
Reported-by: syzbot+7dbcd2d3b85f9b608b23@syzkaller.appspotmail.com
Fixes: 468e2f64d220 ("bpf: introduce BPF_PROG_QUERY command")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
kernel/bpf/core.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 5f35f93dcab2..29ca9208dcfa 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1576,25 +1576,41 @@ int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
__u32 __user *prog_ids, u32 cnt)
{
struct bpf_prog **prog;
- u32 i = 0, id;
-
+ unsigned long err = 0;
+ u32 i = 0, *ids;
+ bool nospc;
+
+ /* users of this function are doing:
+ * cnt = bpf_prog_array_length();
+ * if (cnt > 0)
+ * bpf_prog_array_copy_to_user(..., cnt);
+ * so below kcalloc doesn't need extra cnt > 0 check, but
+ * bpf_prog_array_length() releases rcu lock and
+ * prog array could have been swapped with empty or larger array,
+ * so always copy 'cnt' prog_ids to the user.
+ * In a rare race the user will see zero prog_ids
+ */
+ ids = kcalloc(cnt, sizeof(u32), GFP_USER);
+ if (!ids)
+ return -ENOMEM;
rcu_read_lock();
prog = rcu_dereference(progs)->progs;
for (; *prog; prog++) {
if (*prog == &dummy_bpf_prog.prog)
continue;
- id = (*prog)->aux->id;
- if (copy_to_user(prog_ids + i, &id, sizeof(id))) {
- rcu_read_unlock();
- return -EFAULT;
- }
+ ids[i] = (*prog)->aux->id;
if (++i == cnt) {
prog++;
break;
}
}
+ nospc = !!(*prog);
rcu_read_unlock();
- if (*prog)
+ err = copy_to_user(prog_ids, ids, cnt * sizeof(u32));
+ kfree(ids);
+ if (err)
+ return -EFAULT;
+ if (nospc)
return -ENOSPC;
return 0;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH bpf] bpf: fix bpf_prog_array_copy_to_user() issues
2018-02-02 23:14 [PATCH bpf] bpf: fix bpf_prog_array_copy_to_user() issues Alexei Starovoitov
@ 2018-02-03 1:25 ` Daniel Borkmann
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Borkmann @ 2018-02-03 1:25 UTC (permalink / raw)
To: Alexei Starovoitov, davem; +Cc: netdev, kernel-team
On 02/03/2018 12:14 AM, Alexei Starovoitov wrote:
> 1. move copy_to_user out of rcu section to fix the following issue:
>
> ./include/linux/rcupdate.h:302 Illegal context switch in RCU read-side critical section!
> stack backtrace:
> __dump_stack lib/dump_stack.c:17 [inline]
> dump_stack+0x194/0x257 lib/dump_stack.c:53
> lockdep_rcu_suspicious+0x123/0x170 kernel/locking/lockdep.c:4592
> rcu_preempt_sleep_check include/linux/rcupdate.h:301 [inline]
> ___might_sleep+0x385/0x470 kernel/sched/core.c:6079
> __might_sleep+0x95/0x190 kernel/sched/core.c:6067
> __might_fault+0xab/0x1d0 mm/memory.c:4532
> _copy_to_user+0x2c/0xc0 lib/usercopy.c:25
> copy_to_user include/linux/uaccess.h:155 [inline]
> bpf_prog_array_copy_to_user+0x217/0x4d0 kernel/bpf/core.c:1587
> bpf_prog_array_copy_info+0x17b/0x1c0 kernel/bpf/core.c:1685
> perf_event_query_prog_array+0x196/0x280 kernel/trace/bpf_trace.c:877
> _perf_ioctl kernel/events/core.c:4737 [inline]
> perf_ioctl+0x3e1/0x1480 kernel/events/core.c:4757
>
> 2. move *prog under rcu, since it's not ok to dereference it afterwards
>
> 3. in a rare case of prog array being swapped between bpf_prog_array_length()
> and bpf_prog_array_copy_to_user() calls make sure to copy zeros to user space,
> so the user doesn't walk over uninited prog_ids while kernel reported
> uattr->query.prog_cnt > 0
>
> Reported-by: syzbot+7dbcd2d3b85f9b608b23@syzkaller.appspotmail.com
> Fixes: 468e2f64d220 ("bpf: introduce BPF_PROG_QUERY command")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
LGTM, fix applied to bpf tree, thanks Alexei!
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-02-03 1:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-02 23:14 [PATCH bpf] bpf: fix bpf_prog_array_copy_to_user() issues Alexei Starovoitov
2018-02-03 1:25 ` Daniel Borkmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.