| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4CB86431.5000305@bfs.de>
Date: Fri, 15 Oct 2010 16:24:49 +0200
From: walter harms <wharms@....de>
To: Julia Lawall <julia@...u.dk>
CC: kernel-janitors@...r.kernel.org, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/7] drivers/staging: Return -ENOMEM on memory allocation
failure
This looks like a case for kasprintf().
re
wh
Julia Lawall schrieb:
> From: Julia Lawall <julia@...u.dk>
>
> Each of these functions defines a variable err and a variable retval. err
> is always 0 and is returned in the error case. At one point near the end
> of the function, retval is initialized to an error code. For each
> function, the patch drops err, stores -ENOMEM in retval in two cases where
> memory allocation fails, and changes the return value in the error case
> from err to retval.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression ret;
> expression x,e1,e2,e3;
> @@
>
> ret = 0
> ... when != ret = e1
> *x = \(kmalloc\|kcalloc\|kzalloc\)(...)
> ... when != ret = e2
> if (x == NULL) { ... when != ret = e3
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@...u.dk>
>
> ---
> drivers/staging/cx25821/cx25821-audio-upstream.c | 11 +++++++----
> drivers/staging/cx25821/cx25821-video-upstream-ch2.c | 11 +++++++----
> drivers/staging/cx25821/cx25821-video-upstream.c | 11 +++++++----
> 3 files changed, 21 insertions(+), 12 deletions(-)
>
> diff -u -p a/drivers/staging/cx25821/cx25821-audio-upstream.c b/drivers/staging/cx25821/cx25821-audio-upstream.c
> --- a/drivers/staging/cx25821/cx25821-audio-upstream.c
> +++ b/drivers/staging/cx25821/cx25821-audio-upstream.c
> @@ -723,7 +723,6 @@ int cx25821_audio_upstream_init(struct c
> {
> struct sram_channel *sram_ch;
> int retval = 0;
> - int err = 0;
> int str_length = 0;
>
> if (dev->_audio_is_running) {
> @@ -756,8 +755,10 @@ int cx25821_audio_upstream_init(struct c
> str_length = strlen(dev->input_audiofilename);
> dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_audiofilename)
> + if (!dev->_audiofilename) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_audiofilename, dev->input_audiofilename,
> str_length + 1);
> @@ -770,8 +771,10 @@ int cx25821_audio_upstream_init(struct c
> str_length = strlen(_defaultAudioName);
> dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_audiofilename)
> + if (!dev->_audiofilename) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_audiofilename, _defaultAudioName, str_length + 1);
> }
> @@ -802,5 +805,5 @@ int cx25821_audio_upstream_init(struct c
> error:
> cx25821_dev_unregister(dev);
>
> - return err;
> + return retval;
> }
> diff -u -p a/drivers/staging/cx25821/cx25821-video-upstream-ch2.c b/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
> --- a/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
> +++ b/drivers/staging/cx25821/cx25821-video-upstream-ch2.c
> @@ -747,7 +747,6 @@ int cx25821_vidupstream_init_ch2(struct
> struct sram_channel *sram_ch;
> u32 tmp;
> int retval = 0;
> - int err = 0;
> int data_frame_size = 0;
> int risc_buffer_size = 0;
> int str_length = 0;
> @@ -792,8 +791,10 @@ int cx25821_vidupstream_init_ch2(struct
> str_length = strlen(dev->input_filename_ch2);
> dev->_filename_ch2 = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_filename_ch2)
> + if (!dev->_filename_ch2) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_filename_ch2, dev->input_filename_ch2,
> str_length + 1);
> @@ -801,8 +802,10 @@ int cx25821_vidupstream_init_ch2(struct
> str_length = strlen(dev->_defaultname_ch2);
> dev->_filename_ch2 = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_filename_ch2)
> + if (!dev->_filename_ch2) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_filename_ch2, dev->_defaultname_ch2,
> str_length + 1);
> @@ -851,5 +854,5 @@ int cx25821_vidupstream_init_ch2(struct
> error:
> cx25821_dev_unregister(dev);
>
> - return err;
> + return retval;
> }
> diff -u -p a/drivers/staging/cx25821/cx25821-video-upstream.c b/drivers/staging/cx25821/cx25821-video-upstream.c
> --- a/drivers/staging/cx25821/cx25821-video-upstream.c
> +++ b/drivers/staging/cx25821/cx25821-video-upstream.c
> @@ -800,7 +800,6 @@ int cx25821_vidupstream_init_ch1(struct
> struct sram_channel *sram_ch;
> u32 tmp;
> int retval = 0;
> - int err = 0;
> int data_frame_size = 0;
> int risc_buffer_size = 0;
> int str_length = 0;
> @@ -844,16 +843,20 @@ int cx25821_vidupstream_init_ch1(struct
> str_length = strlen(dev->input_filename);
> dev->_filename = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_filename)
> + if (!dev->_filename) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_filename, dev->input_filename, str_length + 1);
> } else {
> str_length = strlen(dev->_defaultname);
> dev->_filename = kmalloc(str_length + 1, GFP_KERNEL);
>
> - if (!dev->_filename)
> + if (!dev->_filename) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> memcpy(dev->_filename, dev->_defaultname, str_length + 1);
> }
> @@ -908,5 +911,5 @@ int cx25821_vidupstream_init_ch1(struct
> error:
> cx25821_dev_unregister(dev);
>
> - return err;
> + return retval;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists