[PATCH 0/7] iio: fix possible race condition during access of available info lists

[PATCH 0/7] iio: fix possible race condition during access of available info lists

[Matteo Martelli]

Some iio drivers currently share an available info list buffer that
might be changed while iio core prints it to sysfs. This could cause the
buffer shared with iio core to be corrupted. However, note that I was
able to trigger the race condition only by adding a delay between each
sysfs_emit_at calls in the iio_format_list() to force the concurrent
access to the shared available list buffer.

This patch set extends the iio APIs and fixes some affected drivers.

Summary:
- Patch 1: iio core: introduce a iio info release callback to let
  drivers share a copy of their available info list and later free it.

- Patch 2: pac1921: handle the current scale available info via the
  read_avail+read_avail_release_resource APIs instead of using an ad-hoc
  ext_info attribute. The latter was used to avoid the risk of a race in
  the available list.

- Patch 3,4: ad7192, as73211: fix the possible race in the drivers by
  copying/releasing the affected available lists.

- Patch 5: inkern: make consumers copy and release the available info
  lists of their producers, necessary after patch 1.

- Patch 6,7: iio-mux, iio-rescale, dpot-dac, ingenic-battery: adapt
  consumers to inkern API change by freeing the now copied available
  lists of their producers.

Tested:
- pac1921: could not reproduce the race condition with the new APIs,
  even with additional delays among the sysfs_emit_at calls during a
  shunt resistor write. No new issue found after the change.

- iio-mux, iio-rescale, dpot-dac: tested with pac1921 as producer, which
  was adapted to produce a mock raw available info list.
  The tests did not cover the driver features but focused on assessing
  the function call sequence. For example the following traced function
  graph shows a read of the dpot mocked out voltage (with ftrace
  filters: pac1921* iio* dpot* kmemdup_array* kfree*):

 3)               |  iio_read_channel_info_avail [industrialio]() {
 3)               |    dpot_dac_read_avail [dpot_dac]() {
 3)               |      iio_read_avail_channel_raw [industrialio]() {
 3)               |        iio_channel_read_avail [industrialio]() {
 3)               |          pac1921_read_avail [pac1921]() {
 3)   5.208 us    |            kmemdup_array();
 3) + 11.459 us   |          }
 3)   3.167 us    |          kmemdup_array();
 3)               |          pac1921_read_avail_release_res [pac1921]() {
 3)   1.709 us    |            kfree();
 3)   4.458 us    |          }
 3) + 25.750 us   |        }
 3) + 31.792 us   |      }
 3) + 35.000 us   |    }
 3) + 37.083 us   |    iio_format_list [industrialio]();
 3)               |    dpot_dac_read_avail_release_res [dpot_dac]() {
 3)   1.583 us    |      kfree();
 3)   4.250 us    |    }
 3) + 84.292 us   |  }

- ingenic-battery: also tested with mock available info produced by the
  pac1921 driver. Following the traced graph part that should correspond
  to the ingenic_battery_set_scale() flow (which is not traceable with
  the additional ingenic* ftrace filter for some reason):

 2)               |  ingenic_battery_probe [ingenic_battery]() {
                ...
 2)               |    iio_read_max_channel_raw [industrialio]() {
 2)               |      iio_channel_read_avail [industrialio]() {
 2)               |        pac1921_read_avail [pac1921]() {
 2)   4.333 us    |          kmemdup_array();
 2) + 10.834 us   |        }
 2)   3.500 us    |        kmemdup_array();
 2)               |        pac1921_read_avail_release_res [pac1921]() {
 2)   1.791 us    |          kfree();
 2)   4.625 us    |        }
 2) + 26.291 us   |      }
 2)   1.583 us    |      kfree();
 2) + 35.750 us   |    }
 2)               |    iio_read_avail_channel_attribute [industrialio]() {
 2)               |      iio_channel_read_avail [industrialio]() {
 2)               |        pac1921_read_avail [pac1921]() {
 2)   3.250 us    |          kmemdup_array();
 2)   8.209 us    |        }
 2)   3.458 us    |        kmemdup_array();
 2)               |        pac1921_read_avail_release_res [pac1921]() {
 2)   1.542 us    |          kfree();
 2)   4.292 us    |        }
 2) + 21.417 us   |      }
 2) + 26.333 us   |    }
 2)               |    iio_write_channel_attribute [industrialio]() {
 2)   4.375 us    |      pac1921_write_raw [pac1921]();
 2)   9.625 us    |    }
 2)   1.666 us    |    kfree();
 2) * 47810.08 us |  }

Not tested:
- ad7192, as73211

Link: https://lore.kernel.org/linux-iio/20240724-iio-pac1921-v4-0-723698e903a3@gmail.com/

Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
---
Matteo Martelli (7):
      iio: core: add read_avail_release_resource callback to fix race
      iio: pac1921: use read_avail+release APIs instead of custom ext_info
      iio: ad7192: copy/release available filter frequencies to fix race
      iio: as73211: copy/release available integration times to fix race
      iio: inkern: copy/release available info from producer
      iio: consumers: release available info buffer copied from producer
      power: supply: ingenic-battery: free scale buffer after use

 drivers/iio/adc/ad7192.c               |  22 +++++-
 drivers/iio/adc/pac1921.c              | 128 ++++++++++++---------------------
 drivers/iio/afe/iio-rescale.c          |   8 +++
 drivers/iio/dac/dpot-dac.c             |   8 +++
 drivers/iio/industrialio-core.c        |  14 +++-
 drivers/iio/inkern.c                   |  64 ++++++++++++-----
 drivers/iio/light/as73211.c            |  23 +++++-
 drivers/iio/multiplexer/iio-mux.c      |   8 +++
 drivers/power/supply/ingenic-battery.c |  16 +++--
 include/linux/iio/consumer.h           |   4 +-
 include/linux/iio/iio.h                |   4 ++
 11 files changed, 185 insertions(+), 114 deletions(-)
---
base-commit: fec496684388685647652ab4213454fbabdab099
change-id: 20240802-iio-read-avail-release-cb3d2a1e1b98

Best regards,
-- 
Matteo Martelli <matteomartelli3@gmail.com>

[PATCH 1/7] iio: core: add read_avail_release_resource callback to fix race

[Matteo Martelli]

