* [PATCH v3] staging: greybus: audio: Check null pointer
@ 2022-01-04 15:06 Jiasheng Jiang
2022-01-04 20:46 ` Alex Elder
0 siblings, 1 reply; 2+ messages in thread
From: Jiasheng Jiang @ 2022-01-04 15:06 UTC (permalink / raw)
To: elder, vaibhav.sr, mgreer, johan, elder, gregkh
Cc: greybus-dev, linux-staging, linux-kernel, Jiasheng Jiang
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>
---
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);
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] staging: greybus: audio: Check null pointer
2022-01-04 15:06 [PATCH v3] staging: greybus: audio: Check null pointer Jiasheng Jiang
@ 2022-01-04 20:46 ` Alex Elder
0 siblings, 0 replies; 2+ messages in thread
From: Alex Elder @ 2022-01-04 20:46 UTC (permalink / raw)
To: Jiasheng Jiang, vaibhav.sr, mgreer, johan, elder, gregkh
Cc: greybus-dev, linux-staging, linux-kernel
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);
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-01-04 20:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-04 15:06 [PATCH v3] staging: greybus: audio: Check null pointer Jiasheng Jiang
2022-01-04 20:46 ` Alex Elder
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.