Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Anant Thazhemadam <anant.thazhemadam@gmail.com>
Cc:     andriin@fb.com, ast@kernel.org, bpf@vger.kernel.org,
        daniel@iogearbox.net, davem@davemloft.net, hawk@kernel.org,
        john.fastabend@gmail.com, kafai@fb.com, kpsingh@chromium.org,
        kuba@kernel.org, linux-kernel@vger.kernel.org,
        anant.thazhemadam@gmail.com
Subject: [PATCH] Using a pointer and kzalloc in place of a struct directly 
Date:   Sat, 12 Sep 2020 17:08:04 +0530
Message-Id: <20200912113804.6465-1-anant.thazhemadam@gmail.com>
In-Reply-To: <000000000000c82fe505aef233c6@google.com>
References: <000000000000c82fe505aef233c6@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
To:     unlisted-recipients:; (no To-header on input)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@gmail.com>
---
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