| Message ID | 20241103-sysfs-const-bin_attr-v2-0-71110628844c@weissschuh.net |
|---|---|
| Headers | show |
| Series | sysfs: constify struct bin_attribute (Part 1) | expand |
Hello, > struct bin_attribute contains a bunch of pointer members, which when > overwritten by accident or malice can lead to system instability and > security problems. > Moving the definitions of struct bin_attribute to read-only memory > makes these modifications impossible. > The same change has been performed for many other structures in the > past. (struct class, struct ctl_table...) > > For the structure definitions throughout the core to be moved to > read-only memory the following steps are necessary. > > 1) Change all callbacks invoked from the sysfs core to only pass const > pointers > 2) Adapt the sysfs core to only work in terms of const pointers > 3) Adapt the sysfs core APIs to allow const pointers > 4) Change all structure definitions through the core to const > > This series provides the foundation for step 1) above. > It converts some callbacks in a single step to const and provides a > foundation for those callbacks where a single step is not possible. > > Patches 1-5 change the bin_attribute callbacks of 'struct > attribute_group'. The remaining ones touch 'struct bin_attribute' itself. > > The techniques employed by this series can later be reused for the > same change for other sysfs attributes. > > This series is intended to be merged through the driver core tree. This is very nice. Thank you! For PCI changes: Acked-by: Krzysztof Wilczyński <kw@linux.com> This reminded me of an old discussions with Greg and Bjorn about how to set size correctly for our ROM and BAR sysfs objects. Nice to see a very nice approach here, indeed. Krzysztof
On Sun, 2024-11-03 at 17:03 +0000, Thomas Weißschuh wrote: > The mmap() callbacks should not modify the struct > bin_attribute passed as argument. > Enforce this by marking the argument as const. > > As there are not many callback implementers perform this change > throughout the tree at once. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> Acked-by: Andrew Donnellan <ajd@linux.ibm.com> # ocxl
On Sun, Nov 03, 2024 at 05:03:34PM +0000, Thomas Weißschuh wrote: > The is_bin_visible() callbacks should not modify the struct > bin_attribute passed as argument. > Enforce this by marking the argument as const. > > As there are not many callback implementers perform this change > throughout the tree at once. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- > drivers/cxl/port.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- > drivers/infiniband/hw/qib/qib_sysfs.c | 2 +- > drivers/mtd/spi-nor/sysfs.c | 2 +- > drivers/nvmem/core.c | 3 ++- > drivers/pci/pci-sysfs.c | 2 +- > drivers/pci/vpd.c | 2 +- > drivers/platform/x86/amd/hsmp.c | 2 +- > drivers/platform/x86/intel/sdsi.c | 2 +- > drivers/scsi/scsi_sysfs.c | 2 +- > drivers/usb/core/sysfs.c | 2 +- > include/linux/sysfs.h | 30 +++++++++++++++--------------- > 12 files changed, 27 insertions(+), 26 deletions(-) For infiniband: Acked-by: Jason Gunthorpe <jgg@nvidia.com>
struct bin_attribute contains a bunch of pointer members, which when overwritten by accident or malice can lead to system instability and security problems. Moving the definitions of struct bin_attribute to read-only memory makes these modifications impossible. The same change has been performed for many other structures in the past. (struct class, struct ctl_table...) For the structure definitions throughout the core to be moved to read-only memory the following steps are necessary. 1) Change all callbacks invoked from the sysfs core to only pass const pointers 2) Adapt the sysfs core to only work in terms of const pointers 3) Adapt the sysfs core APIs to allow const pointers 4) Change all structure definitions through the core to const This series provides the foundation for step 1) above. It converts some callbacks in a single step to const and provides a foundation for those callbacks where a single step is not possible. Patches 1-5 change the bin_attribute callbacks of 'struct attribute_group'. The remaining ones touch 'struct bin_attribute' itself. The techniques employed by this series can later be reused for the same change for other sysfs attributes. This series is intended to be merged through the driver core tree. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- Changes in v2: - Drop RFC state - Refuse registration of attributes with both read/read_new or write/write_new - Remove don't drop llseek() callback, as it is actually used. Instead also migrate it to "const". - _Generic machinery: Simplify and make more robust against misuse - Link to v1: https://lore.kernel.org/r/20241031-sysfs-const-bin_attr-v1-0-2281afa7f055@weissschuh.net --- Thomas Weißschuh (10): sysfs: explicitly pass size to sysfs_add_bin_file_mode_ns() sysfs: introduce callback attribute_group::bin_size PCI/sysfs: Calculate bin_attribute size through bin_size() nvmem: core: calculate bin_attribute size through bin_size() sysfs: treewide: constify attribute callback of bin_is_visible() sysfs: treewide: constify attribute callback of bin_attribute::mmap() sysfs: treewide: constify attribute callback of bin_attribute::llseek() sysfs: implement all BIN_ATTR_* macros in terms of __BIN_ATTR() sysfs: bin_attribute: add const read/write callback variants driver core: Constify attribute arguments of binary attributes arch/alpha/kernel/pci-sysfs.c | 6 +-- drivers/base/node.c | 4 +- drivers/base/topology.c | 4 +- drivers/cxl/port.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- drivers/infiniband/hw/qib/qib_sysfs.c | 2 +- drivers/misc/ocxl/sysfs.c | 2 +- drivers/mtd/spi-nor/sysfs.c | 2 +- drivers/nvmem/core.c | 16 ++++-- drivers/pci/p2pdma.c | 2 +- drivers/pci/pci-sysfs.c | 42 ++++++++------- drivers/pci/vpd.c | 2 +- drivers/platform/x86/amd/hsmp.c | 2 +- drivers/platform/x86/intel/pmt/class.c | 2 +- drivers/platform/x86/intel/sdsi.c | 2 +- drivers/scsi/scsi_sysfs.c | 2 +- drivers/uio/uio_hv_generic.c | 2 +- drivers/usb/core/sysfs.c | 2 +- fs/sysfs/file.c | 30 +++++++---- fs/sysfs/group.c | 5 +- fs/sysfs/sysfs.h | 2 +- include/linux/sysfs.h | 94 ++++++++++++++++++++------------- 22 files changed, 138 insertions(+), 91 deletions(-) --- base-commit: 3e5e6c9900c3d71895e8bdeacfb579462e98eba1 change-id: 20241028-sysfs-const-bin_attr-a00896481d0b Best regards,