linux-kernel - Re: [PATCH] mm/slab_common.c: fix possible spectre-v1 in kmalloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFbcbMA6XjZqrgHmG70Vm_a34Rn4tKqoMgQkRBXES2r3+ymYwg@mail.gmail.com>
Date:   Thu, 30 May 2019 13:20:01 +0800
From:   Dianzhang Chen <dianzhangchen0@...il.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     cl@...ux.com, penberg@...nel.org, rientjes@...gle.com,
        iamjoonsoo.kim@....com, akpm@...ux-foundation.org,
        linux-mm@...ck.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm/slab_common.c: fix possible spectre-v1 in kmalloc_slab()

It is possible that a CPU mis-predicts the conditional branch, and
speculatively loads size_index[size_index_elem(size)], even if size >192.
Although this value will subsequently be discarded,
but it can not drop all the effects of speculative execution,
such as the presence or absence of data in caches. Such effects may
form side-channels which can be
observed to extract secret information.

As for "why this particular path a needs special treatment while other
size branches are ok",
i think the other size branches need to treatment as well at first place,
but in code `index = fls(size - 1)` the function `fls` will make the
index at specific range,
so it can not use `kmalloc_caches[kmalloc_type(flags)][index]` to load
arbitury data.
But, still it may load some date that it shouldn't, if necessary, i
think can add array_index_nospec as well.

On Thu, May 30, 2019 at 1:49 AM Michal Hocko <mhocko@...nel.org> wrote:
>
> On Thu 30-05-19 00:39:53, Dianzhang Chen wrote:
> > It's come from `192+1`.
> >
> >
> > The more code fragment is:
> >
> >
> > if (size <= 192) {
> >
> >     if (!size)
> >
> >         return ZERO_SIZE_PTR;
> >
> >     size = array_index_nospec(size, 193);
> >
> >     index = size_index[size_index_elem(size)];
> >
> > }
>
> OK I see, I could have looked into the code, my bad. But I am still not
> sure what is the potential exploit scenario and why this particular path
> a needs special treatment while other size branches are ok. Could you be
> more specific please?
> --
> Michal Hocko
> SUSE Labs