All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer
@ 2022-12-01 19:30 Greg Kroah-Hartman
  2022-12-01 19:30 ` [PATCH 2/4] device.h: move kobj_to_dev() to use container_of_const() Greg Kroah-Hartman
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2022-12-01 19:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, Jason Gunthorpe, Matthew Wilcox,
	Sakari Ailus, Andy Shevchenko, Rafael J. Wysocki

container_of does not preserve the const-ness of a pointer that is
passed into it, which can cause C code that passes in a const pointer to
get a pointer back that is not const and then scribble all over the data
in it.  To prevent this, container_of_const() will preserve the const
status of the pointer passed into it using the newly available _Generic()
method.

Co-developed-by: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/container_of.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 2008e9f4058c..3c290e865151 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -22,4 +22,18 @@
 		      "pointer type mismatch in container_of()");	\
 	((type *)(__mptr - offsetof(type, member))); })
 
+/**
+ * container_of_const - cast a member of a structure out to the containing
+ *			structure and preserve the const-ness of the pointer
+ * @ptr_type:		the type of the pointer @ptr
+ * @ptr:		the pointer to the member
+ * @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, ptr, member_type, member)		\
+	_Generic(ptr,							\
+		const ptr_type *: ((const member_type *)container_of(ptr, member_type, member)),\
+		ptr_type *: ((member_type *)container_of(ptr, member_type, member))	\
+	)
+
 #endif	/* _LINUX_CONTAINER_OF_H */
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/4] device.h: move kobj_to_dev() to use container_of_const()
  2022-12-01 19:30 [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Greg Kroah-Hartman
@ 2022-12-01 19:30 ` Greg Kroah-Hartman
  2022-12-01 19:30 ` [PATCH 3/4] usb.h: take advantage of container_of_const() Greg Kroah-Hartman
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2022-12-01 19:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, Thomas Gleixner, Jason Gunthorpe, Rafael J. Wysocki

Instead of rolling our own const-checking logic, use the newly
introduced container_of_const() to handle it all for us automatically.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/device.h | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index 84ae52de6746..8ad2cd21b335 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -680,26 +680,8 @@ struct device_link {
 	bool supplier_preactivated; /* Owned by consumer probe. */
 };
 
-static inline struct device *__kobj_to_dev(struct kobject *kobj)
-{
-	return container_of(kobj, struct device, kobj);
-}
-
-static inline const struct device *__kobj_to_dev_const(const struct kobject *kobj)
-{
-	return container_of(kobj, const struct device, kobj);
-}
-
-/*
- * container_of() will happily take a const * and spit back a non-const * as it
- * is just doing pointer math.  But we want to be a bit more careful in the
- * driver code, so manually force any const * of a kobject to also be a const *
- * to a device.
- */
-#define kobj_to_dev(kobj)					\
-	_Generic((kobj),					\
-		 const struct kobject *: __kobj_to_dev_const,	\
-		 struct kobject *: __kobj_to_dev)(kobj)
+#define kobj_to_dev(__kobj)	\
+	container_of_const(struct kobject, __kobj, struct device, kobj)
 
 /**
  * device_iommu_mapped - Returns true when the device DMA is translated
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/4] usb.h: take advantage of container_of_const()
  2022-12-01 19:30 [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Greg Kroah-Hartman
  2022-12-01 19:30 ` [PATCH 2/4] device.h: move kobj_to_dev() to use container_of_const() Greg Kroah-Hartman
@ 2022-12-01 19:30 ` Greg Kroah-Hartman
  2022-12-01 19:30 ` [PATCH 4/4] firmware_loader: fix up to_fw_sysfs() to preserve const Greg Kroah-Hartman
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2022-12-01 19:30 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman

Instead of rolling our own const-checking logic in to_usb_interface()
and to_usb_device() use the newly added container_of_const() instead,
making the logic much simpler overall.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/usb.h | 44 ++++----------------------------------------
 1 file changed, 4 insertions(+), 40 deletions(-)

diff --git a/include/linux/usb.h b/include/linux/usb.h
index 4b463a5e4ba2..cd5637c950ae 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -259,26 +259,8 @@ struct usb_interface {
 	struct work_struct reset_ws;	/* for resets in atomic context */
 };
 
