[PATCH 1/2] ALSA: info: Avoid leaking kernel memory

[PATCH 1/2] ALSA: info: Avoid leaking kernel memory

[Takashi Iwai]

Make sure that the allocated buffer for reading the proc file won't
expose the uncleared kernel memory.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/info.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/core/info.c b/sound/core/info.c
index db308db..58e97b3 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -89,7 +89,7 @@ static int resize_info_buffer(struct snd_info_buffer *buffer,
 	char *nbuf;
 
 	nsize = PAGE_ALIGN(nsize);
-	nbuf = krealloc(buffer->buffer, nsize, GFP_KERNEL);
+	nbuf = krealloc(buffer->buffer, nsize, GFP_KERNEL | __GFP_ZERO);
 	if (! nbuf)
 		return -ENOMEM;
 
@@ -353,7 +353,7 @@ static int snd_info_entry_open(struct inode *inode, struct file *file)
 				goto __nomem;
 			data->rbuffer = buffer;
 			buffer->len = PAGE_SIZE;
-			buffer->buffer = kmalloc(buffer->len, GFP_KERNEL);
+			buffer->buffer = kzalloc(buffer->len, GFP_KERNEL);
 			if (buffer->buffer == NULL)
 				goto __nomem;
 		}
-- 
1.8.1.4

[PATCH 2/2] ALSA: info: Small refactoring and a sanity check in snd_info_get_line()

[Takashi Iwai]

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/info.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/sound/core/info.c b/sound/core/info.c
index 58e97b3..c9042b4 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -700,26 +700,21 @@ int snd_info_get_line(struct snd_info_buffer *buffer, char *line, int len)
 {
 	int c = -1;
 
+	if (snd_BUG_ON(!buffer || !buffer->buffer))
+		return 1;
 	if (len <= 0 || buffer->stop || buffer->error)
 		return 1;
-	while (--len > 0) {
+	while (!buffer->stop) {
 		c = buffer->buffer[buffer->curr++];
-		if (c == '\n') {
-			if (buffer->curr >= buffer->size)
-				buffer->stop = 1;
-			break;
-		}
-		*line++ = c;
-		if (buffer->curr >= buffer->size) {
+		if (buffer->curr >= buffer->size)
 			buffer->stop = 1;
+		if (c == '\n')
 			break;
+		if (len) {
+			len--;
+			*line++ = c;
 		}
 	}
-	while (c != '\n' && !buffer->stop) {
-		c = buffer->buffer[buffer->curr++];
-		if (buffer->curr >= buffer->size)
-			buffer->stop = 1;
-	}
 	*line = '\0';
 	return 0;
 }
-- 
1.8.1.4

Re: [PATCH 1/2] ALSA: info: Avoid leaking kernel memory

[David Henningsson]

On 03/13/2013 05:36 PM, Takashi Iwai wrote:
> Make sure that the allocated buffer for reading the proc file won't
> expose the uncleared kernel memory.

This should go to stable too, due to the security implications of 
leaking possibly sensitive information to userspace?

>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   sound/core/info.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sound/core/info.c b/sound/core/info.c
> index db308db..58e97b3 100644
> --- a/sound/core/info.c
> +++ b/sound/core/info.c
> @@ -89,7 +89,7 @@ static int resize_info_buffer(struct snd_info_buffer *buffer,
>   	char *nbuf;
>
>   	nsize = PAGE_ALIGN(nsize);
> -	nbuf = krealloc(buffer->buffer, nsize, GFP_KERNEL);
> +	nbuf = krealloc(buffer->buffer, nsize, GFP_KERNEL | __GFP_ZERO);
>   	if (! nbuf)
>   		return -ENOMEM;
>
> @@ -353,7 +353,7 @@ static int snd_info_entry_open(struct inode *inode, struct file *file)
>   				goto __nomem;
>   			data->rbuffer = buffer;
>   			buffer->len = PAGE_SIZE;
> -			buffer->buffer = kmalloc(buffer->len, GFP_KERNEL);
> +			buffer->buffer = kzalloc(buffer->len, GFP_KERNEL);
>   			if (buffer->buffer == NULL)
>   				goto __nomem;
>   		}
>



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH 1/2] ALSA: info: Avoid leaking kernel memory

[Takashi Iwai]

At Thu, 14 Mar 2013 07:15:28 +0100,
David Henningsson wrote:
> 
> On 03/13/2013 05:36 PM, Takashi Iwai wrote:
> > Make sure that the allocated buffer for reading the proc file won't
> > expose the uncleared kernel memory.
> 
> This should go to stable too, due to the security implications of 
> leaking possibly sensitive information to userspace?

It's no problem as long as the driver formats the proc output properly
via snd_iprintf(), thus no actual exposure happens in the codes we
have for now, AFAIK.

The patch is just to be sure on the ground level.


Takashi

> 
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >   sound/core/info.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/core/info.c b/sound/core/info.c
> > index db308db..58e97b3 100644
> > --- a/sound/core/info.c
> > +++ b/sound/core/info.c
> > @@ -89,7 +89,7 @@ static int resize_info_buffer(struct snd_info_buffer *buffer,
> >   	char *nbuf;
> >
> >   	nsize = PAGE_ALIGN(nsize);
> > -	nbuf = krealloc(buffer->buffer, nsize, GFP_KERNEL);
> > +	nbuf = krealloc(buffer->buffer, nsize, GFP_KERNEL | __GFP_ZERO);
> >   	if (! nbuf)
> >   		return -ENOMEM;
> >
> > @@ -353,7 +353,7 @@ static int snd_info_entry_open(struct inode *inode, struct file *file)
> >   				goto __nomem;
> >   			data->rbuffer = buffer;
> >   			buffer->len = PAGE_SIZE;
> > -			buffer->buffer = kmalloc(buffer->len, GFP_KERNEL);
> > +			buffer->buffer = kzalloc(buffer->len, GFP_KERNEL);
> >   			if (buffer->buffer == NULL)
> >   				goto __nomem;
> >   		}
> >
> 
> 
> 
> -- 
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
>