Messages in this thread | | | From | Cong Wang <> | Date | Tue, 16 Apr 2019 10:01:41 -0700 | 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@alien8.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.
| |