Messages in this thread | | | Date | Mon, 21 Mar 2022 12:31:58 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH 1/6] property: add fwnode_property_read_string_index() |
| |
On Mon, Mar 21, 2022 at 08:49:21AM +0100, Clément Léger wrote: > Le Fri, 18 Mar 2022 20:09:37 +0200, > Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : > > On Fri, Mar 18, 2022 at 05:49:12PM +0100, Clément Léger wrote: > > > Le Fri, 18 Mar 2022 18:26:00 +0200, > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit : > > > > On Fri, Mar 18, 2022 at 05:00:47PM +0100, Clément Léger wrote:
...
> > > > > + values = kcalloc(nval, sizeof(*values), GFP_KERNEL); > > > > > + if (!values) > > > > > + return -ENOMEM; > > > > > + > > > > > + ret = fwnode_property_read_string_array(fwnode, propname, values, nval); > > > > > + if (ret < 0) > > > > > + goto out; > > > > > + > > > > > + *string = values[index]; > > > > > +out: > > > > > + kfree(values); > > > > > > > > Here is UAF (use after free). How is it supposed to work? > > > > > > values is an array of pointers. I'm only retrieving a pointer out of > > > it. > > > > I see, thanks for pointing out. > > > > Nevertheless, I don't like the idea of allocating memory in this case. > > Can we rather add a new callback that will provide us the necessary > > property directly? > > > > IMHO, it would indeed be better. However, > fwnode_property_match_string() also allocates memory to do the same > kind of operation. Would you also like a callback for this one ?
But matching string will need all of them to cover all possible cases. So, it doesn't rely on the certain index and needs allocation anyway.
-- With Best Regards, Andy Shevchenko
| |