| Message ID | 20241205094446.1491176-1-sakari.ailus@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/1] media: ccs: Clean up parsed CCS static data on parse failure | expand |
Hi Sakari, On Thu, Dec 05, 2024 at 11:44:46AM +0200, Sakari Ailus wrote: > ccs_data_parse() releases the allocated in-memory data structure when the > parser fails, but it does not clean up parsed metadata that is there to > help access the actual data. Do that, in order to return the data > structure in a sane state. > > Reported-by: David Heidelberg <david@ixit.cz> > Fixes: a6b396f410b1 ("media: ccs: Add CCS static data parser library") > Cc: stable@vger.kernel.org > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > since v2: > > - Properly clean up after all error cases. > > drivers/media/i2c/ccs/ccs-data.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/i2c/ccs/ccs-data.c b/drivers/media/i2c/ccs/ccs-data.c > index 9d42137f4799..2591dba51e17 100644 > --- a/drivers/media/i2c/ccs/ccs-data.c > +++ b/drivers/media/i2c/ccs/ccs-data.c > @@ -10,6 +10,7 @@ > #include <linux/limits.h> > #include <linux/mm.h> > #include <linux/slab.h> > +#include <linux/string.h> > > #include "ccs-data-defs.h" > > @@ -948,15 +949,15 @@ int ccs_data_parse(struct ccs_data_container *ccsdata, const void *data, > > rval = __ccs_data_parse(&bin, ccsdata, data, len, dev, verbose); > if (rval) > - return rval; > + goto out_cleanup; > > rval = bin_backing_alloc(&bin); > if (rval) > - return rval; > + goto out_cleanup; > > rval = __ccs_data_parse(&bin, ccsdata, data, len, dev, false); > if (rval) > - goto out_free; > + goto out_cleanup; > > if (verbose && ccsdata->version) > print_ccs_data_version(dev, ccsdata->version); > @@ -965,15 +966,16 @@ int ccs_data_parse(struct ccs_data_container *ccsdata, const void *data, > rval = -EPROTO; > dev_dbg(dev, "parsing mismatch; base %p; now %p; end %p\n", > bin.base, bin.now, bin.end); > - goto out_free; > + goto out_cleanup; > } > > ccsdata->backing = bin.base; > > return 0; > > -out_free: > +out_cleanup: Don't you think some kind of logging or at least a dev_dbg() would be helpful here to let the user know that ccs_data_parse() failed ? > kvfree(bin.base); > + memset(ccsdata, 0, sizeof(*ccsdata)); > > return rval; > } > -- > 2.39.5 > -- Kind Regards Mehdi Djait
diff --git a/drivers/media/i2c/ccs/ccs-data.c b/drivers/media/i2c/ccs/ccs-data.c index 9d42137f4799..2591dba51e17 100644 --- a/drivers/media/i2c/ccs/ccs-data.c +++ b/drivers/media/i2c/ccs/ccs-data.c @@ -10,6 +10,7 @@ #include <linux/limits.h> #include <linux/mm.h> #include <linux/slab.h> +#include <linux/string.h> #include "ccs-data-defs.h" @@ -948,15 +949,15 @@ int ccs_data_parse(struct ccs_data_container *ccsdata, const void *data, rval = __ccs_data_parse(&bin, ccsdata, data, len, dev, verbose); if (rval) - return rval; + goto out_cleanup; rval = bin_backing_alloc(&bin); if (rval) - return rval; + goto out_cleanup; rval = __ccs_data_parse(&bin, ccsdata, data, len, dev, false); if (rval) - goto out_free; + goto out_cleanup; if (verbose && ccsdata->version) print_ccs_data_version(dev, ccsdata->version); @@ -965,15 +966,16 @@ int ccs_data_parse(struct ccs_data_container *ccsdata, const void *data, rval = -EPROTO; dev_dbg(dev, "parsing mismatch; base %p; now %p; end %p\n", bin.base, bin.now, bin.end); - goto out_free; + goto out_cleanup; } ccsdata->backing = bin.base; return 0; -out_free: +out_cleanup: kvfree(bin.base); + memset(ccsdata, 0, sizeof(*ccsdata)); return rval; }
ccs_data_parse() releases the allocated in-memory data structure when the parser fails, but it does not clean up parsed metadata that is there to help access the actual data. Do that, in order to return the data structure in a sane state. Reported-by: David Heidelberg <david@ixit.cz> Fixes: a6b396f410b1 ("media: ccs: Add CCS static data parser library") Cc: stable@vger.kernel.org Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- since v2: - Properly clean up after all error cases. drivers/media/i2c/ccs/ccs-data.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)