-static inline struct usb_interface *__to_usb_interface(struct device *d)
-{
-	return container_of(d, struct usb_interface, dev);
-}
-
-static inline const struct usb_interface *__to_usb_interface_const(const struct device *d)
-{
-	return container_of(d, struct usb_interface, dev);
-}
-
-/*
- * container_of() will happily take a const * and spit back a non-const * as it
- * is just doing pointer math.  But we want to be a bit more careful in the USB
- * driver code, so manually force any const * of a device to also be a const *
- * to a usb_device.
- */
-#define to_usb_interface(dev)						\
-	_Generic((dev),							\
-		 const struct device *: __to_usb_interface_const,	\
-		 struct device *: __to_usb_interface)(dev)
+#define to_usb_interface(__dev)	\
+	container_of_const(struct device, __dev, struct usb_interface, dev)
 
 static inline void *usb_get_intfdata(struct usb_interface *intf)
 {
@@ -730,26 +712,8 @@ struct usb_device {
 	unsigned use_generic_driver:1;
 };
 
-static inline struct usb_device *__to_usb_device(struct device *d)
-{
-	return container_of(d, struct usb_device, dev);
-}
-
-static inline const struct usb_device *__to_usb_device_const(const struct device *d)
-{
-	return container_of(d, struct usb_device, dev);
-}
-
-/*
- * container_of() will happily take a const * and spit back a non-const * as it
- * is just doing pointer math.  But we want to be a bit more careful in the USB
- * driver code, so manually force any const * of a device to also be a const *
- * to a usb_device.
- */
-#define to_usb_device(dev)					\
-	_Generic((dev),						\
-		 const struct device *: __to_usb_device_const,	\
-		 struct device *: __to_usb_device)(dev)
+#define to_usb_device(__dev)	\
+	container_of_const(struct device, __dev, struct usb_device, dev)
 
 static inline struct usb_device *__intf_to_usbdev(struct usb_interface *intf)
 {
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/4] firmware_loader: fix up to_fw_sysfs() to preserve const
  2022-12-01 19:30 [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Greg Kroah-Hartman
  2022-12-01 19:30 ` [PATCH 2/4] device.h: move kobj_to_dev() to use container_of_const() Greg Kroah-Hartman
  2022-12-01 19:30 ` [PATCH 3/4] usb.h: take advantage of container_of_const() Greg Kroah-Hartman
@ 2022-12-01 19:30 ` Greg Kroah-Hartman
  2022-12-01 20:25   ` Russ Weight
  2022-12-01 22:50 ` [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Andy Shevchenko
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2022-12-01 19:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, Luis Chamberlain, Russ Weight, Rafael J. Wysocki

to_fw_sysfs() was changed in commit 23680f0b7d7f ("driver core: make
struct class.dev_uevent() take a const *") to pass in a const pointer
but not pass it back out to handle some changes in the driver core.
That isn't the best idea as it could cause problems if used incorrectly,
so switch to use the container_of_const() macro instead which will
preserve the const status of the pointer and enforce it by the compiler.

Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Russ Weight <russell.h.weight@intel.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Fixes: 23680f0b7d7f ("driver core: make struct class.dev_uevent() take a const *")
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/base/firmware_loader/sysfs.h | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/base/firmware_loader/sysfs.h b/drivers/base/firmware_loader/sysfs.h
index fd0b4ad9bdbb..0f0b1f7bf335 100644
--- a/drivers/base/firmware_loader/sysfs.h
+++ b/drivers/base/firmware_loader/sysfs.h
@@ -80,11 +80,8 @@ struct fw_sysfs {
 	struct firmware *fw;
 	void *fw_upload_priv;
 };
-
-static inline struct fw_sysfs *to_fw_sysfs(const struct device *dev)
-{
-	return container_of(dev, struct fw_sysfs, dev);
-}
+#define to_fw_sysfs(__dev)	\
+	container_of_const(struct device, __dev, struct fw_sysfs, dev)
 
 void __fw_load_abort(struct fw_priv *fw_priv);
 
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/4] firmware_loader: fix up to_fw_sysfs() to preserve const
  2022-12-01 19:30 ` [PATCH 4/4] firmware_loader: fix up to_fw_sysfs() to preserve const Greg Kroah-Hartman
@ 2022-12-01 20:25   ` Russ Weight
  0 siblings, 0 replies; 14+ messages in thread
From: Russ Weight @ 2022-12-01 20:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, linux-kernel; +Cc: Luis Chamberlain, Rafael J. Wysocki



On 12/1/22 11:30, Greg Kroah-Hartman wrote:
> to_fw_sysfs() was changed in commit 23680f0b7d7f ("driver core: make
> struct class.dev_uevent() take a const *") to pass in a const pointer
> but not pass it back out to handle some changes in the driver core.
> That isn't the best idea as it could cause problems if used incorrectly,
> so switch to use the container_of_const() macro instead which will
> preserve the const status of the pointer and enforce it by the compiler.
>
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Russ Weight <russell.h.weight@intel.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Fixes: 23680f0b7d7f ("driver core: make struct class.dev_uevent() take a const *")
Acked-by: Russ Weight <russell.h.weight@intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/base/firmware_loader/sysfs.h | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/firmware_loader/sysfs.h b/drivers/base/firmware_loader/sysfs.h
> index fd0b4ad9bdbb..0f0b1f7bf335 100644
> --- a/drivers/base/firmware_loader/sysfs.h
> +++ b/drivers/base/firmware_loader/sysfs.h
> @@ -80,11 +80,8 @@ struct fw_sysfs {
>  	struct firmware *fw;
>  	void *fw_upload_priv;
>  };
> -
> -static inline struct fw_sysfs *to_fw_sysfs(const struct device *dev)
> -{
> -	return container_of(dev, struct fw_sysfs, dev);
> -}
> +#define to_fw_sysfs(__dev)	\
> +	container_of_const(struct device, __dev, struct fw_sysfs, dev)
>  
>  void __fw_load_abort(struct fw_priv *fw_priv);
>  


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer
  2022-12-01 19:30 [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Greg Kroah-Hartman
                   ` (2 preceding siblings ...)
  2022-12-01 19:30 ` [PATCH 4/4] firmware_loader: fix up to_fw_sysfs() to preserve const Greg Kroah-Hartman
@ 2022-12-01 22:50 ` Andy Shevchenko
  2022-12-02  0:46   ` Jason Gunthorpe
  2022-12-01 23:21 ` Sakari Ailus
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2022-12-01 22:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Jason Gunthorpe, Matthew Wilcox, Sakari Ailus,
	Rafael J. Wysocki

On Thu, Dec 01, 2022 at 08:30:54PM +0100, Greg Kroah-Hartman wrote:
> container_of does not preserve the const-ness of a pointer that is
> passed into it, which can cause C code that passes in a const pointer to
> get a pointer back that is not const and then scribble all over the data
> in it.  To prevent this, container_of_const() will preserve the const
> status of the pointer passed into it using the newly available _Generic()
> method.
> 
> Co-developed-by: Jason Gunthorpe <jgg@ziepe.ca>

I believe this tag requires SoB of the co-develper.
The code looks good to me, thanks!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  include/linux/container_of.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> index 2008e9f4058c..3c290e865151 100644
> --- a/include/linux/container_of.h
> +++ b/include/linux/container_of.h
> @@ -22,4 +22,18 @@
>  		      "pointer type mismatch in container_of()");	\
>  	((type *)(__mptr - offsetof(type, member))); })
>  
> +/**
> + * container_of_const - cast a member of a structure out to the containing
> + *			structure and preserve the const-ness of the pointer
> + * @ptr_type:		the type of the pointer @ptr
> + * @ptr:		the pointer to the member
> + * @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, ptr, member_type, member)		\
> +	_Generic(ptr,							\
> +		const ptr_type *: ((const member_type *)container_of(ptr, member_type, member)),\
> +		ptr_type *: ((member_type *)container_of(ptr, member_type, member))	\
> +	)
> +
>  #endif	/* _LINUX_CONTAINER_OF_H */
> -- 
> 2.38.1
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer
  2022-12-01 19:30 [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Greg Kroah-Hartman
                   ` (3 preceding siblings ...)
  2022-12-01 22:50 ` [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Andy Shevchenko
@ 2022-12-01 23:21 ` Sakari Ailus
  2022-12-02 10:45   ` Greg Kroah-Hartman
  2022-12-02  0:48 ` Jason Gunthorpe
  2022-12-02 12:48 ` Sakari Ailus
  6 siblings, 1 reply; 14+ messages in thread
From: Sakari Ailus @ 2022-12-01 23:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Jason Gunthorpe, Matthew Wilcox, Andy Shevchenko,
	Rafael J. Wysocki

Hi Greg,

On Thu, Dec 01, 2022 at 08:30:54PM +0100, Greg Kroah-Hartman wrote:
> container_of does not preserve the const-ness of a pointer that is
> passed into it, which can cause C code that passes in a const pointer to
> get a pointer back that is not const and then scribble all over the data
> in it.  To prevent this, container_of_const() will preserve the const
> status of the pointer passed into it using the newly available _Generic()
> method.

"_const" in the name suggests that the macro would always take a const
argument. Could this be called e.g. container_of_safe() (for type-safe, but
full type_safe would be a bit long)?

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer
  2022-12-01 22:50 ` [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Andy Shevchenko
@ 2022-12-02  0:46   ` Jason Gunthorpe
  2022-12-02  6:41     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Gunthorpe @ 2022-12-02  0:46 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, linux-kernel, Matthew Wilcox, Sakari Ailus,
	Rafael J. Wysocki

On Fri, Dec 02, 2022 at 12:50:05AM +0200, Andy Shevchenko wrote:
> On Thu, Dec 01, 2022 at 08:30:54PM +0100, Greg Kroah-Hartman wrote:
> > container_of does not preserve the const-ness of a pointer that is
> > passed into it, which can cause C code that passes in a const pointer to
> > get a pointer back that is not const and then scribble all over the data
> > in it.  To prevent this, container_of_const() will preserve the const
> > status of the pointer passed into it using the newly available _Generic()
> > method.
> > 
> > Co-developed-by: Jason Gunthorpe <jgg@ziepe.ca>
> 
> I believe this tag requires SoB of the co-develper.

Sure, Greg you can add whatever tags are required

Did you look at possibly just calling this "container_of" ?

Thanks,
Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer
  2022-12-01 19:30 [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Greg Kroah-Hartman
                   ` (4 preceding siblings ...)
  2022-12-01 23:21 ` Sakari Ailus
@ 2022-12-02  0:48 ` Jason Gunthorpe
  2022-12-02 12:48 ` Sakari Ailus
  6 siblings, 0 replies; 14+ messages in thread
From: Jason Gunthorpe @ 2022-12-02  0:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Matthew Wilcox, Sakari Ailus, Andy Shevchenko,
	Rafael J. Wysocki

On Thu, Dec 01, 2022 at 08:30:54PM +0100, Greg Kroah-Hartman wrote:
> container_of does not preserve the const-ness of a pointer that is
> passed into it, which can cause C code that passes in a const pointer to
> get a pointer back that is not const and then scribble all over the data
> in it.  To prevent this, container_of_const() will preserve the const
> status of the pointer passed into it using the newly available _Generic()
> method.
> 
> Co-developed-by: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  include/linux/container_of.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

For the whole series

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer
  2022-12-02  0:46   ` Jason Gunthorpe
@ 2022-12-02  6:41     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2022-12-02  6:41 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Andy Shevchenko, linux-kernel, Matthew Wilcox, Sakari Ailus,
	Rafael J. Wysocki

On Thu, Dec 01, 2022 at 08:46:12PM -0400, Jason Gunthorpe wrote:
> On Fri, Dec 02, 2022 at 12:50:05AM +0200, Andy Shevchenko wrote:
> > On Thu, Dec 01, 2022 at 08:30:54PM +0100, Greg Kroah-Hartman wrote:
> > > container_of does not preserve the const-ness of a pointer that is
> > > passed into it, which can cause C code that passes in a const pointer to
> > > get a pointer back that is not const and then scribble all over the data
> > > in it.  To prevent this, container_of_const() will preserve the const
> > > status of the pointer passed into it using the newly available _Generic()
> > > method.
> > > 
> > > Co-developed-by: Jason Gunthorpe <jgg@ziepe.ca>
> > 
> > I believe this tag requires SoB of the co-develper.
> 
> Sure, Greg you can add whatever tags are required

I need you to send me a signed-off-by line, I can't add that on my own
for obvious reasons.

> Did you look at possibly just calling this "container_of" ?

Yes, but to do that would require all instances to be touched as this
call takes 4 parameters, while container_of() takes 3, so that can't be
done simply, AND there would be a lot of build errors all at once.

I'll work on moving code over to the new call as needed.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer
  2022-12-01 23:21 ` Sakari Ailus
@ 2022-12-02 10:45   ` Greg Kroah-Hartman
  2022-12-02 12:01     ` Andy Shevchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2022-12-02 10:45 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-kernel, Jason Gunthorpe, Matthew Wilcox, Andy Shevchenko,
	Rafael J. Wysocki

On Thu, Dec 01, 2022 at 11:21:50PM +0000, Sakari Ailus wrote:
> Hi Greg,
> 
> On Thu, Dec 01, 2022 at 08:30:54PM +0100, Greg Kroah-Hartman wrote:
> > container_of does not preserve the const-ness of a pointer that is
> > passed into it, which can cause C code that passes in a const pointer to
> > get a pointer back that is not const and then scribble all over the data
> > in it.  To prevent this, container_of_const() will preserve the const
> > status of the pointer passed into it using the newly available _Generic()
> > method.
> 
> "_const" in the name suggests that the macro would always take a const
> argument.

I mean it here to be "this will preserve const" as yes, it can take a
const.  Or not.

> Could this be called e.g. container_of_safe() (for type-safe, but
> full type_safe would be a bit long)?

const is an attribute of type safety, container_of is also type-safe
as-is, it's just the const portion here that is the difference between
the two.

Naming is hard :(

> 
> -- 
> Kind regards,
> 
> Sakari Ailus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer
  2022-12-02 10:45   ` Greg Kroah-Hartman
@ 2022-12-02 12:01     ` Andy Shevchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2022-12-02 12:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Sakari Ailus, linux-kernel, Jason Gunthorpe, Matthew Wilcox,
	Rafael J. Wysocki

On Fri, Dec 02, 2022 at 11:45:55AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Dec 01, 2022 at 11:21:50PM +0000, Sakari Ailus wrote:
> > On Thu, Dec 01, 2022 at 08:30:54PM +0100, Greg Kroah-Hartman wrote:
> > > container_of does not preserve the const-ness of a pointer that is
> > > passed into it, which can cause C code that passes in a const pointer to
> > > get a pointer back that is not const and then scribble all over the data
> > > in it.  To prevent this, container_of_const() will preserve the const
> > > status of the pointer passed into it using the newly available _Generic()
> > > method.
> > 
> > "_const" in the name suggests that the macro would always take a const
> > argument.
> 
> I mean it here to be "this will preserve const" as yes, it can take a
> const.  Or not.
> 
> > Could this be called e.g. container_of_safe() (for type-safe, but
> > full type_safe would be a bit long)?
> 
> const is an attribute of type safety, container_of is also type-safe
> as-is, it's just the const portion here that is the difference between
> the two.

container_of_const_safe() :-)

> Naming is hard :(

True.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer
  2022-12-01 19:30 [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Greg Kroah-Hartman
                   ` (5 preceding siblings ...)
  2022-12-02  0:48 ` Jason Gunthorpe
@ 2022-12-02 12:48 ` Sakari Ailus
  2022-12-02 16:24   ` Greg Kroah-Hartman
  6 siblings, 1 reply; 14+ messages in thread
From: Sakari Ailus @ 2022-12-02 12:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Jason Gunthorpe, Matthew Wilcox, Andy Shevchenko,
	Rafael J. Wysocki

Hi Greg,

On Thu, Dec 01, 2022 at 08:30:54PM +0100, Greg Kroah-Hartman wrote:
> container_of does not preserve the const-ness of a pointer that is
> passed into it, which can cause C code that passes in a const pointer to
> get a pointer back that is not const and then scribble all over the data
> in it.  To prevent this, container_of_const() will preserve the const
> status of the pointer passed into it using the newly available _Generic()
> method.
> 
> Co-developed-by: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  include/linux/container_of.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> index 2008e9f4058c..3c290e865151 100644
> --- a/include/linux/container_of.h
> +++ b/include/linux/container_of.h
> @@ -22,4 +22,18 @@
>  		      "pointer type mismatch in container_of()");	\
>  	((type *)(__mptr - offsetof(type, member))); })
>  
> +/**
> + * container_of_const - cast a member of a structure out to the containing
> + *			structure and preserve the const-ness of the pointer
> + * @ptr_type:		the type of the pointer @ptr
> + * @ptr:		the pointer to the member
> + * @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, ptr, member_type, member)		\

I missed earlier you had four arguments for the macro instead of three.

With default: this can be done with just three:

#define container_of_const(ptr, member_type, member)		\
	_Generic(ptr,							\
		 const typeof(*(ptr)) *: ((const member_type *)container_of(ptr, member_type, member)),\
		 default: ((member_type *)container_of(ptr, member_type, member))	\
	)

The const typeof(*(ptr)) * will match if ptr is const, otherwise default
matches. But you can't have typeof(*(ptr)) * instead of default as the two
types are still the same, hence default.

I've tested this on GCC 10.2.1 and clang 11.0.1.

This should also make it a bit easier for existing users to switch to the
new macro and hopefully eventual rename back to container_of() once all
users have been converted.

> +	_Generic(ptr,							\
> +		const ptr_type *: ((const member_type *)container_of(ptr, member_type, member)),\
> +		ptr_type *: ((member_type *)container_of(ptr, member_type, member))	\
> +	)
> +
>  #endif	/* _LINUX_CONTAINER_OF_H */

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer
  2022-12-02 12:48 ` Sakari Ailus
@ 2022-12-02 16:24   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2022-12-02 16:24 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-kernel, Jason Gunthorpe, Matthew Wilcox, Andy Shevchenko,
	Rafael J. Wysocki

On Fri, Dec 02, 2022 at 12:48:57PM +0000, Sakari Ailus wrote:
> Hi Greg,
> 
> On Thu, Dec 01, 2022 at 08:30:54PM +0100, Greg Kroah-Hartman wrote:
> > container_of does not preserve the const-ness of a pointer that is
> > passed into it, which can cause C code that passes in a const pointer to
> > get a pointer back that is not const and then scribble all over the data
> > in it.  To prevent this, container_of_const() will preserve the const
> > status of the pointer passed into it using the newly available _Generic()
> > method.
> > 
> > Co-developed-by: Jason Gunthorpe <jgg@ziepe.ca>
> > Cc: Matthew Wilcox <willy@infradead.org>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  include/linux/container_of.h | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> > index 2008e9f4058c..3c290e865151 100644
> > --- a/include/linux/container_of.h
> > +++ b/include/linux/container_of.h
> > @@ -22,4 +22,18 @@
> >  		      "pointer type mismatch in container_of()");	\
> >  	((type *)(__mptr - offsetof(type, member))); })
> >  
> > +/**
> > + * container_of_const - cast a member of a structure out to the containing
> > + *			structure and preserve the const-ness of the pointer
> > + * @ptr_type:		the type of the pointer @ptr
> > + * @ptr:		the pointer to the member
> > + * @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, ptr, member_type, member)		\
> 
> I missed earlier you had four arguments for the macro instead of three.
> 
> With default: this can be done with just three:
> 
> #define container_of_const(ptr, member_type, member)		\
> 	_Generic(ptr,							\
> 		 const typeof(*(ptr)) *: ((const member_type *)container_of(ptr, member_type, member)),\
> 		 default: ((member_type *)container_of(ptr, member_type, member))	\
> 	)
> 
> The const typeof(*(ptr)) * will match if ptr is const, otherwise default
> matches. 

I had tried to use typeof before, your way is easier, thanks, I couldn't
get it to work myself...

> But you can't have typeof(*(ptr)) * instead of default as the two
> types are still the same, hence default.

Ah.  Ok, I really didn't want to use default, and that's probably why it
wasn't working for me.

> I've tested this on GCC 10.2.1 and clang 11.0.1.
> 
> This should also make it a bit easier for existing users to switch to the
> new macro and hopefully eventual rename back to container_of() once all
> users have been converted.

True.  Ok, I'll try this later tomorrow and send out a new version if
all goes well, thanks!

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-12-02 16:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01 19:30 [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Greg Kroah-Hartman
2022-12-01 19:30 ` [PATCH 2/4] device.h: move kobj_to_dev() to use container_of_const() Greg Kroah-Hartman
2022-12-01 19:30 ` [PATCH 3/4] usb.h: take advantage of container_of_const() Greg Kroah-Hartman
2022-12-01 19:30 ` [PATCH 4/4] firmware_loader: fix up to_fw_sysfs() to preserve const Greg Kroah-Hartman
2022-12-01 20:25   ` Russ Weight
2022-12-01 22:50 ` [PATCH 1/4] container_of: add container_of_const() that preserves const-ness of the pointer Andy Shevchenko
2022-12-02  0:46   ` Jason Gunthorpe
2022-12-02  6:41     ` Greg Kroah-Hartman
2022-12-01 23:21 ` Sakari Ailus
2022-12-02 10:45   ` Greg Kroah-Hartman
2022-12-02 12:01     ` Andy Shevchenko
2022-12-02  0:48 ` Jason Gunthorpe
2022-12-02 12:48 ` Sakari Ailus
2022-12-02 16:24   ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.