On 3/6/25 09:34, Harry Yoo wrote: > Hi Lilith, the patch looks good, and it's great to see the improvements > over the revisions! I've added my Reviewed-by: tag after the '---' line. > > A few nit comments are inlined below. > > From Documentation/process/submitting-patches.rst: >> Describe your changes in imperative mood, e.g. “make xyzzy do frotz” >> instead of “[This patch] makes xyzzy do frotz” or >> “[I] changed xyzzy to do frotz”, as if you are giving orders to the codebase >> to change its behaviour. > > nit: Per submitting-patches.rst, I think the subject could be: > - "slub: Add a way to handle freelist cycle in on_freelist()" > or more concisely, > - "slub: Handle freelist cycle in on_freelist()" Right, some of the sentences later too. I've adjusted it a bit to avoid the need to resubmit, hope it's ok. The result is here in slab/for-next https://web.git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git/commit/?h=slab/for-6.15/fixes-cleanups&id=747e2cf137f44058a093d3226bf83974d9d117e7 Thanks a lot! > On Wed, Mar 05, 2025 at 05:48:39PM +0200, Lilith Gkini wrote: >> The on_freelist() doesn't have a way to handle the edgecase of having a >> full freelist that doesn't end in NULL and instead has another valid >> pointer in the slab as a result of a Use-After-Free or anything similar. >> >> This case won't get caught by check_valid_pointer() and it will result in >> nr incrementing to `slab->objects + 1`, corrupting the slab->inuse entry >> later in the code by setting it to -1. >> >> The Patch adds an if check to detect that case, notifies us and handles >> the freelist and slab appropriately, as is the standard process in these >> situations. >> >> Furthermore the Patch changes the return type of the function from >> int to bool as per codying style guidelines. > > nit: codying -> coding > >> It also moves the `break;` line inside the `if (object) {` to make it more >> obvious that the code breaks the while loop in that branch. >> >> Signed-off-by: Lilith Persefoni Gkini <lilithgkini@xxxxxxxxx> >> --- > > Looks good to me, > Reviewed-by: Harry Yoo <harry.yoo@xxxxxxxxxx> >
