[PATCH v2] mm/slub: fix lost local objects when bulk remote free batch fills

[PATCH v2] mm/slub: fix lost local objects when bulk remote free batch fills

[hu.shengming]

From: Shengming Hu <hu.shengming@zte.com.cn>

In free_to_pcs_bulk(), when remote_objects[] fills to PCS_BATCH_MAX,
the code jumps to flush_remote to free the batch. If all remote entries
have already been compacted out of p[] via tail swaps while local objects
remain, the flush_remote path returns early since `i < size` no longer
holds. The leftover local objects are then neither cached in the sheaf 
nor returned to the slab freelist, causing a memory leak.

For illustration:
 size = 64, local objects at p[0..31], remote objects at p[32..63]
 After scanning all remotes: i = 32, size = 32
 p[0..31] local objects are dropped.

Harry pointed out that, although the logic contains a real leak, it does
not appear to be triggerable with the current in-tree users. To hit this
path, at least PCS_BATCH_MAX objects, currently hardcoded to 32, need to
be collected in remote_objects[]. Looking at current kmem_cache_free_bulk()
users:

* maple_node has sheaf_capacity = 32
* skbuff_head_cache has sheaf_capacity = 28
* panthor and msm drivers have sheaf_capacity = 4

The sheaf capacity is, at least for now, derived purely from the object
size, with the user-requested capacity used as a minimum. Therefore, among
the current users, only maple_node has a sheaf_capacity large enough to
reach PCS_BATCH_MAX.

However, for the bug to trigger in maple_node, all objects in the sheaf
would have to be from remote nodes. In that case, there would be no local
objects left to leak. So this issue was found by code review rather than
from a runtime report, and it does not seem to be triggerable by current
users.

Still, the bug could become reachable with future users, a different sheaf
capacity, or a change to PCS_BATCH_MAX. Fix the logic by freeing a full
remote batch in place during the scan and then continuing to process the
compacted array. This keeps all local objects on the normal fast path,
while the tail path only handles any leftover partial remote batch. The
redundant next_remote_batch jump label is removed as well.

Fixes: <989b09b73978>("slab: skip percpu sheaves for remote object freeing")
Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
---
Changes in v2:
- Incorporate Harry's detailed trigger condition analysis (thanks Harry)
- Link to v1: https://lore.kernel.org/linux-mm/20260704160450302fnznThT3hBeBiwlhE-oGv@zte.com.cn/

---
 mm/slub.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 9f754cf1c187..4d7e3067a5d6 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -6193,7 +6193,6 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 	void *remote_objects[PCS_BATCH_MAX];
 	unsigned int remote_nr = 0;

-next_remote_batch:
 	while (i < size) {
 		struct slab *slab = virt_to_slab(p[i]);

@@ -6208,8 +6207,11 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 		if (unlikely(!can_free_to_pcs(slab))) {
 			remote_objects[remote_nr] = p[i];
 			p[i] = p[--size];
-			if (++remote_nr >= PCS_BATCH_MAX)
-				goto flush_remote;
+			if (++remote_nr >= PCS_BATCH_MAX) {
+				__kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
+				stat_add(s, FREE_SLOWPATH, remote_nr);
+				remote_nr = 0;
+			}
 			continue;
 		}

@@ -6293,10 +6295,6 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 	if (remote_nr) {
 		__kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
 		stat_add(s, FREE_SLOWPATH, remote_nr);
-		if (i < size) {
-			remote_nr = 0;
-			goto next_remote_batch;
-		}
 	}
 }

-- 
2.25.1

Re: [PATCH v2] mm/slub: fix lost local objects when bulk remote free batch fills

[Vlastimil Babka (SUSE)]

On 7/6/26 15:39, hu.shengming@zte.com.cn wrote:
> From: Shengming Hu <hu.shengming@zte.com.cn>
> 
> In free_to_pcs_bulk(), when remote_objects[] fills to PCS_BATCH_MAX,
> the code jumps to flush_remote to free the batch. If all remote entries
> have already been compacted out of p[] via tail swaps while local objects
> remain, the flush_remote path returns early since `i < size` no longer
> holds. The leftover local objects are then neither cached in the sheaf 
> nor returned to the slab freelist, causing a memory leak.
> 
> For illustration:
>  size = 64, local objects at p[0..31], remote objects at p[32..63]
>  After scanning all remotes: i = 32, size = 32
>  p[0..31] local objects are dropped.
> 
> Harry pointed out that, although the logic contains a real leak, it does
> not appear to be triggerable with the current in-tree users. To hit this
> path, at least PCS_BATCH_MAX objects, currently hardcoded to 32, need to
> be collected in remote_objects[]. Looking at current kmem_cache_free_bulk()
> users:
> 
> * maple_node has sheaf_capacity = 32
> * skbuff_head_cache has sheaf_capacity = 28
> * panthor and msm drivers have sheaf_capacity = 4
> 
> The sheaf capacity is, at least for now, derived purely from the object
> size, with the user-requested capacity used as a minimum. Therefore, among
> the current users, only maple_node has a sheaf_capacity large enough to
> reach PCS_BATCH_MAX.
> 
> However, for the bug to trigger in maple_node, all objects in the sheaf
> would have to be from remote nodes. In that case, there would be no local
> objects left to leak. So this issue was found by code review rather than
> from a runtime report, and it does not seem to be triggerable by current
> users.
> 
> Still, the bug could become reachable with future users, a different sheaf
> capacity, or a change to PCS_BATCH_MAX. Fix the logic by freeing a full
> remote batch in place during the scan and then continuing to process the
> compacted array. This keeps all local objects on the normal fast path,
> while the tail path only handles any leftover partial remote batch. The
> redundant next_remote_batch jump label is removed as well.
> 
> Fixes: <989b09b73978>("slab: skip percpu sheaves for remote object freeing")

