2022-10-01 08:10:04

by Matti Vaittinen

[permalink] [raw]
Subject: [RFT PATCH v2 0/2] iio: Fix unsafe buffer attributes

IIO The iio_triggered_buffer_setup_ext() has been changed to expect that
all attributes given in buffer_attrs array are device-attributes. This
expectation has not been forced by the API and not all existing users
were checked. Some drivers do register attributes created by
IIO_CONST_ATTR().

The added attribute "wrapping" does not copy the pointer to stored
string constant and when the sysfs file is read the kernel will access
to invalid address.

This series aims to address both the drivers using IIO_CONST_ATTR() and
the API. Use of IIO_CONST_ATTR() has been dropped from drivers for the
triggered buffers and the iio_triggered_buffer_setup_ext() is changed to
take an array of pointers to struct iio_dev_attr instead of an array of
pointers to struct attribute. This should fix the existing users and
also prevent similar problem to occur in the future.

I am not super happy about the new macro IIO_STATIC_CONST_DEVICE_ATTR()
which unconditionally creates a static function and a static struct
iio_dev_attr. OTOH, I do believe static function + static struct
iio_dev_attr should be the right thing to do for majority of use cases.

I did also change the struct iio_buffer to have array of pointers to
iio_dev_attr in order to avoid yet another copying in side the
iio_triggered_buffer_setup_ext(). This change appeared to be somewhat
intrusive - and as I lack the hardware to do thorough testing I added
the request for testing tag here. Especially testing of adi-axi-adc
would be highly appreciated as it is using the
industrialio-buffer-dmaengine.

Changelog v2:
- fix also the cros_ec_sensors_core.c
- fix also the industrialio-buffer-dmaengine.c
- add RFT + this cover-letter.

--

Matti Vaittinen (2):
iio: Add IIO_STATIC_CONST_DEVICE_ATTR
iio: Fix unsafe buffer attributes

drivers/iio/accel/adxl367.c | 16 ++++++++--------
drivers/iio/accel/adxl372.c | 16 ++++++++--------
drivers/iio/accel/bmc150-accel-core.c | 18 +++++++++---------
drivers/iio/adc/at91-sama5d2_adc.c | 16 ++++++++--------
.../iio/buffer/industrialio-buffer-dmaengine.c | 4 ++--
.../iio/buffer/industrialio-triggered-buffer.c | 4 ++--
drivers/iio/buffer/kfifo_buf.c | 2 +-
.../cros_ec_sensors/cros_ec_sensors_core.c | 6 +++---
.../common/hid-sensors/hid-sensor-trigger.c | 8 ++++----
drivers/iio/industrialio-buffer.c | 11 +++++++----
include/linux/iio/buffer_impl.h | 2 +-
include/linux/iio/kfifo_buf.h | 3 ++-
include/linux/iio/sysfs.h | 11 +++++++++++
include/linux/iio/triggered_buffer.h | 6 +++---
14 files changed, 69 insertions(+), 54 deletions(-)


base-commit: f76349cf41451c5c42a99f18a9163377e4b364ff
--
2.37.3


--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]


Attachments:
(No filename) (3.13 kB)
signature.asc (499.00 B)
Download all attachments

2022-10-01 08:26:52

by Matti Vaittinen

[permalink] [raw]
Subject: [RFT PATCH v2 1/2] iio: Add IIO_STATIC_CONST_DEVICE_ATTR

Add IIO_STATIC_CONST_DEVICE_ATTR macro for creating an read-only
iio_dev_attr which returns constant value. This macro is intended to be
used when replacing the IIO_CONST_ATTR - attributes for triggered
buffers because the triggered buffer attributes must be of type
iio_dev_attr.

Signed-off-by: Matti Vaittinen <[email protected]>
---
include/linux/iio/sysfs.h | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/include/linux/iio/sysfs.h b/include/linux/iio/sysfs.h
index e51fba66de4b..de5bb125815c 100644
--- a/include/linux/iio/sysfs.h
+++ b/include/linux/iio/sysfs.h
@@ -97,6 +97,17 @@ struct iio_const_attr {
= { .string = _string, \
.dev_attr = __ATTR(_name, S_IRUGO, iio_read_const_attr, NULL)}

+#define IIO_STATIC_CONST_DEVICE_ATTR(_name, _string) \
+ static ssize_t iio_const_dev_attr_show_##_name( \
+ struct device *dev, \
+ struct device_attribute *attr, \
+ char *buf) \
+ { \
+ return sysfs_emit(buf, "%s\n", _string); \
+ } \
+ static IIO_DEVICE_ATTR(_name, 0444, \
+ iio_const_dev_attr_show_##_name, NULL, 0)
+
/* Generic attributes of onetype or another */

/**
--
2.37.3


--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]


Attachments:
(No filename) (1.52 kB)
signature.asc (499.00 B)
Download all attachments