Some iio drivers currently share an available info buffer that might be
changed while iio core prints it to sysfs. To avoid the buffer
corruption, add a release callback to let iio drivers allocate a copy of
the available info buffer and later free it in the release callback.
Such control is kept in the driver logic so that some driver that needs
a big available info buffer might also perform some check to keep the
copied buffer around in case no race has occurred.

Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
---
 drivers/iio/industrialio-core.c | 14 +++++++++++---
 include/linux/iio/iio.h         |  4 ++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 6a6568d4a2cb3a3f63381d5a6f25a2881b3ba2ed..4aea9de9f15a4d70f9d02fb3d47df49eef8c8423 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -859,12 +859,20 @@ static ssize_t iio_read_channel_info_avail(struct device *dev,
 		return ret;
 	switch (ret) {
 	case IIO_AVAIL_LIST:
-		return iio_format_avail_list(buf, vals, type, length);
+		ret = iio_format_avail_list(buf, vals, type, length);
+		break;
 	case IIO_AVAIL_RANGE:
-		return iio_format_avail_range(buf, vals, type);
+		ret = iio_format_avail_range(buf, vals, type);
+		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
 	}
+
+	if (indio_dev->info->read_avail_release_resource)
+		indio_dev->info->read_avail_release_resource(
+			indio_dev, this_attr->c, vals, this_attr->address);
+
+	return ret;
 }
 
 /**
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 18779b631e9071801958fc9d50b941b548fab940..55326f0310b2e9b3fa561c4728e7eabe1d7a5e78 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -491,6 +491,10 @@ struct iio_info {
 			  int *length,
 			  long mask);
 
+	void (*read_avail_release_resource)(struct iio_dev *indio_dev,
+					    struct iio_chan_spec const *chan,
+					    const int *vals, long mask);
+
 	int (*write_raw)(struct iio_dev *indio_dev,
 			 struct iio_chan_spec const *chan,
 			 int val,

-- 
2.46.2

[PATCH 2/7] iio: pac1921: use read_avail+release APIs instead of custom ext_info

[Matteo Martelli]

The pac1921 driver was exposing the available scale info via an ad-hoc
ext_info attribute instead of using the read_avail API. This to avoid a
possible race condition: while the available current scales were being
printed to sysfs by iio core (iio_read_channel_info_avail), the shunt
resistor might have been changed concurrently.

Switch to the read_avail+release APIs now that the race condition has
been addressed.

Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
---
 drivers/iio/adc/pac1921.c | 128 ++++++++++++++++------------------------------
 1 file changed, 45 insertions(+), 83 deletions(-)

diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c
index 4c2a1c07bc399028f0334885fc9cd4552d5892b1..f491a4923380ef9b406a68f8cc413df873869e3e 100644
--- a/drivers/iio/adc/pac1921.c
+++ b/drivers/iio/adc/pac1921.c
@@ -445,11 +445,52 @@ static int pac1921_read_avail(struct iio_dev *indio_dev,
 		*vals = pac1921_int_num_samples;
 		*length = ARRAY_SIZE(pac1921_int_num_samples);
 		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->channel) {
+		case PAC1921_CHAN_VBUS:
+			*vals = (const int *)pac1921_vbus_scales;
+			*length = ARRAY_SIZE(pac1921_vbus_scales) * 2;
+			*type = IIO_VAL_INT_PLUS_NANO;
+			return IIO_AVAIL_LIST;
+
+		case PAC1921_CHAN_VSENSE:
+			*vals = (const int *)pac1921_vsense_scales;
+			*length = ARRAY_SIZE(pac1921_vsense_scales) * 2;
+			*type = IIO_VAL_INT_PLUS_NANO;
+			return IIO_AVAIL_LIST;
+
+		case PAC1921_CHAN_CURRENT: {
+			struct pac1921_priv *priv = iio_priv(indio_dev);
+
+			*length = ARRAY_SIZE(priv->current_scales) * 2;
+			*type = IIO_VAL_INT_PLUS_NANO;
+
+			guard(mutex)(&priv->lock);
+
+			*vals = kmemdup_array((int *)priv->current_scales,
+					      *length, sizeof(int), GFP_KERNEL);
+			if (!*vals)
+				return -ENOMEM;
+
+			return IIO_AVAIL_LIST;
+		}
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
 }
 
+static void pac1921_read_avail_release_res(struct iio_dev *indio_dev,
+					   struct iio_chan_spec const *chan,
+					   const int *vals, long mask)
+{
+	if (mask == IIO_CHAN_INFO_SCALE &&
+	    chan->channel == PAC1921_CHAN_CURRENT)
+		kfree(vals);
+}
+
 /*
  * Perform configuration update sequence: set the device into read state, then
  * write the config register and set the device back into integration state.
@@ -749,6 +790,7 @@ static int pac1921_read_event_value(struct iio_dev *indio_dev,
 static const struct iio_info pac1921_iio = {
 	.read_raw = pac1921_read_raw,
 	.read_avail = pac1921_read_avail,
+	.read_avail_release_resource = pac1921_read_avail_release_res,
 	.write_raw = pac1921_write_raw,
 	.write_raw_get_fmt = pac1921_write_raw_get_fmt,
 	.read_label = pac1921_read_label,
@@ -806,88 +848,7 @@ static ssize_t pac1921_write_shunt_resistor(struct iio_dev *indio_dev,
 	return len;
 }
 
-/*
- * Emit on sysfs the list of available scales contained in scales_tbl
- *
- * TODO:: this function can be replaced with iio_format_avail_list() if the
- * latter will ever be exported.
- *
- * Must be called with lock held if the scales_tbl can change runtime (e.g. for
- * the current scales table)
- */
-static ssize_t pac1921_format_scale_avail(const int (*const scales_tbl)[2],
-					  size_t size, char *buf)
-{
-	ssize_t len = 0;
-
-	for (unsigned int i = 0; i < size; i++) {
-		if (i != 0) {
-			len += sysfs_emit_at(buf, len, " ");
-			if (len >= PAGE_SIZE)
-				return -EFBIG;
-		}
-		len += sysfs_emit_at(buf, len, "%d.%09d", scales_tbl[i][0],
-				     scales_tbl[i][1]);
-		if (len >= PAGE_SIZE)
-			return -EFBIG;
-	}
-
-	len += sysfs_emit_at(buf, len, "\n");
-	return len;
-}
-
-/*
- * Read available scales for a specific channel
- *
- * NOTE: using extended info insted of iio.read_avail() because access to
- * current scales must be locked as they depend on shunt resistor which may
- * change runtime. Caller of iio.read_avail() would access the table unlocked
- * instead.
- */
-static ssize_t pac1921_read_scale_avail(struct iio_dev *indio_dev,
-					uintptr_t private,
-					const struct iio_chan_spec *chan,
-					char *buf)
-{
-	struct pac1921_priv *priv = iio_priv(indio_dev);
-	const int (*scales_tbl)[2];
-	size_t size;
-
-	switch (chan->channel) {
-	case PAC1921_CHAN_VBUS:
-		scales_tbl = pac1921_vbus_scales;
-		size = ARRAY_SIZE(pac1921_vbus_scales);
-		return pac1921_format_scale_avail(scales_tbl, size, buf);
-
-	case PAC1921_CHAN_VSENSE:
-		scales_tbl = pac1921_vsense_scales;
-		size = ARRAY_SIZE(pac1921_vsense_scales);
-		return pac1921_format_scale_avail(scales_tbl, size, buf);
-
-	case PAC1921_CHAN_CURRENT: {
-		guard(mutex)(&priv->lock);
-		scales_tbl = priv->current_scales;
-		size = ARRAY_SIZE(priv->current_scales);
-		return pac1921_format_scale_avail(scales_tbl, size, buf);
-	}
-	default:
-		return -EINVAL;
-	}
-}
-
-#define PAC1921_EXT_INFO_SCALE_AVAIL {					\
-	.name = "scale_available",					\
-	.read = pac1921_read_scale_avail,				\
-	.shared = IIO_SEPARATE,						\
-}
-
-static const struct iio_chan_spec_ext_info pac1921_ext_info_voltage[] = {
-	PAC1921_EXT_INFO_SCALE_AVAIL,
-	{}
-};
-
 static const struct iio_chan_spec_ext_info pac1921_ext_info_current[] = {
-	PAC1921_EXT_INFO_SCALE_AVAIL,
 	{
 		.name = "shunt_resistor",
 		.read = pac1921_read_shunt_resistor,
@@ -911,6 +872,7 @@ static const struct iio_chan_spec pac1921_channels[] = {
 		.type = IIO_VOLTAGE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
 				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_all =
 			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
 			BIT(IIO_CHAN_INFO_SAMP_FREQ),
@@ -928,12 +890,12 @@ static const struct iio_chan_spec pac1921_channels[] = {
 		.indexed = 1,
 		.event_spec = pac1921_overflow_event,
 		.num_event_specs = ARRAY_SIZE(pac1921_overflow_event),
-		.ext_info = pac1921_ext_info_voltage,
 	},
 	{
 		.type = IIO_VOLTAGE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
 				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_all =
 			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
 			BIT(IIO_CHAN_INFO_SAMP_FREQ),
@@ -951,12 +913,12 @@ static const struct iio_chan_spec pac1921_channels[] = {
 		.indexed = 1,
 		.event_spec = pac1921_overflow_event,
 		.num_event_specs = ARRAY_SIZE(pac1921_overflow_event),
-		.ext_info = pac1921_ext_info_voltage,
 	},
 	{
 		.type = IIO_CURRENT,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
 				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_all =
 			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
 			BIT(IIO_CHAN_INFO_SAMP_FREQ),

-- 
2.46.2

[PATCH 3/7] iio: ad7192: copy/release available filter frequencies to fix race

[Matteo Martelli]

While available filter frequencies are being printed to sysfs by iio
core (iio_read_channel_info_avail), the sampling frequency might be
changed. This could cause the buffer shared with iio core to be
corrupted. To prevent it, make a copy of the filter frequencies buffer
and free it in the read_avail_release_resource callback.

Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
---
 drivers/iio/adc/ad7192.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 7042ddfdfc03ee5ea58ca07fb1943feb6538175b..acf625ced0b21db8d44f77929e8a875b3c10e1b1 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -1056,12 +1056,19 @@ static int ad7192_read_avail(struct iio_dev *indio_dev,
 		*length = ARRAY_SIZE(st->scale_avail) * 2;
 
 		return IIO_AVAIL_LIST;
-	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
-		*vals = (int *)st->filter_freq_avail;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: {
 		*type = IIO_VAL_FRACTIONAL;
 		*length = ARRAY_SIZE(st->filter_freq_avail) * 2;
 
+		guard(mutex)(&st->lock);
+
+		*vals = kmemdup_array((int *)st->filter_freq_avail, *length,
+				      sizeof(int), GFP_KERNEL);
+		if (!*vals)
+			return -ENOMEM;
+
 		return IIO_AVAIL_LIST;
+	}
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		*vals = (int *)st->oversampling_ratio_avail;
 		*type = IIO_VAL_INT;
@@ -1073,6 +1080,14 @@ static int ad7192_read_avail(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static void ad7192_read_avail_release_res(struct iio_dev *indio_dev,
+					  struct iio_chan_spec const *chan,
+					  const int *vals, long mask)
+{
+	if (mask == IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY)
+		kfree(vals);
+}
+
 static int ad7192_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask)
 {
 	struct ad7192_state *st = iio_priv(indio_dev);
@@ -1098,6 +1113,7 @@ static const struct iio_info ad7192_info = {
 	.write_raw = ad7192_write_raw,
 	.write_raw_get_fmt = ad7192_write_raw_get_fmt,
 	.read_avail = ad7192_read_avail,
+	.read_avail_release_resource = ad7192_read_avail_release_res,
 	.attrs = &ad7192_attribute_group,
 	.validate_trigger = ad_sd_validate_trigger,
 	.update_scan_mode = ad7192_update_scan_mode,
@@ -1108,6 +1124,7 @@ static const struct iio_info ad7194_info = {
 	.write_raw = ad7192_write_raw,
 	.write_raw_get_fmt = ad7192_write_raw_get_fmt,
 	.read_avail = ad7192_read_avail,
+	.read_avail_release_resource = ad7192_read_avail_release_res,
 	.validate_trigger = ad_sd_validate_trigger,
 };
 
@@ -1116,6 +1133,7 @@ static const struct iio_info ad7195_info = {
 	.write_raw = ad7192_write_raw,
 	.write_raw_get_fmt = ad7192_write_raw_get_fmt,
 	.read_avail = ad7192_read_avail,
+	.read_avail_release_resource = ad7192_read_avail_release_res,
 	.attrs = &ad7195_attribute_group,
 	.validate_trigger = ad_sd_validate_trigger,
 	.update_scan_mode = ad7192_update_scan_mode,

-- 
2.46.2

[PATCH 4/7] iio: as73211: copy/release available integration times to fix race

[Matteo Martelli]

While available integration times are being printed to sysfs by iio core
(iio_read_channel_info_avail), the sampling frequency might be changed.
This could cause the buffer shared with iio core to be corrupted. To
prevent it, make a copy of the integration times buffer and free it in
the read_avail_release_resource callback.

Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
---
 drivers/iio/light/as73211.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
index be0068081ebbbb37fdfb252b67a77b302ff725f6..520c898e0ff9c530b4fdd45589559f9014d7992c 100644
--- a/drivers/iio/light/as73211.c
+++ b/drivers/iio/light/as73211.c
@@ -493,17 +493,33 @@ static int as73211_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec co
 		*type = IIO_VAL_INT;
 		return IIO_AVAIL_LIST;
 
-	case IIO_CHAN_INFO_INT_TIME:
+	case IIO_CHAN_INFO_INT_TIME: {
 		*length = ARRAY_SIZE(data->int_time_avail);
-		*vals = data->int_time_avail;
 		*type = IIO_VAL_INT_PLUS_MICRO;
-		return IIO_AVAIL_LIST;
 
+		guard(mutex)(&data->mutex);
+
+		*vals = kmemdup_array(data->int_time_avail, *length,
+				      sizeof(int), GFP_KERNEL);
+		if (!*vals)
+			return -ENOMEM;
+
+		return IIO_AVAIL_LIST;
+	}
 	default:
 		return -EINVAL;
 	}
 }
 
+static void as73211_read_avail_release_res(struct iio_dev *indio_dev,
+					   struct iio_chan_spec const *chan,
+					   const int *vals, long mask)
+{
+	if (mask == IIO_CHAN_INFO_INT_TIME)
+		kfree(vals);
+}
+
+
 static int _as73211_write_raw(struct iio_dev *indio_dev,
 			       struct iio_chan_spec const *chan __always_unused,
 			       int val, int val2, long mask)
@@ -699,6 +715,7 @@ static irqreturn_t as73211_trigger_handler(int irq __always_unused, void *p)
 static const struct iio_info as73211_info = {
 	.read_raw = as73211_read_raw,
 	.read_avail = as73211_read_avail,
+	.read_avail_release_resource = as73211_read_avail_release_res,
 	.write_raw = as73211_write_raw,
 };
 

-- 
2.46.2

Re: [PATCH 4/7] iio: as73211: copy/release available integration times to fix race

[Jonathan Cameron]

On Thu, 03 Oct 2024 19:34:09 +0200
Matteo Martelli <matteomartelli3@gmail.com> wrote:

> While available integration times are being printed to sysfs by iio core
> (iio_read_channel_info_avail), the sampling frequency might be changed.
> This could cause the buffer shared with iio core to be corrupted. To
> prevent it, make a copy of the integration times buffer and free it in
> the read_avail_release_resource callback.
> 
> Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
> ---
>  drivers/iio/light/as73211.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
> index be0068081ebbbb37fdfb252b67a77b302ff725f6..520c898e0ff9c530b4fdd45589559f9014d7992c 100644
> --- a/drivers/iio/light/as73211.c
> +++ b/drivers/iio/light/as73211.c
> @@ -493,17 +493,33 @@ static int as73211_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec co
>  		*type = IIO_VAL_INT;
>  		return IIO_AVAIL_LIST;
>  
> -	case IIO_CHAN_INFO_INT_TIME:
> +	case IIO_CHAN_INFO_INT_TIME: {
>  		*length = ARRAY_SIZE(data->int_time_avail);
> -		*vals = data->int_time_avail;
>  		*type = IIO_VAL_INT_PLUS_MICRO;
> -		return IIO_AVAIL_LIST;
>  
> +		guard(mutex)(&data->mutex);
> +
> +		*vals = kmemdup_array(data->int_time_avail, *length,
> +				      sizeof(int), GFP_KERNEL);
> +		if (!*vals)
> +			return -ENOMEM;
> +
> +		return IIO_AVAIL_LIST;
> +	}
>  	default:
>  		return -EINVAL;
>  	}
>  }
>  
> +static void as73211_read_avail_release_res(struct iio_dev *indio_dev,
> +					   struct iio_chan_spec const *chan,
> +					   const int *vals, long mask)
> +{
> +	if (mask == IIO_CHAN_INFO_INT_TIME)
> +		kfree(vals);
> +}
> +
Trivial but one blank line is enough.  If there is no other feedback on the
series I can tidy this up whilst applying.

> +
>  static int _as73211_write_raw(struct iio_dev *indio_dev,
>  			       struct iio_chan_spec const *chan __always_unused,
>  			       int val, int val2, long mask)
> @@ -699,6 +715,7 @@ static irqreturn_t as73211_trigger_handler(int irq __always_unused, void *p)
>  static const struct iio_info as73211_info = {
>  	.read_raw = as73211_read_raw,
>  	.read_avail = as73211_read_avail,
> +	.read_avail_release_resource = as73211_read_avail_release_res,
>  	.write_raw = as73211_write_raw,
>  };
>  
> 

[PATCH 5/7] iio: inkern: copy/release available info from producer

[Matteo Martelli]

Consumers need to call the read_avail_release_resource after reading the
available info. To call the release with info_exists locked, copy the
available info from the producer and immediately call its release
callback. With this change, users of iio_read_avail_channel_raw() and
iio_read_avail_channel_attribute() must free the copied avail info after
calling them.

Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
---
 drivers/iio/inkern.c         | 64 +++++++++++++++++++++++++++++++++-----------
 include/linux/iio/consumer.h |  4 +--
 2 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 7f325b3ed08fae6674245312cf8f57bb151006c0..cc65ef79451e5aa2cea447e168007a447ffc0d91 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -760,9 +760,25 @@ static int iio_channel_read_avail(struct iio_channel *chan,
 	if (!iio_channel_has_available(chan->channel, info))
 		return -EINVAL;
 
-	if (iio_info->read_avail)
-		return iio_info->read_avail(chan->indio_dev, chan->channel,
-					    vals, type, length, info);
+	if (iio_info->read_avail) {
+		const int *vals_tmp;
+		int ret;
+
+		ret = iio_info->read_avail(chan->indio_dev, chan->channel,
+					   &vals_tmp, type, length, info);
+		if (ret < 0)
+			return ret;
+
+		*vals = kmemdup_array(vals_tmp, *length, sizeof(int), GFP_KERNEL);
+		if (!*vals)
+			return -ENOMEM;
+
+		if (iio_info->read_avail_release_resource)
+			iio_info->read_avail_release_resource(
+				chan->indio_dev, chan->channel, vals_tmp, info);
+
+		return ret;
+	}
 	return -EINVAL;
 }
 
@@ -789,9 +805,11 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
 	ret = iio_read_avail_channel_attribute(chan, vals, &type, length,
 					       IIO_CHAN_INFO_RAW);
 
-	if (ret >= 0 && type != IIO_VAL_INT)
+	if (ret >= 0 && type != IIO_VAL_INT) {
 		/* raw values are assumed to be IIO_VAL_INT */
+		kfree(*vals);
 		ret = -EINVAL;
+	}
 
 	return ret;
 }
@@ -820,24 +838,31 @@ static int iio_channel_read_max(struct iio_channel *chan,
 			if (val2)
 				*val2 = vals[5];
 		}
-		return 0;
+		ret = 0;
+		break;
 
 	case IIO_AVAIL_LIST:
-		if (length <= 0)
-			return -EINVAL;
+		if (length <= 0) {
+			ret = -EINVAL;
+			goto out;
+		}
 		switch (*type) {
 		case IIO_VAL_INT:
 			*val = max_array(vals, length);
+			ret = 0;
 			break;
 		default:
 			/* TODO: learn about max for other iio values */
-			return -EINVAL;
+			ret = -EINVAL;
 		}
-		return 0;
+		break;
 
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
 	}
+out:
+	kfree(vals);
+	return ret;
 }
 
 int iio_read_max_channel_raw(struct iio_channel *chan, int *val)
@@ -876,24 +901,31 @@ static int iio_channel_read_min(struct iio_channel *chan,
 			if (val2)
 				*val2 = vals[1];
 		}
-		return 0;
+		ret = 0;
+		break;
 
 	case IIO_AVAIL_LIST:
-		if (length <= 0)
-			return -EINVAL;
+		if (length <= 0) {
+			ret = -EINVAL;
+			goto out;
+		}
 		switch (*type) {
 		case IIO_VAL_INT:
 			*val = min_array(vals, length);
+			ret = 0;
 			break;
 		default:
 			/* TODO: learn about min for other iio values */
-			return -EINVAL;
+			ret = -EINVAL;
 		}
-		return 0;
+		break;
 
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
 	}
+out:
+	kfree(vals);
+	return ret;
 }
 
 int iio_read_min_channel_raw(struct iio_channel *chan, int *val)
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 333d1d8ccb37f387fe531577ac5e0bfc7f752cec..e3e268d2574b3e01c9412449d90d627de7efcd84 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -316,7 +316,7 @@ int iio_read_min_channel_raw(struct iio_channel *chan, int *val);
 /**
  * iio_read_avail_channel_raw() - read available raw values from a given channel
  * @chan:		The channel being queried.
- * @vals:		Available values read back.
+ * @vals:		Available values read back. Must be freed after use.
  * @length:		Number of entries in vals.
  *
  * Returns an error code, IIO_AVAIL_RANGE or IIO_AVAIL_LIST.
@@ -334,7 +334,7 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
 /**
  * iio_read_avail_channel_attribute() - read available channel attribute values
  * @chan:		The channel being queried.
- * @vals:		Available values read back.
+ * @vals:		Available values read back. Must be freed after use.
  * @type:		Type of values read back.
  * @length:		Number of entries in vals.
  * @attribute:		info attribute to be read back.

-- 
2.46.2

[PATCH 6/7] iio: consumers: release available info buffer copied from producer

[Matteo Martelli]

The iio_read_avail_channel_raw() inkern interface now allocates a copy
of the available info buffer that must be freed after use. To do so,
free the buffer in the consumers read_avail_relese_resource callback.

Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
---
 drivers/iio/afe/iio-rescale.c     | 8 ++++++++
 drivers/iio/dac/dpot-dac.c        | 8 ++++++++
 drivers/iio/multiplexer/iio-mux.c | 8 ++++++++
 3 files changed, 24 insertions(+)

diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
index 56e5913ab82d1c045c9ca27012008a4495502cbf..78bb86c291706748b4072a484532ad20c415ff9f 100644
--- a/drivers/iio/afe/iio-rescale.c
+++ b/drivers/iio/afe/iio-rescale.c
@@ -249,9 +249,17 @@ static int rescale_read_avail(struct iio_dev *indio_dev,
 	}
 }
 
+static void rescale_read_avail_release_res(struct iio_dev *indio_dev,
+					   struct iio_chan_spec const *chan,
+					   const int *vals, long mask)
+{
+	kfree(vals);
+}
+
 static const struct iio_info rescale_info = {
 	.read_raw = rescale_read_raw,
 	.read_avail = rescale_read_avail,
+	.read_avail_release_resource = rescale_read_avail_release_res,
 };
 
 static ssize_t rescale_read_ext_info(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/dpot-dac.c b/drivers/iio/dac/dpot-dac.c
index 7332064d0852d979620f90066fb027f6f68f8c95..beec76247acb5170b81058d28a71ed17c831c905 100644
--- a/drivers/iio/dac/dpot-dac.c
+++ b/drivers/iio/dac/dpot-dac.c
@@ -108,6 +108,13 @@ static int dpot_dac_read_avail(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static void dpot_dac_read_avail_release_res(struct iio_dev *indio_dev,
+					    struct iio_chan_spec const *chan,
+					    const int *vals, long mask)
+{
+	kfree(vals);
+}
+
 static int dpot_dac_write_raw(struct iio_dev *indio_dev,
 			      struct iio_chan_spec const *chan,
 			      int val, int val2, long mask)
@@ -125,6 +132,7 @@ static int dpot_dac_write_raw(struct iio_dev *indio_dev,
 static const struct iio_info dpot_dac_info = {
 	.read_raw = dpot_dac_read_raw,
 	.read_avail = dpot_dac_read_avail,
+	.read_avail_release_resource = dpot_dac_read_avail_release_res,
 	.write_raw = dpot_dac_write_raw,
 };
 
diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
index 2953403bef53bbe47a97a8ab1c475ed88d7f86d2..31345437784b01c5d6f8ea70263f4c2574388e7a 100644
--- a/drivers/iio/multiplexer/iio-mux.c
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -142,6 +142,13 @@ static int mux_read_avail(struct iio_dev *indio_dev,
 	return ret;
 }
 
+static void mux_read_avail_release_res(struct iio_dev *indio_dev,
+				       struct iio_chan_spec const *chan,
+				       const int *vals, long mask)
+{
+	kfree(vals);
+}
+
 static int mux_write_raw(struct iio_dev *indio_dev,
 			 struct iio_chan_spec const *chan,
 			 int val, int val2, long mask)
@@ -171,6 +178,7 @@ static int mux_write_raw(struct iio_dev *indio_dev,
 static const struct iio_info mux_info = {
 	.read_raw = mux_read_raw,
 	.read_avail = mux_read_avail,
+	.read_avail_release_resource = mux_read_avail_release_res,
 	.write_raw = mux_write_raw,
 };
 

-- 
2.46.2

Re: [PATCH 6/7] iio: consumers: release available info buffer copied from producer

[Jonathan Cameron]

On Thu, 03 Oct 2024 19:34:11 +0200
Matteo Martelli <matteomartelli3@gmail.com> wrote:

> The iio_read_avail_channel_raw() inkern interface now allocates a copy
> of the available info buffer that must be freed after use. To do so,
> free the buffer in the consumers read_avail_relese_resource callback.
release

Otherwise fine.

J
> 
> Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
> ---
>  drivers/iio/afe/iio-rescale.c     | 8 ++++++++
>  drivers/iio/dac/dpot-dac.c        | 8 ++++++++
>  drivers/iio/multiplexer/iio-mux.c | 8 ++++++++
>  3 files changed, 24 insertions(+)
> 
> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> index 56e5913ab82d1c045c9ca27012008a4495502cbf..78bb86c291706748b4072a484532ad20c415ff9f 100644
> --- a/drivers/iio/afe/iio-rescale.c
> +++ b/drivers/iio/afe/iio-rescale.c
> @@ -249,9 +249,17 @@ static int rescale_read_avail(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static void rescale_read_avail_release_res(struct iio_dev *indio_dev,
> +					   struct iio_chan_spec const *chan,
> +					   const int *vals, long mask)
> +{
> +	kfree(vals);
> +}
> +
>  static const struct iio_info rescale_info = {
>  	.read_raw = rescale_read_raw,
>  	.read_avail = rescale_read_avail,
> +	.read_avail_release_resource = rescale_read_avail_release_res,
>  };
>  
>  static ssize_t rescale_read_ext_info(struct iio_dev *indio_dev,
> diff --git a/drivers/iio/dac/dpot-dac.c b/drivers/iio/dac/dpot-dac.c
> index 7332064d0852d979620f90066fb027f6f68f8c95..beec76247acb5170b81058d28a71ed17c831c905 100644
> --- a/drivers/iio/dac/dpot-dac.c
> +++ b/drivers/iio/dac/dpot-dac.c
> @@ -108,6 +108,13 @@ static int dpot_dac_read_avail(struct iio_dev *indio_dev,
>  	return -EINVAL;
>  }
>  
> +static void dpot_dac_read_avail_release_res(struct iio_dev *indio_dev,
> +					    struct iio_chan_spec const *chan,
> +					    const int *vals, long mask)
> +{
> +	kfree(vals);
> +}
> +
>  static int dpot_dac_write_raw(struct iio_dev *indio_dev,
>  			      struct iio_chan_spec const *chan,
>  			      int val, int val2, long mask)
> @@ -125,6 +132,7 @@ static int dpot_dac_write_raw(struct iio_dev *indio_dev,
>  static const struct iio_info dpot_dac_info = {
>  	.read_raw = dpot_dac_read_raw,
>  	.read_avail = dpot_dac_read_avail,
> +	.read_avail_release_resource = dpot_dac_read_avail_release_res,
>  	.write_raw = dpot_dac_write_raw,
>  };
>  
> diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
> index 2953403bef53bbe47a97a8ab1c475ed88d7f86d2..31345437784b01c5d6f8ea70263f4c2574388e7a 100644
> --- a/drivers/iio/multiplexer/iio-mux.c
> +++ b/drivers/iio/multiplexer/iio-mux.c
> @@ -142,6 +142,13 @@ static int mux_read_avail(struct iio_dev *indio_dev,
>  	return ret;
>  }
>  
> +static void mux_read_avail_release_res(struct iio_dev *indio_dev,
> +				       struct iio_chan_spec const *chan,
> +				       const int *vals, long mask)
> +{
> +	kfree(vals);
> +}
> +
>  static int mux_write_raw(struct iio_dev *indio_dev,
>  			 struct iio_chan_spec const *chan,
>  			 int val, int val2, long mask)
> @@ -171,6 +178,7 @@ static int mux_write_raw(struct iio_dev *indio_dev,
>  static const struct iio_info mux_info = {
>  	.read_raw = mux_read_raw,
>  	.read_avail = mux_read_avail,
> +	.read_avail_release_resource = mux_read_avail_release_res,
>  	.write_raw = mux_write_raw,
>  };
>  
> 

[PATCH 7/7] power: supply: ingenic-battery: free scale buffer after use

[Matteo Martelli]

The iio_read_avail_channel_attribute() iio interface now allocates a
copy of the available info buffer that must be freed after use.

Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
---
 drivers/power/supply/ingenic-battery.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/ingenic-battery.c b/drivers/power/supply/ingenic-battery.c
index 0a40f425c27723ccec49985b8b5e14a737b6a7eb..fa6d6898f8722cc8e06a888a762a3edeb0474a6e 100644
--- a/drivers/power/supply/ingenic-battery.c
+++ b/drivers/power/supply/ingenic-battery.c
@@ -79,8 +79,10 @@ static int ingenic_battery_set_scale(struct ingenic_battery *bat)
 		dev_err(bat->dev, "Unable to read channel avail scale\n");
 		return ret;
 	}
-	if (ret != IIO_AVAIL_LIST || scale_type != IIO_VAL_FRACTIONAL_LOG2)
-		return -EINVAL;
+	if (ret != IIO_AVAIL_LIST || scale_type != IIO_VAL_FRACTIONAL_LOG2) {
+		ret = -EINVAL;
+		goto out;
+	}
 
 	max_mV = bat->info->voltage_max_design_uv / 1000;
 
@@ -99,7 +101,8 @@ static int ingenic_battery_set_scale(struct ingenic_battery *bat)
 
 	if (best_idx < 0) {
 		dev_err(bat->dev, "Unable to find matching voltage scale\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	/* Only set scale if there is more than one (fractional) entry */
