linux-kernel - Re: [PATCH 1/2] ras: fix an off-by-one error in __find

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: <CAM_iQpV4gMkh2xd9e3rBwaXpYuwG_QPDqw+Zj71f5Yf_n54rPQ@mail.gmail.com>
Date:   Tue, 16 Apr 2019 10:01:41 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, linux-edac@...r.kernel.org,
        Tony Luck <tony.luck@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 1/2] ras: fix an off-by-one error in __find_elem()

On Tue, Apr 16, 2019 at 2:07 AM Borislav Petkov <bp@...en8.de> wrote:
>
> On Mon, Apr 15, 2019 at 06:20:00PM -0700, Cong Wang wrote:
> > ce_arr.array[] is always within the range [0, ce_arr.n-1].
> > However, the binary search code in __find_elem() uses ce_arr.n
> > as the maximum index, which could lead to an off-by-one
> > out-of-bound access when the element after the last is exactly
> > the one just got deleted, that is, 'min' returned to caller as
> > 'ce_arr.n'.
>
> Sorry, I don't follow.

Sorry for the confusion here. Let me try to make it clear with
the example I wrote down on a paper before submitting this patch.

Imagine we have the ca->array[] with ca->n = 4, the elements inside
are 0, 1, 2, 3. We are trying to find 5 in this array. (This is just to
simplify the following iterations of the while loop.)

So in this specific scenario, without my patch we have the following
inside the while loop:

min = 0, max = 4
tmp = 2
min = 3, max = 4
tmp = 3
min = 4, max = 4
break

It is okay to have min==4 after this loop as we still need to check
if array[4] is whether 5. The problem is array[4] could really be 5
before we delete array[4], so 4 could be returned to caller. And,
after that, 4 could be passed to del_elem() inside cec_add_elem(),
then the if check inside is passed as it is an unsigned operation,
then the memmove() accesses index 5...

To actually crash the kernel, we have to replace 4 with MAX_ELEMS
in the above example, kernel would crash either when reading
array[MAX_ELEMS] or in the memmove().

>
> There's a debugfs interface in /sys/kernel/debug/ras/cec/ with which you
> can input random PFNs and test the thing.
>
> Show me pls how this can happen with an example.
>

Let me try if I can figure out how to add and remove PFN's.

Thanks.