Dear Andrew,
On Tue, Aug 11, 2020 at 01:49:09PM -0700, Andrew Morton wrote:
On Tue, 11 Aug 2020 14:46:56 +0200 Eugeniu Rosca erosca@de.adit-jv.com wrote:
Commit 52f23478081ae0 ("mm/slub.c: fix corrupted freechain in deactivate_slab()") suffered an update when picked up from LKML [1].
Specifically, relocating 'freelist = NULL' into 'freelist_corrupted()' created a no-op statement. Fix it by sticking to the behavior intended in the original patch [1]. Prefer the lowest-line-count solution.
[1] https://lore.kernel.org/linux-mm/20200331031450.12182-1-dongli.zhang@oracle....
...
--- a/mm/slub.c +++ b/mm/slub.c @@ -677,7 +677,6 @@ static bool freelist_corrupted(struct kmem_cache *s, struct page *page, if ((s->flags & SLAB_CONSISTENCY_CHECKS) && !check_valid_pointer(s, page, nextfree)) { object_err(s, page, freelist, "Freechain corrupt");
freelist = NULL;@@ -2184,8 +2183,10 @@ static void deactivate_slab(struct kmem_cache *s, struct page *page, * 'freelist' is already corrupted. So isolate all objects * starting at 'freelist'. */
if (freelist_corrupted(s, page, freelist, nextfree))
if (freelist_corrupted(s, page, freelist, nextfree)) {freelist = NULL; break;}do { prior = page->freelist;
Looks right.
What are the runtime effects of this change? In other words, what are the end user visible effects of the present defect?
Thank you for the prompt feedback.
The issue popped up as a result of static analysis and code review. Therefore, I lack any specific runtime behavior example being fixed. Nevertheless, I think this does not diminish the concern expressed in the description of the patch.