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

Miclaus, Antoniu
2026-03-03T12:02:02Z

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
> Sent: Tuesday, March 3, 2026 1:34 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Jonathan Cameron <jic23@kernel.org>;
> David Lechner <dlechner@baylibre.com>; Sa, Nuno <Nuno.Sa@analog.com>;
> Andy Shevchenko <andy@kernel.org>; linux-iio@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2] iio: frequency: admv1013: fix NULL pointer
> dereference on str
>
> [External]
>
> 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.
>

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

static const char * const admv1013_quad_se_modes[] = {
[ADMV1013_QUAD_SE_DIFF] = "diff",
[ADMV1013_QUAD_SE_POS] = "se-pos",
[ADMV1013_QUAD_SE_NEG] = "se-neg",
};

static const unsigned int admv1013_quad_se_regvals[] = {
[ADMV1013_QUAD_SE_DIFF] = ADMV1013_SE_MODE_DIFF,
[ADMV1013_QUAD_SE_POS] = ADMV1013_SE_MODE_POS,
[ADMV1013_QUAD_SE_NEG] = ADMV1013_SE_MODE_NEG,
};

Does this make sense?

> ...
>
> > + 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
>