Messages in this thread | | | From | Y Song <> | Date | Mon, 22 Oct 2018 08:57:46 -0700 | Subject | Re: [PATCH] bpf: btf: Fix a missing-check bug |
| |
On Fri, Oct 19, 2018 at 3:30 PM Wenwen Wang <wang6495@umn.edu> wrote: > > 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 rewrites the header after the second > copy, using 'btf->hdr', which is obtained in the first copy. > > Signed-off-by: Wenwen Wang <wang6495@umn.edu> > --- > kernel/bpf/btf.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 138f030..2a85f91 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -2202,6 +2202,9 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size, > goto errout; > } > > + memcpy(data, &btf->hdr, > + min_t(u32, btf->hdr.hdr_len, sizeof(btf->hdr)));
Could you restructure the code to memcpy the header followed by copying the rest of btf_data with copy_from_user? This way, each byte is only copied once. Could you add some comments right before memcpy so later people will know why we implement this way?
> + > err = btf_parse_str_sec(env); > if (err) > goto errout; > -- > 2.7.4 >
| |