linux-kernel - Re: [PATCH v2 09/10] sysfs: bin_attribute: add const read/write callback variants

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5b589ddb-e3c9-40e1-987f-30ba81dc8ace@t-8ch.de>
Date: Tue, 3 Dec 2024 17:11:10 +0100
From: Thomas Weißschuh <linux@...ssschuh.net>
To: James Bottomley <James.Bottomley@...senpartnership.com>
Cc: Xinhui.Pan@....com, airlied@...il.com, ajd@...ux.ibm.com, 
	alexander.deucher@....com, alison.schofield@...el.com, amd-gfx@...ts.freedesktop.org, 
	arnd@...db.de, bhelgaas@...gle.com, carlos.bilbao.osdev@...il.com, 
	christian.koenig@....com, dan.j.williams@...el.com, dave.jiang@...el.com, 
	dave@...olabs.net, david.e.box@...ux.intel.com, decui@...rosoft.com, 
	dennis.dalessandro@...nelisnetworks.com, dri-devel@...ts.freedesktop.org, fbarrat@...ux.ibm.com, 
	gregkh@...uxfoundation.org, haiyangz@...rosoft.com, hdegoede@...hat.com, 
	ilpo.jarvinen@...ux.intel.com, ira.weiny@...el.com, jgg@...pe.ca, jonathan.cameron@...wei.com, 
	kys@...rosoft.com, leon@...nel.org, linux-alpha@...r.kernel.org, 
	linux-cxl@...r.kernel.org, linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-mtd@...ts.infradead.org, linux-pci@...r.kernel.org, linux-rdma@...r.kernel.org, 
	linux-scsi@...r.kernel.org, linux-usb@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org, 
	logang@...tatee.com, martin.petersen@...cle.com, mattst88@...il.com, 
	miquel.raynal@...tlin.com, mwalle@...nel.org, naveenkrishna.chatradhi@....com, 
	platform-driver-x86@...r.kernel.org, pratyush@...nel.org, rafael@...nel.org, 
	richard.henderson@...aro.org, richard@....at, simona@...ll.ch, srinivas.kandagatla@...aro.org, 
	tudor.ambarus@...aro.org, vigneshr@...com, vishal.l.verma@...el.com, wei.liu@...nel.org
Subject: Re: [PATCH v2 09/10] sysfs: bin_attribute: add const read/write
 callback variants

On 2024-12-03 11:06:16-0500, James Bottomley wrote:
> > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> > index
> > d17c473c1ef292875475bf3bdf62d07241c13882..d713a6445a6267145a7014f308d
> > f3bb25b8c3287 100644
> > --- a/include/linux/sysfs.h
> > +++ b/include/linux/sysfs.h
> > @@ -305,8 +305,12 @@ struct bin_attribute {
> >  	struct address_space *(*f_mapping)(void);
> >  	ssize_t (*read)(struct file *, struct kobject *, struct
> > bin_attribute *,
> >  			char *, loff_t, size_t);
> > +	ssize_t (*read_new)(struct file *, struct kobject *, const
> > struct bin_attribute *,
> > +			    char *, loff_t, size_t);
> >  	ssize_t (*write)(struct file *, struct kobject *, struct
> > bin_attribute *,
> >  			 char *, loff_t, size_t);
> > +	ssize_t (*write_new)(struct file *, struct kobject *,
> > +			     const struct bin_attribute *, char *,
> > loff_t, size_t);
> >  	loff_t (*llseek)(struct file *, struct kobject *, const
> > struct bin_attribute *,
> >  			 loff_t, int);
> >  	int (*mmap)(struct file *, struct kobject *, const struct
> > bin_attribute *attr,
> > @@ -325,11 +329,28 @@ struct bin_attribute {
> >   */
> >  #define sysfs_bin_attr_init(bin_attr) sysfs_attr_init(&(bin_attr)-
> > >attr)
> >  
> > +typedef ssize_t __sysfs_bin_rw_handler_new(struct file *, struct
> > kobject *,
> > +					   const struct
> > bin_attribute *, char *, loff_t, size_t);
> > +
> >  /* macros to create static binary attributes easier */
> >  #define __BIN_ATTR(_name, _mode, _read, _write, _size)
> > {		\
> >  	.attr = { .name = __stringify(_name), .mode = _mode
> > },		\
> > -	.read	=
> > _read,						\
> > -	.write	=
> > _write,						\
> > +	.read =
> > _Generic(_read,						\
> > +		__sysfs_bin_rw_handler_new * :
> > NULL,			\
> > +		default :
> > _read						\
> > +	),							
> > 	\
> > +	.read_new =
> > _Generic(_read,					\
> > +		__sysfs_bin_rw_handler_new * :
> > _read,			\
> > +		default :
> > NULL						\
> > +	),							
> > 	\
> > +	.write =
> > _Generic(_write,					\
> > +		__sysfs_bin_rw_handler_new * :
> > NULL,			\
> > +		default :
> > _write					\
> > +	),							
> > 	\
> > +	.write_new =
> > _Generic(_write,					\
> > +		__sysfs_bin_rw_handler_new * :
> > _write,			\
> > +		default :
> > NULL						\
> > +	),							
> > 	\
> >  	.size	=
> > _size,						\
> >  }
> 
> It's probably a bit late now, but you've done this the wrong way
> around.  What you should have done is added the const to .read/.write
> then added a .read_old/.write_old with the original function prototype
> and used _Generic() to switch between them.  Then when there are no
> more non const left, you can simply remove .read_old and .write_old
> without getting Linus annoyed by having to do something like this:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e70140ba0d2b1a30467d4af6bcfe761327b9ec95

Not all users are using the macros to define their attributes.
(Nor do they want to)

These users would break with your suggestion.
Otherwise I agree.


Thomas