Skip to content

Commit

Permalink
bpf: Local storage helpers should check nullness of owner ptr passed
Browse files Browse the repository at this point in the history
The verifier allows ARG_PTR_TO_BTF_ID helper arguments to be NULL, so
helper implementations need to check this before dereferencing them.
This was already fixed for the socket storage helpers but not for task
and inode.

The issue can be reproduced by attaching an LSM program to
inode_rename hook (called when moving files) which tries to get the
inode of the new file without checking for its nullness and then trying
to move an existing file to a new path:

  mv existing_file new_file_does_not_exist

The report including the sample program and the steps for reproducing
the bug:

  https://lore.kernel.org/bpf/CANaYP3HWkH91SN=wTNO9FL_2ztHfqcXKX38SSE-JJ2voh+vssw@mail.gmail.com

Fixes: 4cf1bc1 ("bpf: Implement task local storage")
Fixes: 8ea6368 ("bpf: Implement bpf_local_storage for inodes")
Reported-by: Gilad Reti <gilad.reti@gmail.com>
Signed-off-by: KP Singh <kpsingh@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
Link: https://lore.kernel.org/bpf/20210112075525.256820-3-kpsingh@kernel.org
  • Loading branch information
KP Singh authored and Daniel Borkmann committed Jan 12, 2021
1 parent 5541075 commit 1a9c72a
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
5 changes: 4 additions & 1 deletion kernel/bpf/bpf_inode_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ BPF_CALL_4(bpf_inode_storage_get, struct bpf_map *, map, struct inode *, inode,
* bpf_local_storage_update expects the owner to have a
* valid storage pointer.
*/
if (!inode_storage_ptr(inode))
if (!inode || !inode_storage_ptr(inode))
return (unsigned long)NULL;

sdata = inode_storage_lookup(inode, map, true);
Expand All @@ -200,6 +200,9 @@ BPF_CALL_4(bpf_inode_storage_get, struct bpf_map *, map, struct inode *, inode,
BPF_CALL_2(bpf_inode_storage_delete,
struct bpf_map *, map, struct inode *, inode)
{
if (!inode)
return -EINVAL;

/* This helper must only called from where the inode is gurranteed
* to have a refcount and cannot be freed.
*/
Expand Down
5 changes: 4 additions & 1 deletion kernel/bpf/bpf_task_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ BPF_CALL_4(bpf_task_storage_get, struct bpf_map *, map, struct task_struct *,
* bpf_local_storage_update expects the owner to have a
* valid storage pointer.
*/
if (!task_storage_ptr(task))
if (!task || !task_storage_ptr(task))
return (unsigned long)NULL;

sdata = task_storage_lookup(task, map, true);
Expand All @@ -243,6 +243,9 @@ BPF_CALL_4(bpf_task_storage_get, struct bpf_map *, map, struct task_struct *,
BPF_CALL_2(bpf_task_storage_delete, struct bpf_map *, map, struct task_struct *,
task)
{
if (!task)
return -EINVAL;

/* This helper must only be called from places where the lifetime of the task
* is guaranteed. Either by being refcounted or by being protected
* by an RCU read-side critical section.
Expand Down

0 comments on commit 1a9c72a

Please sign in to comment.