mm/shrinker: avoid out-of-bounds read in set_shrinker_bit()

[PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit()

Posted by David Carlier 1 month ago

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

Re: [PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit()

Posted by Muchun Song 1 month ago


> 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.

Re: [PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit()

Posted by Qi Zheng 1 month ago

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);

Re: [PATCH] mm/shrinker: avoid out-of-bounds read in set_shrinker_bit()

Posted by David CARLIER 1 month ago

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);
>