On Sun, Apr 21, 2013 at 01:40:25PM +0200, walter harms wrote:

Am 21.04.2013 13:07, schrieb Dan Carpenter:

...

If the ->get_caps() function doesn't clear the buffer then there would stack information leaked to userspace. For example, soc_compr_get_caps() can return success without clearing the buffer.

Signed-off-by: Dan Carpenter dan.carpenter@oracle.com

Perhaps the soc_compr_get_caps() function should return an error code if the platform->driver->compr_ops is NULL. I'm not sure about that, and it's a separate issue anyway.

diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index c84abc8..8d3190a 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -375,6 +375,7 @@ snd_compr_get_caps(struct snd_compr_stream *stream, unsigned long arg) if (!stream->ops->get_caps) return -ENXIO;

• memset(&caps, 0, sizeof(caps)); retval = stream->ops->get_caps(stream, &caps); if (retval) goto out;

--

IMHO this should be done in get_caps() as it will manipulate the entries. or is there a special reason to have it here ?

There isn't a get_caps() function. It's a pointer to many functions. It's safer to do it here where it can be audited by static analisys. It's cleaner to do it once instead of many times.

regards, dan carpenter

On Mon, Apr 22, 2013 at 10:42:25AM +0200, Takashi Iwai wrote:

At Sun, 21 Apr 2013 14:07:29 +0300, Dan Carpenter wrote:

...

If the ->get_caps() function doesn't clear the buffer then there would stack information leaked to userspace. For example, soc_compr_get_caps() can return success without clearing the buffer.

Thanks, applied now.

I found that we have also a similar problem in other ioctls (snd_compr_get_codec_caps() and snd_compr_get_params()), so applied the following patch in addition.

Takashi

---

From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: compress: Use kzalloc() for ioctls writing back data

Like the previous patch by Dan, we should clear the data to be returned from certain compress ioctls, namely, snd_compr_get_codec_caps() and snd_compr_get_params(). This time, we can simply replace kmalloc() with kzalloc().

Signed-off-by: Takashi Iwai tiwai@suse.de

If not too late.. Both Acked-by: Vinod Koul vinod.koul@intel.com

---

sound/core/compress_offload.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index 664c693..a0bc47f 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -428,7 +428,7 @@ snd_compr_get_codec_caps(struct snd_compr_stream *stream, unsigned long arg) if (!stream->ops->get_codec_caps) return -ENXIO;

• caps = kmalloc(sizeof(*caps), GFP_KERNEL);

• caps = kzalloc(sizeof(*caps), GFP_KERNEL); if (!caps) return -ENOMEM;

@@ -545,7 +545,7 @@ snd_compr_get_params(struct snd_compr_stream *stream, unsigned long arg) if (!stream->ops->get_params) return -EBADFD;

• params = kmalloc(sizeof(*params), GFP_KERNEL);

• params = kzalloc(sizeof(*params), GFP_KERNEL); if (!params) return -ENOMEM; retval = stream->ops->get_params(stream, params);

-- 1.8.2.1