@@ -109,10 +112,13 @@ static int ingenic_battery_set_scale(struct ingenic_battery *bat)
 						  scale_raw[best_idx + 1],
 						  IIO_CHAN_INFO_SCALE);
 		if (ret)
-			return ret;
+			goto out;
 	}
 
-	return 0;
+	ret = 0;
+out:
+	kfree(scale_raw);
+	return ret;
 }
 
 static enum power_supply_property ingenic_battery_properties[] = {

-- 
2.46.2

Re: [PATCH 7/7] power: supply: ingenic-battery: free scale buffer after use

[kernel test robot]

Hi Matteo,

kernel test robot noticed the following build errors:

[auto build test ERROR on fec496684388685647652ab4213454fbabdab099]

url:    https://github.com/intel-lab-lkp/linux/commits/Matteo-Martelli/iio-core-add-read_avail_release_resource-callback-to-fix-race/20241004-013654
base:   fec496684388685647652ab4213454fbabdab099
patch link:    https://lore.kernel.org/r/20241003-iio-read-avail-release-v1-7-c70cc7d9c2e0%40gmail.com
patch subject: [PATCH 7/7] power: supply: ingenic-battery: free scale buffer after use
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20241005/202410050547.Pybj1FLp-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241005/202410050547.Pybj1FLp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410050547.Pybj1FLp-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/power/supply/ingenic-battery.c: In function 'ingenic_battery_set_scale':
>> drivers/power/supply/ingenic-battery.c:120:9: error: implicit declaration of function 'kfree'; did you mean 'kvfree'? [-Wimplicit-function-declaration]
     120 |         kfree(scale_raw);
         |         ^~~~~
         |         kvfree


vim +120 drivers/power/supply/ingenic-battery.c

    59	
    60	/* Set the most appropriate IIO channel voltage reference scale
    61	 * based on the battery's max voltage.
    62	 */
    63	static int ingenic_battery_set_scale(struct ingenic_battery *bat)
    64	{
    65		const int *scale_raw;
    66		int scale_len, scale_type, best_idx = -1, best_mV, max_raw, i, ret;
    67		u64 max_mV;
    68	
    69		ret = iio_read_max_channel_raw(bat->channel, &max_raw);
    70		if (ret) {
    71			dev_err(bat->dev, "Unable to read max raw channel value\n");
    72			return ret;
    73		}
    74	
    75		ret = iio_read_avail_channel_attribute(bat->channel, &scale_raw,
    76						       &scale_type, &scale_len,
    77						       IIO_CHAN_INFO_SCALE);
    78		if (ret < 0) {
    79			dev_err(bat->dev, "Unable to read channel avail scale\n");
    80			return ret;
    81		}
    82		if (ret != IIO_AVAIL_LIST || scale_type != IIO_VAL_FRACTIONAL_LOG2) {
    83			ret = -EINVAL;
    84			goto out;
    85		}
    86	
    87		max_mV = bat->info->voltage_max_design_uv / 1000;
    88	
    89		for (i = 0; i < scale_len; i += 2) {
    90			u64 scale_mV = (max_raw * scale_raw[i]) >> scale_raw[i + 1];
    91	
    92			if (scale_mV < max_mV)
    93				continue;
    94	
    95			if (best_idx >= 0 && scale_mV > best_mV)
    96				continue;
    97	
    98			best_mV = scale_mV;
    99			best_idx = i;
   100		}
   101	
   102		if (best_idx < 0) {
   103			dev_err(bat->dev, "Unable to find matching voltage scale\n");
   104			ret = -EINVAL;
   105			goto out;
   106		}
   107	
   108		/* Only set scale if there is more than one (fractional) entry */
   109		if (scale_len > 2) {
   110			ret = iio_write_channel_attribute(bat->channel,
   111							  scale_raw[best_idx],
   112							  scale_raw[best_idx + 1],
   113							  IIO_CHAN_INFO_SCALE);
   114			if (ret)
   115				goto out;
   116		}
   117	
   118		ret = 0;
   119	out:
 > 120		kfree(scale_raw);
   121		return ret;
   122	}
   123	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 7/7] power: supply: ingenic-battery: free scale buffer after use

