On Mon, Mar 03, 2025 at 12:06:58PM +0100, Vlastimil Babka wrote:
> On 3/2/25 19:01, Lilith Persefoni Gkini wrote:
> > If the `search` pattern is not found in the freelist then the function
> > should return `fp == search` where fp is the last freepointer from the
> > while loop.
> >
> > If the caller of the function was searching for NULL and the freelist is
> > valid it should return True (1), otherwise False (0).
>
> This suggests we should change the function return value to bool :)
>
Alright, If you want to be more technical it's
`1 (true), otherwise 0 (false).`
Its just easier to communicate with the true or false concepts, but in C
we usually don't use bools cause its just 1s or 0s.
> I think there's a problem that none of this will fix or even report the
> situation properly. Even worse we'll set slab->inuse to 0 and thus pretend
> all objects are free. This goes contrary to the other places that respond to
> slab corruption by setting all objects to used and trying not to touch the
> slab again at all.
>
> So I think after the while loop we could determine there was a cycle if (nr
> == slab->objects && fp != NULL), right? In that case we could perform the
> same report and fix as in the "Freepointer corrupt" case?
True! We could either add an if check after the while as you said to
replicate the "Freepointer corrupt" behavior...
Or...
I hate to say it, or we could leave the while condition with the equal
sign intact, as it was, and change that `if` check from
`if (!check_valid_pointer(s, slab, fp)) {`
to
`if (!check_valid_pointer(s, slab, fp) || nr == slab->objects) {`
When it reaches nr == slab->objects and we are still in the while loop
it means that fp != NULL and therefore the freelist is corrupted (note
that nr starts from 0).
This would add fewer lines of code and there won't be any repeating
code.
It will enter in the "Freechain corrupt" branch and set the tail of
the freelist to NULL, inform us of the error and it won't get a chance
to do the nr++ part, leaving nr == slab->objects in that particular
case, because it breaks of the loop afterwards.
But it will not Null-out the freelist and set inuse to objects like you
suggested. If that is the desired behavior instead then we could do
something like you suggested.
