[PATCH 0/2] iio: proximity: sx9310/sx9360: reject unsupported hardware gain values

[PATCH 0/2] iio: proximity: sx9310/sx9360: reject unsupported hardware gain values

[Arnav Kapoor]

sx9310_write_gain() and sx9360_write_gain() accept any integer and
squeeze ilog2() of it into the register field. Values other than the
advertised 1, 2, 4 and 8 are silently rounded, wrapped or (for 0 and
negative values) turned into the maximum code, and the write reports
success. sx9324 already rejects such values; this does the same for the
other two. One patch per driver, as the Fixes tags differ.

Compile-tested only (W=1); I don't have SX9310 or SX9360 hardware.

Arnav Kapoor (2):
  iio: proximity: sx9310: reject unsupported hardware gain values
  iio: proximity: sx9360: reject unsupported hardware gain values

 drivers/iio/proximity/sx9310.c | 3 +++
 drivers/iio/proximity/sx9360.c | 3 +++
 2 files changed, 6 insertions(+)


base-commit: 340d44bdf9db0f095f99845b5ad8628781d79b6f
-- 
2.53.0

[PATCH 1/2] iio: proximity: sx9310: reject unsupported hardware gain values

[Arnav Kapoor]

sx9310_write_gain() passes any value from userspace to ilog2() and then
FIELD_PREP() into the 2-bit gain field without checking it. Only gains
of 1, 2, 4 and 8 are supported (and listed in hardwaregain_available),
but other values are not rejected; they are silently changed:

  - 3, 5, 6, 7 are rounded down to the next power of two;
  - 16 and larger wrap around in the 2-bit field, e.g. 16 sets a gain
    of 1 and 32 a gain of 2;
  - 0 and negative values set the maximum gain of 8, because ilog2(0)
    is -1 and a negative int becomes a large unsigned value.

The write returns success in every case, so userspace cannot tell that
the gain it asked for was not applied.

Return -EINVAL for anything other than 1, 2, 4 or 8.