[kernel test robot]

Hi Matteo,

kernel test robot noticed the following build errors:

[auto build test ERROR on fec496684388685647652ab4213454fbabdab099]

url:    https://github.com/intel-lab-lkp/linux/commits/Matteo-Martelli/iio-core-add-read_avail_release_resource-callback-to-fix-race/20241004-013654
base:   fec496684388685647652ab4213454fbabdab099
patch link:    https://lore.kernel.org/r/20241003-iio-read-avail-release-v1-7-c70cc7d9c2e0%40gmail.com
patch subject: [PATCH 7/7] power: supply: ingenic-battery: free scale buffer after use
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20241005/202410050737.0PgqTuD1-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project fef3566a25ff0e34fb87339ba5e13eca17cec00f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241005/202410050737.0PgqTuD1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410050737.0PgqTuD1-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/power/supply/ingenic-battery.c:120:2: error: call to undeclared function 'kfree'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     120 |         kfree(scale_raw);
         |         ^
   1 error generated.


vim +/kfree +120 drivers/power/supply/ingenic-battery.c

    59	
    60	/* Set the most appropriate IIO channel voltage reference scale
    61	 * based on the battery's max voltage.
    62	 */
    63	static int ingenic_battery_set_scale(struct ingenic_battery *bat)
    64	{
    65		const int *scale_raw;
    66		int scale_len, scale_type, best_idx = -1, best_mV, max_raw, i, ret;
    67		u64 max_mV;
    68	
    69		ret = iio_read_max_channel_raw(bat->channel, &max_raw);
    70		if (ret) {
    71			dev_err(bat->dev, "Unable to read max raw channel value\n");
    72			return ret;
    73		}
    74	
    75		ret = iio_read_avail_channel_attribute(bat->channel, &scale_raw,
    76						       &scale_type, &scale_len,
    77						       IIO_CHAN_INFO_SCALE);
    78		if (ret < 0) {
    79			dev_err(bat->dev, "Unable to read channel avail scale\n");
    80			return ret;
    81		}
    82		if (ret != IIO_AVAIL_LIST || scale_type != IIO_VAL_FRACTIONAL_LOG2) {
    83			ret = -EINVAL;
    84			goto out;
    85		}
    86	
    87		max_mV = bat->info->voltage_max_design_uv / 1000;
    88	
    89		for (i = 0; i < scale_len; i += 2) {
    90			u64 scale_mV = (max_raw * scale_raw[i]) >> scale_raw[i + 1];
    91	
    92			if (scale_mV < max_mV)
    93				continue;
    94	
    95			if (best_idx >= 0 && scale_mV > best_mV)
    96				continue;
    97	
    98			best_mV = scale_mV;
    99			best_idx = i;
   100		}
   101	
   102		if (best_idx < 0) {
   103			dev_err(bat->dev, "Unable to find matching voltage scale\n");
   104			ret = -EINVAL;
   105			goto out;
   106		}
   107	
   108		/* Only set scale if there is more than one (fractional) entry */
   109		if (scale_len > 2) {
   110			ret = iio_write_channel_attribute(bat->channel,
   111							  scale_raw[best_idx],
   112							  scale_raw[best_idx + 1],
   113							  IIO_CHAN_INFO_SCALE);
   114			if (ret)
   115				goto out;
   116		}
   117	
   118		ret = 0;
   119	out:
 > 120		kfree(scale_raw);
   121		return ret;
   122	}
   123	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 0/7] iio: fix possible race condition during access of available info lists

