| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 6 Dec 2022 19:46:47 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Matthew Wilcox <willy@...radead.org>
Cc: linux-kernel@...r.kernel.org, Jason Gunthorpe <jgg@...pe.ca>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Jason Gunthorpe <jgg@...dia.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
"Rafael J. Wysocki" <rafael@...nel.org>
Subject: Re: [PATCH v2 1/4] container_of: add container_of_const() that
preserves const-ness of the pointer
On Tue, Dec 06, 2022 at 05:18:22PM +0000, Matthew Wilcox wrote:
> On Tue, Dec 06, 2022 at 04:54:45PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Dec 06, 2022 at 03:13:29PM +0000, Matthew Wilcox wrote:
> > > On Mon, Dec 05, 2022 at 01:12:03PM +0100, Greg Kroah-Hartman wrote:
> > > > +/**
> > > > + * container_of_const - cast a member of a structure out to the containing
> > > > + * structure and preserve the const-ness of the pointer
> > > > + * @ptr: the pointer to the member
> > > > + * @type: the type of the container struct this is embedded in.
> > > > + * @member: the name of the member within the struct.
> > > > + */
> > > > +#define container_of_const(ptr, type, member) \
> > > > + _Generic(ptr, \
> > > > + const typeof(*(ptr)) *: ((const type *)container_of(ptr, type, member)),\
> > > > + default: ((type *)container_of(ptr, type, member)) \
> > > > + )
> > > > +
> > >
> > > Reviewed-by: Matthew Wilcox (Oracle) <willy@...radead.org>
> > >
> > > I tried doing this:
> > >
> > > +++ b/include/linux/container_of.h
> > > @@ -15,11 +15,17 @@
> > > *
> > > * WARNING: any const qualifier of @ptr is lost.
> > > */
> > > -#define container_of(ptr, type, member) ({ \
> > > +#define _c_of(ptr, type, member) ({ \
> > > void *__mptr = (void *)(ptr); \
> > > static_assert(__same_type(*(ptr), ((type *)0)->member) || \
> > > __same_type(*(ptr), void), \
> > > "pointer type mismatch in container_of()"); \
> > > ((type *)(__mptr - offsetof(type, member))); })
> > >
> > > +#define container_of(ptr, type, m) \
> > > + _Generic(ptr, \
> > > + const typeof(*(ptr)) *: (const type *)_c_of(ptr, type, m),\
> > > + default: ((type *)_c_of(ptr, type, m)) \
> > > + )
> > > +
> > > #endif /* _LINUX_CONTAINER_OF_H */
> > >
> > > (whitespace damaged, yes the kernel-doc is now in the wrong place, etc)
> > >
> > > It found a few problems; just building the mlx5 driver (I happened to be
> > > doing some work on it in that tree). We're definitely not ready to do
> > > that yet, but I'll send a few patches to prepare for it.
> >
> > Yeah, that's a good long-term goal to have here. Once everything is
> > moved over, switching all container_of_const() to just container_of()
> > should be simple.
>
> I found a problem in fs/dcache.c:
>
> struct qstr {
> union {
> struct {
> HASH_LEN_DECLARE;
> };
> u64 hash_len;
> };
> const unsigned char *name;
> };
>
> (note the const on "name")
>
> static inline struct external_name *external_name(struct dentry *dentry)
> {
> return container_of(dentry->d_name.name, struct external_name, name[0]);
> }
>
> dentry isn't const, but dentry->d_name.name is, so we match the const
> case and the compiler emits a warning. I don't think there's a way to
> key off the constness of dentry instead of dentry->d_name.name, so
> I've gone with the following for now. Anybody prefer a different
> approach?
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 52e6d5fdab6b..b51a86f3cec6 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -288,7 +288,8 @@ struct external_name {
>
> static inline struct external_name *external_name(struct dentry *dentry)
> {
> - return container_of(dentry->d_name.name, struct external_name, name[0]);
> + return container_of_not_const(dentry->d_name.name,
> + struct external_name, name[0]);
> }
Will just:
return container_of((unsigned char *)dentry->d_name.name, struct external_name, name[0]);
work by casting away the "const" of the name?
Yeah it's ugly, I never considered the address of a const char * being
used as a base to cast back from. The vfs is fun :)
> static void __d_free(struct rcu_head *head)
> @@ -329,7 +330,8 @@ void release_dentry_name_snapshot(struct name_snapshot *name)
> {
> if (unlikely(name->name.name != name->inline_name)) {
> struct external_name *p;
> - p = container_of(name->name.name, struct external_name, name[0]);
> + p = container_of_not_const(name->name.name,
> + struct external_name, name[0]);
> if (unlikely(atomic_dec_and_test(&p->u.count)))
> kfree_rcu(p, u.head);
> }
> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> index 23389af3af94..bf609a072754 100644
> --- a/include/linux/container_of.h
> +++ b/include/linux/container_of.h
> @@ -25,4 +25,7 @@
> const typeof(*(ptr)) *: (const type *)__mptr, \
> default: ((type *)__mptr)); })
>
> +#define container_of_not_const(ptr, type, member) \
> + (type *)container_of(ptr, type, member)
> +
"not_const" feels odd, all you want to do is cast away the pointer
result here, right?
thanks,
greg k-h
Powered by blists - more mailing lists