Removed the < >

> Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>

Thanks! I added cc: stable anyway so we avoid unexpected surprises in case
something else is backported there that exposes the bug. And the fix is
small enough.

Merged to slab/for-next-fixes

> ---
> Changes in v2:
> - Incorporate Harry's detailed trigger condition analysis (thanks Harry)
> - Link to v1: https://lore.kernel.org/linux-mm/20260704160450302fnznThT3hBeBiwlhE-oGv@zte.com.cn/
> 
> ---
>  mm/slub.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 9f754cf1c187..4d7e3067a5d6 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -6193,7 +6193,6 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>  	void *remote_objects[PCS_BATCH_MAX];
>  	unsigned int remote_nr = 0;
> 
> -next_remote_batch:
>  	while (i < size) {
>  		struct slab *slab = virt_to_slab(p[i]);
> 
> @@ -6208,8 +6207,11 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>  		if (unlikely(!can_free_to_pcs(slab))) {
>  			remote_objects[remote_nr] = p[i];
>  			p[i] = p[--size];
> -			if (++remote_nr >= PCS_BATCH_MAX)
> -				goto flush_remote;
> +			if (++remote_nr >= PCS_BATCH_MAX) {
> +				__kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
> +				stat_add(s, FREE_SLOWPATH, remote_nr);
> +				remote_nr = 0;
> +			}
>  			continue;
>  		}
> 
> @@ -6293,10 +6295,6 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>  	if (remote_nr) {
>  		__kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
>  		stat_add(s, FREE_SLOWPATH, remote_nr);
> -		if (i < size) {
> -			remote_nr = 0;
> -			goto next_remote_batch;
> -		}
>  	}
>  }
> 

Re: [PATCH v2] mm/slub: fix lost local objects when bulk remote free batch fills

[hu.shengming]

Vlastimil wrote:
> On 7/6/26 15:39, hu.shengming@zte.com.cn wrote:
> > From: Shengming Hu <hu.shengming@zte.com.cn>
> > 
> > In free_to_pcs_bulk(), when remote_objects[] fills to PCS_BATCH_MAX,
> > the code jumps to flush_remote to free the batch. If all remote entries
> > have already been compacted out of p[] via tail swaps while local objects
> > remain, the flush_remote path returns early since `i < size` no longer
> > holds. The leftover local objects are then neither cached in the sheaf 
> > nor returned to the slab freelist, causing a memory leak.
> > 
> > For illustration:
> >  size = 64, local objects at p[0..31], remote objects at p[32..63]
> >  After scanning all remotes: i = 32, size = 32
> >  p[0..31] local objects are dropped.
> > 
> > Harry pointed out that, although the logic contains a real leak, it does
> > not appear to be triggerable with the current in-tree users. To hit this
> > path, at least PCS_BATCH_MAX objects, currently hardcoded to 32, need to
> > be collected in remote_objects[]. Looking at current kmem_cache_free_bulk()
> > users:
> > 
> > * maple_node has sheaf_capacity = 32
> > * skbuff_head_cache has sheaf_capacity = 28
> > * panthor and msm drivers have sheaf_capacity = 4
> > 
> > The sheaf capacity is, at least for now, derived purely from the object
> > size, with the user-requested capacity used as a minimum. Therefore, among
> > the current users, only maple_node has a sheaf_capacity large enough to
> > reach PCS_BATCH_MAX.
> > 
> > However, for the bug to trigger in maple_node, all objects in the sheaf
> > would have to be from remote nodes. In that case, there would be no local
> > objects left to leak. So this issue was found by code review rather than
> > from a runtime report, and it does not seem to be triggerable by current
> > users.
> > 
> > Still, the bug could become reachable with future users, a different sheaf
> > capacity, or a change to PCS_BATCH_MAX. Fix the logic by freeing a full
> > remote batch in place during the scan and then continuing to process the
> > compacted array. This keeps all local objects on the normal fast path,
> > while the tail path only handles any leftover partial remote batch. The
> > redundant next_remote_batch jump label is removed as well.
> > 
> > Fixes: <989b09b73978>("slab: skip percpu sheaves for remote object freeing")
> 
> Removed the < >
> 

Oh, thanks :)

> > Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
> 
> Thanks! I added cc: stable anyway so we avoid unexpected surprises in case
> something else is backported there that exposes the bug. And the fix is
> small enough.
> 
> Merged to slab/for-next-fixes

Sounds good to me. Thanks for taking it!

--
With Best Regards,
Shengming