[PATCH] Using a pointer and kzalloc in place of a struct directly

From: Anant Thazhemadam
Date: Sat Sep 12 2020 - 07:38:44 EST


Updated the usage of a struct variable directly, in bpf_link_get_info_by_fd
to using a pointer of the same type instead, which points to a memory
location allocated using kzalloc.

Signed-off-by: Anant Thazhemadam <anant.thazhemadam@xxxxxxxxx>
---
I saw this bug (https://syzkaller.appspot.com/bug?extid=976d5ecfab0c7eb43ac3),
and tried to come up with a patch for it (before I saw that this had already
been taken care of).
Although I don't think it fundamentally changes how things work much, it still
seems to have fixed the error on it's own too.
I'd like to hear anyone's 2c on this, and know  if this method of using info
(of type bpf_link_info) instead
would be a welcome change in general, even if it was not centered around
fixing the bug.
If instead, as an unwelcome consequence, this patch might make something go
wrong somewhere, or passing
the syzbot test was a false positive, I would appreciate it if you could shed
some light on that for me as well.
If this patch seems acceptable, then I'll send in a cleaner v2 that's a little
more articulate, if required.
Just trying to understand how things work, and sometimes why things work
in and around the kernel.
Thanks,
Anant


kernel/bpf/syscall.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 4108ef3b828b..01b9c203ef65 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3605,30 +3605,31 @@ static int bpf_link_get_info_by_fd(struct file *file,
union bpf_attr __user *uattr)
{
struct bpf_link_info __user *uinfo = u64_to_user_ptr(attr->info.info);
- struct bpf_link_info info;
+ struct bpf_link_info *info = NULL;
u32 info_len = attr->info.info_len;
int err;

- err = bpf_check_uarg_tail_zero(uinfo, sizeof(info), info_len);
+ err = bpf_check_uarg_tail_zero(uinfo, sizeof(struct bpf_link_info), info_len);
+
if (err)
return err;
info_len = min_t(u32, sizeof(info), info_len);

- memset(&info, 0, sizeof(info));
- if (copy_from_user(&info, uinfo, info_len))
+ info = kzalloc(sizeof(struct bpf_link_info), GFP_KERNEL);
+ if (copy_from_user(info, uinfo, info_len))
return -EFAULT;

- info.type = link->type;
- info.id = link->id;
- info.prog_id = link->prog->aux->id;
+ info->type = link->type;
+ info->id = link->id;
+ info->prog_id = link->prog->aux->id;

if (link->ops->fill_link_info) {
- err = link->ops->fill_link_info(link, &info);
+ err = link->ops->fill_link_info(link, info);
if (err)
return err;
}

- if (copy_to_user(uinfo, &info, info_len) ||
+ if (copy_to_user(uinfo, info, info_len) ||
put_user(info_len, &uattr->info.info_len))
return -EFAULT;

--
2.25.1