lkml.org 
[lkml]   [2022]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] kobject: Refactor kobject_set_name_vargs()
    On Fri, May 06, 2022 at 02:46:47PM +0000, David Laight wrote:
    > From: Jagdish Gediya
    > > Sent: 06 May 2022 14:33
    > >
    > > Setting name as per the format is not only useful for kobjects.
    > > It can also be used to set name for other things for e.g. setting
    > > the name of the struct attribute when multiple same kind of attributes
    > > need to be created with some identifier in name, instead of managing
    > > memory for names at such places case by case, it would be good if
    > > something like current kobject_set_name_vargs() can be utilized.
    > >
    > > Refactor kobject_set_name_vargs(), Create a new generic function
    > > set_name_vargs() which can be used for kobjects as well as at
    > > other places.
    > >
    > > This patch doesn't introduce any functionality change.
    > >
    > > Signed-off-by: Jagdish Gediya <jvgediya@linux.ibm.com>
    > > ---
    > > include/linux/string.h | 1 +
    > > lib/kobject.c | 30 +-----------------------------
    > > mm/util.c | 40 ++++++++++++++++++++++++++++++++++++++++
    > > 3 files changed, 42 insertions(+), 29 deletions(-)
    > >
    > > diff --git a/include/linux/string.h b/include/linux/string.h
    > > index b6572aeca2f5..f329962e5ae9 100644
    > > --- a/include/linux/string.h
    > > +++ b/include/linux/string.h
    > > @@ -9,6 +9,7 @@
    > > #include <linux/stdarg.h>
    > > #include <uapi/linux/string.h>
    > >
    > > +int set_name_vargs(const char **name, const char *fmt, va_list vargs);
    > > extern char *strndup_user(const char __user *, long);
    > > extern void *memdup_user(const void __user *, size_t);
    > > extern void *vmemdup_user(const void __user *, size_t);
    > > diff --git a/lib/kobject.c b/lib/kobject.c
    > > index 5f0e71ab292c..870d05971e3a 100644
    > > --- a/lib/kobject.c
    > > +++ b/lib/kobject.c
    > > @@ -249,35 +249,7 @@ static int kobject_add_internal(struct kobject *kobj)
    > > int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
    > > va_list vargs)
    > > {
    > ...
    > > + return set_name_vargs(&kobj->name, fmt, vargs);
    > > }
    > >
    > > /**
    > > diff --git a/mm/util.c b/mm/util.c
    > > index 54e5e761a9a9..808d29b17ea7 100644
    > > --- a/mm/util.c
    > > +++ b/mm/util.c
    > > @@ -112,6 +112,46 @@ char *kstrndup(const char *s, size_t max, gfp_t gfp)
    > > }
    > > EXPORT_SYMBOL(kstrndup);
    > >
    > > +/**
    > > + * set_name_vargs() - Set the name as per format
    > > + * @name: pointer to point to the name as per format
    > > + * @fmt: format string used to build the name
    > > + * @vargs: vargs to format the string.
    > > + */
    > > +int set_name_vargs(const char **name, const char *fmt, va_list vargs)
    > > +{
    > > + const char *s;
    > > +
    > > + if (*name && !fmt)
    > > + return 0;
    > > +
    > > + s = kvasprintf_const(GFP_KERNEL, fmt, vargs);
    > > + if (!s)
    > > + return -ENOMEM;
    > > +
    > > + /*
    > > + * ewww... some of these buggers have '/' in the name ... If
    > > + * that's the case, we need to make sure we have an actual
    > > + * allocated copy to modify, since kvasprintf_const may have
    > > + * returned something from .rodata.
    > > + */
    > > + if (strchr(s, '/')) {
    > > + char *t;
    > > +
    > > + t = kstrdup(s, GFP_KERNEL);
    > > + kfree_const(s);
    > > + if (!t)
    > > + return -ENOMEM;
    >
    > There is a kstrdup_const() that will DTRT here.
    >
    > > + strreplace(t, '/', '!');
    > > + s = t;
    > > + }
    > > + kfree_const(*name);
    > > + *name = s;
    > > +
    > > + return 0;
    > > +}
    > > +EXPORT_SYMBOL(set_name_vargs);
    >
    > Are you sure this can ever work from a module?
    > This all relies on:
    >
    > static inline bool is_kernel_rodata(unsigned long addr)
    > {
    > return addr >= (unsigned long)__start_rodata &&
    > addr < (unsigned long)__end_rodata;
    > }
    >
    > which isn't going to do anything sane given an "xxx" inside a module.
    >
    > Indeed can kobject_set_name_vargs() end up with a constant string
    > inside a module?

    No, kobject_set_name_vargs() is not exported. I exported
    set_name_vargs() because it can have a broader use, but you are right
    it shouldn't be exported.

    > Seems horribly fragile.
    >
    > David
    >
    > > +
    > > /**
    > > * kmemdup - duplicate region of memory
    > > *
    > > --
    > > 2.35.1
    >
    > -
    > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
    > Registration No: 1397386 (Wales)
    >
    >

    \
     
     \ /
      Last update: 2022-05-06 19:25    [W:3.218 / U:0.152 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site