Message ID | 20241030133208.41061-1-marc.c.dionne@gmail.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [v3] tools/mm: Fix slabinfo crash when MAX_SLABS is exceeded | expand |
On Wed, 30 Oct 2024 10:32:08 -0300 Marc Dionne <marc.c.dionne@gmail.com> wrote: > From: Marc Dionne <marc.dionne@auristor.com> > > The number of slabs can easily exceed the hard coded MAX_SLABS in the > slabinfo tool, causing it to overwrite memory and crash. > > Increase the value of MAX_SLABS, and check if that has been exceeded for > each new slab, instead of at the end when it's already too late. Also > move the check for MAX_ALIASES into the loop body. > > @@ -1240,6 +1240,8 @@ static void read_slab_dir(void) > p--; > alias->ref = strdup(p); > alias++; > + if (alias - aliasinfo == MAX_ALIASES) > + fatal("Too many aliases\n"); Again, this is not correct. It has a potential off-by-one error. If at this point, (alias - aliasinfo == MAX_ALIASES), we *do not know* whether there are "Too many aliases". Because the parsing might have reached the end of input, in which case we're fine. A fix for this is to check for an invalid `alias' immediately before we use it, as I described in the previous email.
diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c index cfaeaea71042..bf770101929a 100644 --- a/tools/mm/slabinfo.c +++ b/tools/mm/slabinfo.c @@ -21,7 +21,7 @@ #include <regex.h> #include <errno.h> -#define MAX_SLABS 500 +#define MAX_SLABS 2000 #define MAX_ALIASES 500 #define MAX_NODES 1024 @@ -1240,6 +1240,8 @@ static void read_slab_dir(void) p--; alias->ref = strdup(p); alias++; + if (alias - aliasinfo == MAX_ALIASES) + fatal("Too many aliases\n"); break; case DT_DIR: if (chdir(de->d_name)) @@ -1301,6 +1303,8 @@ static void read_slab_dir(void) if (slab->name[0] == ':') alias_targets++; slab++; + if (slab - slabinfo == MAX_SLABS) + fatal("Too many slabs\n"); break; default : fatal("Unknown file type %lx\n", de->d_type); @@ -1310,10 +1314,6 @@ static void read_slab_dir(void) slabs = slab - slabinfo; actual_slabs = slabs; aliases = alias - aliasinfo; - if (slabs > MAX_SLABS) - fatal("Too many slabs\n"); - if (aliases > MAX_ALIASES) - fatal("Too many aliases\n"); } static void output_slabs(void)