[PATCH] tools:iio: Fix the potential stack overflow risk

[PATCH] tools:iio: Fix the potential stack overflow risk

[jianchunfu]

Add judgment to fix the potential stack overflow risk.

Signed-off-by: jianchunfu <jianchunfu@cmss.chinamobile.com>
---
 tools/iio/iio_generic_buffer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index 2491c54a5..733fc21c2 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -487,6 +487,10 @@ int main(int argc, char **argv)
 			return -ENOMEM;
 		}
 		trigger_name = malloc(IIO_MAX_NAME_LENGTH);
+		if (!trigger_name) {
+			ret = -ENOMEM;
+			goto error;
+		}
 		ret = read_sysfs_string("name", trig_dev_name, trigger_name);
 		free(trig_dev_name);
 		if (ret < 0) {
-- 
2.18.4

Re: [PATCH] tools:iio: Fix the potential stack overflow risk

[Jonathan Cameron]

On Thu, 24 Mar 2022 19:01:57 +0800
jianchunfu <jianchunfu@cmss.chinamobile.com> wrote:

> Add judgment to fix the potential stack overflow risk.
> 
> Signed-off-by: jianchunfu <jianchunfu@cmss.chinamobile.com>
Yikes.

Whilst it doesn't promise to be good code (it's meant to show
the principles, not for anyone to use it) the error handling
in this function is a mess :( 

Whilst your change here looks good I think this function needs
a closer look so we fix them all together.

Just glancing at it, the decision on whether to go to the unified
error handling or return without doing anything seems entirely random.

If you want to take on a more general rework of the error handling
in that function it would be great. If not I 'might' get to it at
somepoint....

Thanks,

Jonathan


> ---
>  tools/iio/iio_generic_buffer.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> index 2491c54a5..733fc21c2 100644
> --- a/tools/iio/iio_generic_buffer.c
> +++ b/tools/iio/iio_generic_buffer.c
> @@ -487,6 +487,10 @@ int main(int argc, char **argv)
>  			return -ENOMEM;
>  		}
>  		trigger_name = malloc(IIO_MAX_NAME_LENGTH);
> +		if (!trigger_name) {
> +			ret = -ENOMEM;
> +			goto error;
> +		}
>  		ret = read_sysfs_string("name", trig_dev_name, trigger_name);
>  		free(trig_dev_name);
>  		if (ret < 0) {

Re: [PATCH] tools:iio: Fix the potential stack overflow risk

[Andy Shevchenko]

On Thu, Mar 24, 2022 at 3:15 PM jianchunfu
<jianchunfu@cmss.chinamobile.com> wrote:
>
> Add judgment to fix the potential stack overflow risk.

>                 trigger_name = malloc(IIO_MAX_NAME_LENGTH);
> +               if (!trigger_name) {
> +                       ret = -ENOMEM;
> +                       goto error;
> +               }

Isn't malloc() sets errno? Why not use it instead?

-- 
With Best Regards,
Andy Shevchenko