[PATCH v2] iio: frequency: admv1013: fix NULL pointer dereference on str

Antoniu Miclaus posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/iio/frequency/admv1013.c | 41 ++++++++++++++++----------------
1 file changed, 20 insertions(+), 21 deletions(-)
[PATCH v2] iio: frequency: admv1013: fix NULL pointer dereference on str
Posted by Antoniu Miclaus 1 month, 1 week ago
When device_property_read_string() fails, str is left uninitialized
but the code falls through to strcmp(str, ...), dereferencing a garbage
pointer. Replace manual read/strcmp with
device_property_match_property_string() which reads the property as a
single string value and matches it against an array of known valid
strings, handling the missing property case internally.

Fixes: da35a7b526d9 ("iio: frequency: admv1013: add support for ADMV1013")
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
Changes in v2:
 - use device_property_match_property_string() instead of
   device_property_match_string() (Andy)
 - add Reviewed-by tag from Nuno

 drivers/iio/frequency/admv1013.c | 41 ++++++++++++++++----------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/frequency/admv1013.c b/drivers/iio/frequency/admv1013.c
index 9202443ef445..3eb0dc2f8dad 100644
--- a/drivers/iio/frequency/admv1013.c
+++ b/drivers/iio/frequency/admv1013.c
@@ -512,37 +512,36 @@ static void admv1013_powerdown(void *data)
 	admv1013_spi_update_bits(data, ADMV1013_REG_ENABLE, enable_reg_msk, enable_reg);
 }
 
+static const char * const admv1013_input_modes[] = { "iq", "if" };
+
+static const char * const admv1013_quad_se_modes[] = { "diff", "se-pos", "se-neg" };
+
 static int admv1013_properties_parse(struct admv1013_state *st)
 {
 	int ret;
-	const char *str;
 	struct device *dev = &st->spi->dev;
 
 	st->det_en = device_property_read_bool(dev, "adi,detector-enable");
 
-	ret = device_property_read_string(dev, "adi,input-mode", &str);
-	if (ret)
-		st->input_mode = ADMV1013_IQ_MODE;
-
-	if (!strcmp(str, "iq"))
-		st->input_mode = ADMV1013_IQ_MODE;
-	else if (!strcmp(str, "if"))
-		st->input_mode = ADMV1013_IF_MODE;
-	else
-		return -EINVAL;
-
-	ret = device_property_read_string(dev, "adi,quad-se-mode", &str);
-	if (ret)
-		st->quad_se_mode = ADMV1013_SE_MODE_DIFF;
+	ret = device_property_match_property_string(dev, "adi,input-mode",
+						    admv1013_input_modes,
+						    ARRAY_SIZE(admv1013_input_modes));
+	st->input_mode = ret >= 0 ? ret : ADMV1013_IQ_MODE;
 
-	if (!strcmp(str, "diff"))
-		st->quad_se_mode = ADMV1013_SE_MODE_DIFF;
-	else if (!strcmp(str, "se-pos"))
+	ret = device_property_match_property_string(dev, "adi,quad-se-mode",
+						    admv1013_quad_se_modes,
+						    ARRAY_SIZE(admv1013_quad_se_modes));
+	switch (ret) {
+	case 1:
 		st->quad_se_mode = ADMV1013_SE_MODE_POS;
-	else if (!strcmp(str, "se-neg"))
+		break;
+	case 2:
 		st->quad_se_mode = ADMV1013_SE_MODE_NEG;
-	else
-		return -EINVAL;
+		break;
+	default:
+		st->quad_se_mode = ADMV1013_SE_MODE_DIFF;
+		break;
+	}
 
 	ret = devm_regulator_bulk_get_enable(dev,
 					     ARRAY_SIZE(admv1013_vcc_regs),
-- 
2.43.0
Re: [PATCH v2] iio: frequency: admv1013: fix NULL pointer dereference on str
Posted by Andy Shevchenko 1 month, 1 week ago
On Tue, Mar 03, 2026 at 11:52:28AM +0200, Antoniu Miclaus wrote:
> When device_property_read_string() fails, str is left uninitialized
> but the code falls through to strcmp(str, ...), dereferencing a garbage
> pointer. Replace manual read/strcmp with
> device_property_match_property_string() which reads the property as a
> single string value and matches it against an array of known valid
> strings, handling the missing property case internally.

...

> +static const char * const admv1013_input_modes[] = { "iq", "if" };

This array has to be indexed.

static const char * const admv1013_input_modes[] = {
	[ADMV1013_IQ_MODE] = "iq",
        [ADMV1013_IF_MODE] = "if",
};

...

> +static const char * const admv1013_quad_se_modes[] = { "diff", "se-pos", "se-neg" };

Also this one.
Taking into account the indices are not sequential, this may require another
enumerator.

Ideally you need to list all possible modes and choose only supported by
assigning an empty string to unsupported ones.

I haven't checked datasheet to understand why only 6, 9, 12 are in use.
Maybe they can be simply 1, 2, 3 with a formula like 3 + x * 3 ? Dunno.

...

> +	switch (ret) {
> +	case 1:
>  		st->quad_se_mode = ADMV1013_SE_MODE_POS;
> -	else if (!strcmp(str, "se-neg"))
> +		break;
> +	case 2:
>  		st->quad_se_mode = ADMV1013_SE_MODE_NEG;
> +		break;

This (the default) should take

	case 0:

as well

> +	default:
> +		st->quad_se_mode = ADMV1013_SE_MODE_DIFF;
> +		break;
> +	}

-- 
With Best Regards,
Andy Shevchenko