Fixes: 227c83faa2f8 ("iio: sx9310: Support hardware gain factor")
Signed-off-by: Arnav Kapoor <kapoorarnav43@gmail.com>
---
 drivers/iio/proximity/sx9310.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index 2585b9c6fb69..27b5d604a409 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -665,6 +665,9 @@ static int sx9310_write_gain(struct sx_common_data *data,
 {
 	unsigned int gain, mask;
 
+	if (val <= 0 || val > 8 || !is_power_of_2(val))
+		return -EINVAL;
+
 	gain = ilog2(val);
 
 	switch (chan->channel) {
-- 
2.53.0

Re: [PATCH 1/2] iio: proximity: sx9310: reject unsupported hardware gain values

[Jonathan Cameron]

On Sat,  7 Nov 2026 16:47:03 +0530
Arnav Kapoor <kapoorarnav43@gmail.com> wrote:

> sx9310_write_gain() passes any value from userspace to ilog2() and then
> FIELD_PREP() into the 2-bit gain field without checking it. Only gains
> of 1, 2, 4 and 8 are supported (and listed in hardwaregain_available),
> but other values are not rejected; they are silently changed:
> 
>   - 3, 5, 6, 7 are rounded down to the next power of two;

There is nothing in the ABI rules that says a driver should not round these
values.  So whilst I don't mind a specific driver not doing that I'm not
sure we should modify the ABI of a driver to start rejecting them after
it has been out there for a while.  The only key thing is that value
read back after a write reflects the true scaling.

>   - 16 and larger wrap around in the 2-bit field, e.g. 16 sets a gain
>     of 1 and 32 a gain of 2;
>   - 0 and negative values set the maximum gain of 8, because ilog2(0)
>     is -1 and a negative int becomes a large unsigned value.

These two improvements are good and unlike rounding behaviour setting (including
reasonable rounding down to 32 - so anything that would not otherwise be 64) an
out of range scale or 0 / negative are not things correct userspace should
be doing.


> 
> The write returns success in every case, so userspace cannot tell that
> the gain it asked for was not applied.
> 
> Return -EINVAL for anything other than 1, 2, 4 or 8.
> 
> Fixes: 227c83faa2f8 ("iio: sx9310: Support hardware gain factor")
> Signed-off-by: Arnav Kapoor <kapoorarnav43@gmail.com>
> ---
>  drivers/iio/proximity/sx9310.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
> index 2585b9c6fb69..27b5d604a409 100644
> --- a/drivers/iio/proximity/sx9310.c
> +++ b/drivers/iio/proximity/sx9310.c
> @@ -665,6 +665,9 @@ static int sx9310_write_gain(struct sx_common_data *data,
>  {
>  	unsigned int gain, mask;
>  
> +	if (val <= 0 || val > 8 || !is_power_of_2(val))
Upshot of above is check for <= 0 here.
> +		return -EINVAL;
> +
>  	gain = ilog2(val);
and check this fits in the field.  FIELD_FIT() will link that
explicitly to where it is being written. That only works here
because all the values that fit are valid (I think!)

Jonathan

>  
>  	switch (chan->channel) {

[PATCH 2/2] iio: proximity: sx9360: reject unsupported hardware gain values

[Arnav Kapoor]

sx9360_write_gain() passes any value from userspace to ilog2() and then
FIELD_PREP() into the 3-bit PROX_CTRL0 gain field without checking it.
Only gains of 1, 2, 4 and 8 are supported (and listed in
hardwaregain_available), but other values are not rejected:

  - 3, 5, 6, 7 are rounded down to the next power of two;
  - 16 to 128 write gain codes 4 to 7, which the driver does not
    advertise, and the next read reports gains of 16 to 128;
  - 256 and larger wrap around in the field, e.g. 256 sets a gain of 1;
  - 0 and negative values write code 7 (read back as 128), because
    ilog2(0) is -1 and a negative int becomes a large unsigned value.

The write returns success in every case.

Return -EINVAL for anything other than 1, 2, 4 or 8.

Fixes: 1cdb4c47f7f5 ("iio:proximity:sx9360: Add sx9360 support")
Signed-off-by: Arnav Kapoor <kapoorarnav43@gmail.com>
---
 drivers/iio/proximity/sx9360.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
index 2a9b37df8f62..c6e5b3e557bb 100644
--- a/drivers/iio/proximity/sx9360.c
+++ b/drivers/iio/proximity/sx9360.c
@@ -625,6 +625,9 @@ static int sx9360_write_gain(struct sx_common_data *data,
 {
 	unsigned int gain, reg;
 
+	if (val <= 0 || val > 8 || !is_power_of_2(val))
+		return -EINVAL;
+
 	gain = ilog2(val);
 	reg = SX9360_REG_PROX_CTRL0_PHR + chan->channel;
 	gain = FIELD_PREP(SX9360_REG_PROX_CTRL0_GAIN_MASK, gain);
-- 
2.53.0

Re: [PATCH 2/2] iio: proximity: sx9360: reject unsupported hardware gain values

[Jonathan Cameron]

On Sat,  7 Nov 2026 16:47:04 +0530
Arnav Kapoor <kapoorarnav43@gmail.com> wrote:

> sx9360_write_gain() passes any value from userspace to ilog2() and then
> FIELD_PREP() into the 3-bit PROX_CTRL0 gain field without checking it.
> Only gains of 1, 2, 4 and 8 are supported (and listed in
> hardwaregain_available), but other values are not rejected:
> 
>   - 3, 5, 6, 7 are rounded down to the next power of two;
>   - 16 to 128 write gain codes 4 to 7, which the driver does not
>     advertise, and the next read reports gains of 16 to 128;
>   - 256 and larger wrap around in the field, e.g. 256 sets a gain of 1;
>   - 0 and negative values write code 7 (read back as 128), because
>     ilog2(0) is -1 and a negative int becomes a large unsigned value.
> 
> The write returns success in every case.
> 
> Return -EINVAL for anything other than 1, 2, 4 or 8.
> 
> Fixes: 1cdb4c47f7f5 ("iio:proximity:sx9360: Add sx9360 support")
> Signed-off-by: Arnav Kapoor <kapoorarnav43@gmail.com>
Same comments as for the previous.  Fine to reject stuff that will
get unexpected results. The rounding down is more reasonable so
don't reject that.

J
> ---
>  drivers/iio/proximity/sx9360.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
> index 2a9b37df8f62..c6e5b3e557bb 100644
> --- a/drivers/iio/proximity/sx9360.c
> +++ b/drivers/iio/proximity/sx9360.c
> @@ -625,6 +625,9 @@ static int sx9360_write_gain(struct sx_common_data *data,
>  {
>  	unsigned int gain, reg;
>  
> +	if (val <= 0 || val > 8 || !is_power_of_2(val))
> +		return -EINVAL;
> +
>  	gain = ilog2(val);
>  	reg = SX9360_REG_PROX_CTRL0_PHR + chan->channel;
>  	gain = FIELD_PREP(SX9360_REG_PROX_CTRL0_GAIN_MASK, gain);