Messages in this thread | | | Date | Tue, 4 Jan 2022 14:46:31 -0600 | Subject | Re: [PATCH v3] staging: greybus: audio: Check null pointer | From | Alex Elder <> |
| |
On 1/4/22 9:06 AM, Jiasheng Jiang wrote: > As the possible alloc failure of devm_kcalloc(), it could return null > pointer. > Therefore, 'strings' should be checked and return NULL if alloc fails to > prevent the dereference of the NULL pointer. > Also, the caller should also deal with the return value of the > gb_generate_enum_strings() and return -ENOMEM if returns NULL. > Moreover, because the memory allocated with devm_kzalloc() will be > freed automatically when the last reference to the device is dropped, > the 'gbe' in gbaudio_tplg_create_enum_kctl() and > gbaudio_tplg_create_enum_ctl() do not need to free manually. > But the 'control' in gbaudio_tplg_create_widget() and > gbaudio_tplg_process_kcontrols() has a specially error handle to > cleanup. > So it should be better to cleanup 'control' when fails. > > Fixes: e65579e335da ("greybus: audio: topology: Enable enumerated control support") > Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
This looks good to me. Thanks for fixing things.
Reviewed-by: Alex Elder <elder@linaro.org>
> --- > v3: Same code as v2, but remove the redundant message as requested. > v2: Updated to use common error processing at the end of both > gbaudio_tplg_create_widget() and gbaudio_tplg_process_kcontrols(). > --- > drivers/staging/greybus/audio_topology.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c > index 1fc7727ab7be..6bba735d2e5c 100644 > --- a/drivers/staging/greybus/audio_topology.c > +++ b/drivers/staging/greybus/audio_topology.c > @@ -147,6 +147,9 @@ static const char **gb_generate_enum_strings(struct gbaudio_module_info *gb, > > items = le32_to_cpu(gbenum->items); > strings = devm_kcalloc(gb->dev, items, sizeof(char *), GFP_KERNEL); > + if (!strings) > + return NULL; > + > data = gbenum->names; > > for (i = 0; i < items; i++) { > @@ -655,6 +658,8 @@ static int gbaudio_tplg_create_enum_kctl(struct gbaudio_module_info *gb, > /* since count=1, and reg is dummy */ > gbe->items = le32_to_cpu(gb_enum->items); > gbe->texts = gb_generate_enum_strings(gb, gb_enum); > + if (!gbe->texts) > + return -ENOMEM; > > /* debug enum info */ > dev_dbg(gb->dev, "Max:%d, name_length:%d\n", gbe->items, > @@ -862,6 +867,8 @@ static int gbaudio_tplg_create_enum_ctl(struct gbaudio_module_info *gb, > /* since count=1, and reg is dummy */ > gbe->items = le32_to_cpu(gb_enum->items); > gbe->texts = gb_generate_enum_strings(gb, gb_enum); > + if (!gbe->texts) > + return -ENOMEM; > > /* debug enum info */ > dev_dbg(gb->dev, "Max:%d, name_length:%d\n", gbe->items, > @@ -1034,6 +1041,10 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module, > csize += le16_to_cpu(gbenum->names_length); > control->texts = (const char * const *) > gb_generate_enum_strings(module, gbenum); > + if (!control->texts) { > + ret = -ENOMEM; > + goto error; > + } > control->items = le32_to_cpu(gbenum->items); > } else { > csize = sizeof(struct gb_audio_control); > @@ -1183,6 +1194,10 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module, > csize += le16_to_cpu(gbenum->names_length); > control->texts = (const char * const *) > gb_generate_enum_strings(module, gbenum); > + if (!control->texts) { > + ret = -ENOMEM; > + goto error; > + } > control->items = le32_to_cpu(gbenum->items); > } else { > csize = sizeof(struct gb_audio_control); >
| |