Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit()
@ 2026-05-10 18:37 David Carlier
  2026-05-11  2:45 ` Qi Zheng
  2026-05-11  2:48 ` Muchun Song
  0 siblings, 2 replies; 4+ messages in thread
From: David Carlier @ 2026-05-10 18:37 UTC (permalink / raw)
  To: Andrew Morton, Dave Chinner, Qi Zheng, Roman Gushchin,
	Muchun Song, linux-mm, linux-kernel
  Cc: David Carlier

set_shrinker_bit() reads info->unit[shrinker_id_to_index(shrinker_id)]
before checking shrinker_id against info->map_nr_max, so an id past the
currently visible map_nr_max reads past the unit[] array before the
WARN_ON_ONCE() catches it.

Move the load into the bounded branch.

Fixes: 307bececcd12 ("mm: shrinker: add a secondary array for shrinker_info::{map, nr_deferred}")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 mm/shrinker.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/shrinker.c b/mm/shrinker.c
index 76b3f750cf65..49256f81199f 100644
--- a/mm/shrinker.c
+++ b/mm/shrinker.c
@@ -197,12 +197,13 @@ void set_shrinker_bit(struct mem_cgroup *memcg, int nid, int shrinker_id)
 {
 	if (shrinker_id >= 0 && memcg && !mem_cgroup_is_root(memcg)) {
 		struct shrinker_info *info;
-		struct shrinker_info_unit *unit;
 
 		rcu_read_lock();
 		info = rcu_dereference(memcg->nodeinfo[nid]->shrinker_info);
-		unit = info->unit[shrinker_id_to_index(shrinker_id)];
 		if (!WARN_ON_ONCE(shrinker_id >= info->map_nr_max)) {
+			struct shrinker_info_unit *unit;
+
+			unit = info->unit[shrinker_id_to_index(shrinker_id)];
 			/* Pairs with smp mb in shrink_slab() */
 			smp_mb__before_atomic();
 			set_bit(shrinker_id_to_offset(shrinker_id), unit->map);
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit()
  2026-05-10 18:37 [PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit() David Carlier
@ 2026-05-11  2:45 ` Qi Zheng
  2026-05-11  2:46   ` David CARLIER
  2026-05-11  2:48 ` Muchun Song
  1 sibling, 1 reply; 4+ messages in thread
From: Qi Zheng @ 2026-05-11  2:45 UTC (permalink / raw)
  To: David Carlier, Andrew Morton, Dave Chinner, Roman Gushchin,
	Muchun Song, linux-mm, linux-kernel

Hi David,

On 5/11/26 2:37 AM, David Carlier wrote:
> set_shrinker_bit() reads info->unit[shrinker_id_to_index(shrinker_id)]
> before checking shrinker_id against info->map_nr_max, so an id past the
> currently visible map_nr_max reads past the unit[] array before the
> WARN_ON_ONCE() catches it.

Did you run into this problem in practice, of just find it via code
inspection? It's virtually impossible to happened, which is why
WARN_ON_ONCE() was added to catch it.

> 
> Move the load into the bounded branch.
> 
> Fixes: 307bececcd12 ("mm: shrinker: add a secondary array for shrinker_info::{map, nr_deferred}")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>   mm/shrinker.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

Anyway, It LGTM, so:

Reviewed-by: Qi Zheng <qi.zheng@linux.dev>

Thanks,
Qi

> 
> diff --git a/mm/shrinker.c b/mm/shrinker.c
> index 76b3f750cf65..49256f81199f 100644
> --- a/mm/shrinker.c
> +++ b/mm/shrinker.c
> @@ -197,12 +197,13 @@ void set_shrinker_bit(struct mem_cgroup *memcg, int nid, int shrinker_id)
>   {
>   	if (shrinker_id >= 0 && memcg && !mem_cgroup_is_root(memcg)) {
>   		struct shrinker_info *info;
> -		struct shrinker_info_unit *unit;
>   
>   		rcu_read_lock();
>   		info = rcu_dereference(memcg->nodeinfo[nid]->shrinker_info);
> -		unit = info->unit[shrinker_id_to_index(shrinker_id)];
>   		if (!WARN_ON_ONCE(shrinker_id >= info->map_nr_max)) {
> +			struct shrinker_info_unit *unit;
> +
> +			unit = info->unit[shrinker_id_to_index(shrinker_id)];
>   			/* Pairs with smp mb in shrink_slab() */
>   			smp_mb__before_atomic();
>   			set_bit(shrinker_id_to_offset(shrinker_id), unit->map);



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit()
  2026-05-11  2:45 ` Qi Zheng
@ 2026-05-11  2:46   ` David CARLIER
  0 siblings, 0 replies; 4+ messages in thread
From: David CARLIER @ 2026-05-11  2:46 UTC (permalink / raw)
  To: Qi Zheng
  Cc: Andrew Morton, Dave Chinner, Roman Gushchin, Muchun Song,
	linux-mm, linux-kernel

Hi Qi,

On Mon, 11 May 2026 at 03:45, Qi Zheng <qi.zheng@linux.dev> wrote:
>
> Hi David,
>
> On 5/11/26 2:37 AM, David Carlier wrote:
> > set_shrinker_bit() reads info->unit[shrinker_id_to_index(shrinker_id)]
> > before checking shrinker_id against info->map_nr_max, so an id past the
> > currently visible map_nr_max reads past the unit[] array before the
> > WARN_ON_ONCE() catches it.
>
> Did you run into this problem in practice, of just find it via code
> inspection? It's virtually impossible to happened, which is why
> WARN_ON_ONCE() was added to catch it.

Definitely code inspection.

>
> >
> > Move the load into the bounded branch.
> >
> > Fixes: 307bececcd12 ("mm: shrinker: add a secondary array for shrinker_info::{map, nr_deferred}")
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
> >   mm/shrinker.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
>
> Anyway, It LGTM, so:
>
> Reviewed-by: Qi Zheng <qi.zheng@linux.dev>
>
> Thanks,
> Qi

Thanks !
>
> >
> > diff --git a/mm/shrinker.c b/mm/shrinker.c
> > index 76b3f750cf65..49256f81199f 100644
> > --- a/mm/shrinker.c
> > +++ b/mm/shrinker.c
> > @@ -197,12 +197,13 @@ void set_shrinker_bit(struct mem_cgroup *memcg, int nid, int shrinker_id)
> >   {
> >       if (shrinker_id >= 0 && memcg && !mem_cgroup_is_root(memcg)) {
> >               struct shrinker_info *info;
> > -             struct shrinker_info_unit *unit;
> >
> >               rcu_read_lock();
> >               info = rcu_dereference(memcg->nodeinfo[nid]->shrinker_info);
> > -             unit = info->unit[shrinker_id_to_index(shrinker_id)];
> >               if (!WARN_ON_ONCE(shrinker_id >= info->map_nr_max)) {
> > +                     struct shrinker_info_unit *unit;
> > +
> > +                     unit = info->unit[shrinker_id_to_index(shrinker_id)];
> >                       /* Pairs with smp mb in shrink_slab() */
> >                       smp_mb__before_atomic();
> >                       set_bit(shrinker_id_to_offset(shrinker_id), unit->map);
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit()
  2026-05-10 18:37 [PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit() David Carlier
  2026-05-11  2:45 ` Qi Zheng
@ 2026-05-11  2:48 ` Muchun Song
  1 sibling, 0 replies; 4+ messages in thread
From: Muchun Song @ 2026-05-11  2:48 UTC (permalink / raw)
  To: David Carlier
  Cc: Andrew Morton, Dave Chinner, Qi Zheng, Roman Gushchin, linux-mm,
	linux-kernel



> On May 11, 2026, at 02:37, David Carlier <devnexen@gmail.com> wrote:
> 
> set_shrinker_bit() reads info->unit[shrinker_id_to_index(shrinker_id)]
> before checking shrinker_id against info->map_nr_max, so an id past the
> currently visible map_nr_max reads past the unit[] array before the
> WARN_ON_ONCE() catches it.
> 
> Move the load into the bounded branch.
> 
> Fixes: 307bececcd12 ("mm: shrinker: add a secondary array for shrinker_info::{map, nr_deferred}")
> Signed-off-by: David Carlier <devnexen@gmail.com>

Acked-by: Muchun Song <muchun.song@linux.dev>

Thanks.



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-11  2:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-10 18:37 [PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit() David Carlier
2026-05-11  2:45 ` Qi Zheng
2026-05-11  2:46   ` David CARLIER
2026-05-11  2:48 ` Muchun Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox