Re: [report] OOB in bpf_load_prog() flow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Dec 20, 2022 at 01:03:47PM -0800, Stanislav Fomichev wrote:
> On Tue, Dec 20, 2022 at 11:08 AM Kees Cook <kees@xxxxxxxxxx> wrote:
> >
> > On December 20, 2022 9:32:51 AM PST, Stanislav Fomichev <sdf@xxxxxxxxxx> wrote:
> > >On Tue, Dec 20, 2022 at 3:37 AM Hyunwoo Kim <v4bel@xxxxxxxxx> wrote:
> > >>
> > >> On Mon, Dec 19, 2022 at 11:02:32AM -0800, sdf@xxxxxxxxxx wrote:
> > >> > On 12/19, Hyunwoo Kim wrote:
> > >> > > Dear,
> > >> >
> > >> > > This slab-out-of-bounds occurs in the bpf_prog_load() flow:
> > >> > > https://syzkaller.appspot.com/text?tag=CrashLog&x=172e2510480000
> > >> >
> > >> > > I was able to trigger KASAN using this syz reproduce code:
> > [...]
> > >> >
> > >> > > IMHO, the root cause of this seems to be commit
> > >> > > ceb35b666d42c2e91b1f94aeca95bb5eb0943268.
> > >> >
> > >> > > Also, a user with permission to load a BPF program can use this OOB to
> > >> > > execute the desired code with kernel privileges.
> > >> >
> > >> > Let's CC Kees if you suspect the commit above. Maybe we can run
> > >> > with/without it to confirm?
> > >>
> > >> I built and tested each commit of 'kernel/bpf/verifier.c' that caused
> > >> OOB, but I couldn't find the commit that caused OOB.
> > >>
> > >> So, starting from upstream, I reversed commits one by one and
> > >> found the commit that triggers KASAN.
> > >>
> > >> As a result of testing, OOB is triggered from commit
> > >> 8fa590bf344816c925810331eea8387627bbeb40.
> > >>
> > >> However, this commit seems to be a kvm related patch,
> > >> not directly related to the bpf subsystem.
> > >>
> > >> IMHO, the cause of this seems to be one of these:
> > >> 1. I ran this KASAN test on a nested guest in L2. That is,
> > >> there is a problem with the kvm patch 8fa590bf34481.
> > >>
> > >> 2. Previously, the BPF subsystem had a patch that triggers KASAN,
> > >> and KASAN is induced when kvm is patched.
> > >>
> > >> 3. There was confusion in the .config I tested, so the wrong
> > >> patch was derived as a test result.
> > >>
> > >> I haven't been able to pinpoint what the root cause is yet.
> > >> So I didn't add a CC for 8fa590bf34481 commit.
> > >
> > >Thanks for the details! Even if this particular one is unrelated,
> > >there are a couple of reports which still somewhat look like they are
> > >related to commit ceb35b666d42 ("bpf/verifier: Use
> > >kmalloc_size_roundup() to match ksize() usage") ?
> > >
> > >https://lore.kernel.org/bpf/000000000000ab724705ee87e321@xxxxxxxxxx/
> > >https://lore.kernel.org/bpf/000000000000269f9a05f02be9d8@xxxxxxxxxx/
> >
> > I suspect something is hitting array_resize() that wasn't maximal-bucket-size allocated. Does reverting 38931d8989b5760b0bd17c9ec99e81986258e4cb make it go away?
> 
> Reverting makes it go away for at least one of them:
> https://lore.kernel.org/bpf/0000000000004bee2205f0484e1d@xxxxxxxxxx/T/#m60dba18e94e01094a899ab7fe8d19aa1a3cf26fe
> 
> The second one didn't like my patch, I'm trying again now:
> https://lore.kernel.org/bpf/Y6Iipad5vz55tl2A@xxxxxxxxxx/T/#m032bed8c3d47f33a9fccd660446beabce98ff5fe

I have found the root cause of this issue.

This happens when krealloc() in push_jmp_history() receives an allocation 
request with a size smaller than the existing slab object.
Based on the poc code I first reported, it occurs when cur->jmp_history 
of 64 bytes is krealloc()ed to 32 bytes.

When krealloc() is called with a smaller size than the previous one as an argument, 
krealloc() does not 'kfree&reallocate', but only resets the slab redzone based on the newly received size (if KASAN is activated).
That's why `ksize(dst)` in copy_array() afterward returns the actual allocation size of 64 bytes. 
However, KASAN recognizes memcpy(dst, src, 64) as OOB because the redzone is set after 32 bytes by krealloc().

In any case, since the actually allocated size of dst is 64 bytes, exploiting this is close to impossible.
However, it seems that copy_array() needs to be patched in the direction of not using ksize().


Regards,
Hyunwoo Kim



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux