Re: [PATCH] slub: print just the function name in /sys/kernel/slab/*/ctor
Alexey Dobriyan <[email protected]>
| Newsgroups | org.kvack.linux-mm |
|---|---|
| Message-ID | <68e28ff5-8cfd-496a-a895-30de110b0214@p183> |
On Tue, Aug 04, 2026 at 07:25:15PM +0200, Vlastimil Babka (SUSE) wrote: > On 8/2/26 07:13, Harry Yoo wrote: > > On Thu, Jul 30, 2026 at 04:15:26PM +0200, Vlastimil Babka (SUSE) wrote: > >> On 7/27/26 07:27, Harry Yoo wrote: > >> > > >> > > >> > On 7/25/26 10:28 PM, Alexey Dobriyan wrote: > >> >> Sysfs "ctor" file prints offset/length of the cache's ctor function > >> >> > >> >> $ sudo cat /sys/kernel/slab/bdev_cache/ctor > >> >> init_once+0x0/0x10 > >> >> > >> >> This is not useful: > >> >> > >> >> Offset will always be 0 because ctor is a function. > >> >> > >> >> I'm not sure what ctor function size is doing here, it should be in > >> >> /proc/kallsyms > >> >> > >> >> Signed-off-by: Alexey Dobriyan <[email protected]> > >> >> --- > >> > > >> > I'm not convinced that changing this (without strong justification) > >> > after exposing it to sysfs for 10+ years is worth the trouble. > >> > >> Agreed. It's a pity this was exported in the first place. I can't see a > >> benefit for anyone knowing what the function is called. Should have been at > >> most a flag whether there's ctor or not. > > > > Agreed. > > > >> But possibly a justification is not to leak the function size, which might > >> be theoretically (although unlikely) a hint to some attack. > > > > ...but you need to be the owner (root) to read this :P > > > >> Well at least if somebody complains about getting broken, it's trivial to > >> revert and we can hear about their usecase. > > > > If we were to experiment with this, we'll change it to print a flag > > rather than the ctor's name, right? > > Yep, let's try that. Alexey? I don't mind ctor name, it is internal implementation detail, but offset/size are kind of useless. My order of preferences is 1) delete +0x0/... part altogeter, 2) when someone complains, restore it as "+0x0/0x4" which is a minimum function size across all archs (or some other fixed fake size). > > >> In-tree tools (slabinfo etc) were checked to work properly? > > > > Looks like it doesn't read ctor at all. > > > > Commit a87615b8f9e2 ("SLUB: slabinfo upgrade") added -o/--ops option > > that should "Display of ctor / dtor etc.", but it reads "ops" attribute > > that has never existed? Bit puzzled. > > Might have been stale code from some previous revisions that included dtor. > > >> >> mm/slub.c | 2 +- > >> >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> >> > >> >> --- a/mm/slub.c > >> >> +++ b/mm/slub.c > >> >> @@ -9149,7 +9149,7 @@ static ssize_t ctor_show(struct kmem_cache *s, char *buf) > >> >> { > >> >> if (!s->ctor) > >> >> return 0; > >> >> - return sysfs_emit(buf, "%pS\n", s->ctor); > >> >> + return sysfs_emit(buf, "%ps\n", s->ctor); > >> >> } > >> >> SLAB_ATTR_RO(ctor);