[Jonathan Cameron]

On Thu, 03 Oct 2024 19:34:05 +0200
Matteo Martelli <matteomartelli3@gmail.com> wrote:

> Some iio drivers currently share an available info list buffer that
> might be changed while iio core prints it to sysfs. This could cause the
> buffer shared with iio core to be corrupted. However, note that I was
> able to trigger the race condition only by adding a delay between each
> sysfs_emit_at calls in the iio_format_list() to force the concurrent
> access to the shared available list buffer.
> 
> This patch set extends the iio APIs and fixes some affected drivers.
Thanks for tidying these up.  My comments are very minor but
as this is changing how a core bit of infrastructure works I'd like
them to sit on the list another week anyway.

There is just enough here that I'd prefer a v2 though if you don't
get time I can probably tidy it up whilst applying.

The build bot issue is presumably a missing include.

Thanks,

Jonathan

> 
> Summary:
> - Patch 1: iio core: introduce a iio info release callback to let
>   drivers share a copy of their available info list and later free it.
> 
> - Patch 2: pac1921: handle the current scale available info via the
>   read_avail+read_avail_release_resource APIs instead of using an ad-hoc
>   ext_info attribute. The latter was used to avoid the risk of a race in
>   the available list.
> 
> - Patch 3,4: ad7192, as73211: fix the possible race in the drivers by
>   copying/releasing the affected available lists.
> 
> - Patch 5: inkern: make consumers copy and release the available info
>   lists of their producers, necessary after patch 1.
> 
> - Patch 6,7: iio-mux, iio-rescale, dpot-dac, ingenic-battery: adapt
>   consumers to inkern API change by freeing the now copied available
>   lists of their producers.
> 
> Tested:
> - pac1921: could not reproduce the race condition with the new APIs,
>   even with additional delays among the sysfs_emit_at calls during a
>   shunt resistor write. No new issue found after the change.
> 
> - iio-mux, iio-rescale, dpot-dac: tested with pac1921 as producer, which
>   was adapted to produce a mock raw available info list.
>   The tests did not cover the driver features but focused on assessing
>   the function call sequence. For example the following traced function
>   graph shows a read of the dpot mocked out voltage (with ftrace
>   filters: pac1921* iio* dpot* kmemdup_array* kfree*):
> 
>  3)               |  iio_read_channel_info_avail [industrialio]() {
>  3)               |    dpot_dac_read_avail [dpot_dac]() {
>  3)               |      iio_read_avail_channel_raw [industrialio]() {
>  3)               |        iio_channel_read_avail [industrialio]() {
>  3)               |          pac1921_read_avail [pac1921]() {
>  3)   5.208 us    |            kmemdup_array();
>  3) + 11.459 us   |          }
>  3)   3.167 us    |          kmemdup_array();
>  3)               |          pac1921_read_avail_release_res [pac1921]() {
>  3)   1.709 us    |            kfree();
>  3)   4.458 us    |          }
>  3) + 25.750 us   |        }
>  3) + 31.792 us   |      }
>  3) + 35.000 us   |    }
>  3) + 37.083 us   |    iio_format_list [industrialio]();
>  3)               |    dpot_dac_read_avail_release_res [dpot_dac]() {
>  3)   1.583 us    |      kfree();
>  3)   4.250 us    |    }
>  3) + 84.292 us   |  }
> 
> - ingenic-battery: also tested with mock available info produced by the
>   pac1921 driver. Following the traced graph part that should correspond
>   to the ingenic_battery_set_scale() flow (which is not traceable with
>   the additional ingenic* ftrace filter for some reason):
> 
>  2)               |  ingenic_battery_probe [ingenic_battery]() {
>                 ...
>  2)               |    iio_read_max_channel_raw [industrialio]() {
>  2)               |      iio_channel_read_avail [industrialio]() {
>  2)               |        pac1921_read_avail [pac1921]() {
>  2)   4.333 us    |          kmemdup_array();
>  2) + 10.834 us   |        }
>  2)   3.500 us    |        kmemdup_array();
>  2)               |        pac1921_read_avail_release_res [pac1921]() {
>  2)   1.791 us    |          kfree();
>  2)   4.625 us    |        }
>  2) + 26.291 us   |      }
>  2)   1.583 us    |      kfree();
>  2) + 35.750 us   |    }
>  2)               |    iio_read_avail_channel_attribute [industrialio]() {
>  2)               |      iio_channel_read_avail [industrialio]() {
>  2)               |        pac1921_read_avail [pac1921]() {
>  2)   3.250 us    |          kmemdup_array();
>  2)   8.209 us    |        }
>  2)   3.458 us    |        kmemdup_array();
>  2)               |        pac1921_read_avail_release_res [pac1921]() {
>  2)   1.542 us    |          kfree();
>  2)   4.292 us    |        }
>  2) + 21.417 us   |      }
>  2) + 26.333 us   |    }
>  2)               |    iio_write_channel_attribute [industrialio]() {
>  2)   4.375 us    |      pac1921_write_raw [pac1921]();
>  2)   9.625 us    |    }
>  2)   1.666 us    |    kfree();
>  2) * 47810.08 us |  }
> 
> Not tested:
> - ad7192, as73211
> 
> Link: https://lore.kernel.org/linux-iio/20240724-iio-pac1921-v4-0-723698e903a3@gmail.com/
> 
> Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
> ---
> Matteo Martelli (7):
>       iio: core: add read_avail_release_resource callback to fix race
>       iio: pac1921: use read_avail+release APIs instead of custom ext_info
>       iio: ad7192: copy/release available filter frequencies to fix race
>       iio: as73211: copy/release available integration times to fix race
>       iio: inkern: copy/release available info from producer
>       iio: consumers: release available info buffer copied from producer
>       power: supply: ingenic-battery: free scale buffer after use
> 
>  drivers/iio/adc/ad7192.c               |  22 +++++-
>  drivers/iio/adc/pac1921.c              | 128 ++++++++++++---------------------
>  drivers/iio/afe/iio-rescale.c          |   8 +++
>  drivers/iio/dac/dpot-dac.c             |   8 +++
>  drivers/iio/industrialio-core.c        |  14 +++-
>  drivers/iio/inkern.c                   |  64 ++++++++++++-----
>  drivers/iio/light/as73211.c            |  23 +++++-
>  drivers/iio/multiplexer/iio-mux.c      |   8 +++
>  drivers/power/supply/ingenic-battery.c |  16 +++--
>  include/linux/iio/consumer.h           |   4 +-
>  include/linux/iio/iio.h                |   4 ++
>  11 files changed, 185 insertions(+), 114 deletions(-)
> ---
> base-commit: fec496684388685647652ab4213454fbabdab099
> change-id: 20240802-iio-read-avail-release-cb3d2a1e1b98
> 
> Best regards,