Re: [patch] ALSA: compress: info leak in snd_compr_get_caps()

[Dan Carpenter <dan.carpenter@oracle.com>]

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