linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Wenwen Wang <wang6495@umn.edu>
To: Wenwen Wang <wang6495@umn.edu>
Cc: Kangjie Lu <kjlu@umn.edu>, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	netdev@vger.kernel.org (open list:BPF (Safe dynamic programs and
	tools)),
	linux-kernel@vger.kernel.org (open list:BPF (Safe dynamic
	programs and tools))
Subject: [PATCH v2] bpf: btf: Fix a missing-check bug
Date: Wed, 24 Oct 2018 08:00:19 -0500	[thread overview]
Message-ID: <1540386020-30680-1-git-send-email-wang6495@umn.edu> (raw)

In btf_parse(), the header of the user-space btf data 'btf_data' is firstly
parsed and verified through btf_parse_hdr(). In btf_parse_hdr(), the header
is copied from user-space 'btf_data' to kernel-space 'btf->hdr' and then
verified. If no error happens during the verification process, the whole
data of 'btf_data', including the header, is then copied to 'data' in
btf_parse(). It is obvious that the header is copied twice here. More
importantly, no check is enforced after the second copy to make sure the
headers obtained in these two copies are same. Given that 'btf_data'
resides in the user space, a malicious user can race to modify the header
between these two copies. By doing so, the user can inject inconsistent
data, which can cause undefined behavior of the kernel and introduce
potential security risk.

To avoid the above issue, this patch copies the parsed header from
'btf->hdr' to 'data'. The remaining part in 'data' is still copied from the
user-space 'btf_data'.

Signed-off-by: Wenwen Wang <wang6495@umn.edu>
---
 kernel/bpf/btf.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 378cef7..b52a834a 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -2152,6 +2152,7 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
 	struct btf_verifier_env *env = NULL;
 	struct bpf_verifier_log *log;
 	struct btf *btf = NULL;
+	u32 hdr_len;
 	u8 *data;
 	int err;
 
@@ -2200,7 +2201,15 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
 	btf->data_size = btf_data_size;
 	btf->nohdr_data = btf->data + btf->hdr.hdr_len;
 
-	if (copy_from_user(data, btf_data, btf_data_size)) {
+	/*
+	 * The header at btf_data could be modified by a malicious user
+	 * after it is parsed. So we copy the parsed header here. The
+	 * remaining part is still copied from btf_data.
+	 */
+	hdr_len = min_t(u32, btf->hdr.hdr_len, sizeof(btf->hdr));
+	memcpy(data, &btf->hdr, hdr_len);
+	if (copy_from_user(data + hdr_len, (u8 __user *)btf_data + hdr_len,
+				btf_data_size - hdr_len)) {
 		err = -EFAULT;
 		goto errout;
 	}
-- 
2.7.4


             reply	other threads:[~2018-10-24 13:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-24 13:00 Wenwen Wang [this message]
2018-10-24 17:26 ` [PATCH v2] bpf: btf: Fix a missing-check bug Martin Lau
2018-10-24 18:22   ` Martin Lau
2018-10-24 20:42     ` Martin Lau
2018-10-24 21:50       ` Song Liu
2018-10-25 22:58       ` Daniel Borkmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1540386020-30680-1-git-send-email-wang6495@umn.edu \
    --to=wang6495@umn.edu \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kjlu@umn.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).