On Mon, Jan 20, 2025 at 5:30 PM Hyesoo Yu <hyesoo.yu@xxxxxxxxxxx> wrote: > > If a slab object is corrupted or an error occurs in its internal > value, continuing after restoration may cause other side effects. > At this point, it is difficult to debug because the problem occurred > in the past. A flag has been added that can cause a panic when there > is a problem with the object. Hi Hyesoo, I'm concerned about this because it goes against the effort to avoid introducing new BUG() calls [1]. And I think it would be more appropriate to use existing panic_on_warn functionality [2] which causes a panic on WARN(), rather than introducing a SLUB-specific knob to do the same thing. However SLUB does not call WARN() and uses pr_err() instead when reporting an error. Vlastimil and I talked about changing it to use WARN() a while ago [3], but neither of us have done that yet. Probably you may want to look at it, as it also aligns with your purpose? FYI, if you would like to work on it, please make sure that it WARN() is suppressed during kunit test. [1] https://docs.kernel.org/process/deprecated.html#bug-and-bug-on [2] https://www.kernel.org/doc/html/v6.9/admin-guide/sysctl/kernel.html#panic-on-warn [3] https://lore.kernel.org/linux-mm/d4219cd9-32d3-4697-93b9-6a44bf77d50c@xxxxxxx Best, Hyeonggon > Signed-off-by: Hyesoo Yu <hyesoo.yu@xxxxxxxxxxx> > Change-Id: I4e7e5e0ec3421a7f6c84d591db052f79d3775493 > --- > Documentation/mm/slub.rst | 2 ++ > include/linux/slab.h | 4 ++++ > mm/slub.c | 14 ++++++++++++++ > 3 files changed, 20 insertions(+) > > diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst > index 84ca1dc94e5e..ce58525db93d 100644 > --- a/Documentation/mm/slub.rst > +++ b/Documentation/mm/slub.rst > @@ -53,6 +53,7 @@ Possible debug options are:: > U User tracking (free and alloc) > T Trace (please only use on single slabs) > A Enable failslab filter mark for the cache > + C Panic if object corruption is checked. > O Switch debugging off for caches that would have > caused higher minimum slab orders > - Switch all debugging off (useful if the kernel is > @@ -113,6 +114,7 @@ options from the ``slab_debug`` parameter translate to the following files:: > U store_user > T trace > A failslab > + C corruption_panic > > failslab file is writable, so writing 1 or 0 will enable or disable > the option at runtime. Write returns -EINVAL if cache is an alias. > diff --git a/include/linux/slab.h b/include/linux/slab.h > index 10a971c2bde3..4391c30564d6 100644 > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -31,6 +31,7 @@ enum _slab_flag_bits { > _SLAB_CACHE_DMA32, > _SLAB_STORE_USER, > _SLAB_PANIC, > + _SLAB_CORRUPTION_PANIC, > _SLAB_TYPESAFE_BY_RCU, > _SLAB_TRACE, > #ifdef CONFIG_DEBUG_OBJECTS > @@ -97,6 +98,9 @@ enum _slab_flag_bits { > #define SLAB_STORE_USER __SLAB_FLAG_BIT(_SLAB_STORE_USER) > /* Panic if kmem_cache_create() fails */ > #define SLAB_PANIC __SLAB_FLAG_BIT(_SLAB_PANIC) > +/* Panic if object corruption is checked */ > +#define SLAB_CORRUPTION_PANIC __SLAB_FLAG_BIT(_SLAB_CORRUPTION_PANIC) > + > /** > * define SLAB_TYPESAFE_BY_RCU - **WARNING** READ THIS! > * > diff --git a/mm/slub.c b/mm/slub.c > index 48cefc969480..36a8dabf1349 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -1306,6 +1306,8 @@ slab_pad_check(struct kmem_cache *s, struct slab *slab) > fault, end - 1, fault - start); > print_section(KERN_ERR, "Padding ", pad, remainder); > > + BUG_ON(s->flags & SLAB_CORRUPTION_PANIC); > + > restore_bytes(s, "slab padding", POISON_INUSE, fault, end); > } > > @@ -1389,6 +1391,8 @@ static int check_object(struct kmem_cache *s, struct slab *slab, > if (!ret && !slab_in_kunit_test()) { > print_trailer(s, slab, object); > add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE); > + > + BUG_ON(s->flags & SLAB_CORRUPTION_PANIC); > } > > return ret; > @@ -1689,6 +1693,9 @@ parse_slub_debug_flags(char *str, slab_flags_t *flags, char **slabs, bool init) > case 'a': > *flags |= SLAB_FAILSLAB; > break; > + case 'c': > + *flags |= SLAB_CORRUPTION_PANIC; > + break; > case 'o': > /* > * Avoid enabling debugging on caches if its minimum > @@ -6874,6 +6881,12 @@ static ssize_t store_user_show(struct kmem_cache *s, char *buf) > > SLAB_ATTR_RO(store_user); > > +static ssize_t corruption_panic_show(struct kmem_cache *s, char *buf) > +{ > + return sysfs_emit(buf, "%d\n", !!(s->flags & SLAB_CORRUPTION_PANIC)); > +} > +SLAB_ATTR_RO(corruption_panic); > + > static ssize_t validate_show(struct kmem_cache *s, char *buf) > { > return 0; > @@ -7092,6 +7105,7 @@ static struct attribute *slab_attrs[] = { > &red_zone_attr.attr, > &poison_attr.attr, > &store_user_attr.attr, > + &corruption_panic_attr.attr, > &validate_attr.attr, > #endif > #ifdef CONFIG_ZONE_DMA > -- > 2.48.0 >
