[PATCH] drivers/tty/tty_io.c: fix a potential memleak at do_tty_write()

[PATCH] drivers/tty/tty_io.c: fix a potential memleak at do_tty_write()

[Jeff Liu]

Hello,

Looks there is a potential memory leak at drivers/tty/tty_io.c: do_tty_write().
It did allocate a buf_chunk if tty->write_cnt < chunk, however, buf_chunk was not
freed after the writing is done.  Below tiny patch could fix it.

Thanks,
-Jeff


diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b425c79..f09e73e 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1011,6 +1011,7 @@ static inline ssize_t do_tty_write(
 	size_t count)
 {
 	ssize_t ret, written = 0;
+	unsigned char *buf_chunk = NULL;
 	unsigned int chunk;
 
 	ret = tty_write_lock(tty, file->f_flags & O_NDELAY);
@@ -1041,8 +1042,6 @@ static inline ssize_t do_tty_write(
 
 	/* write_buf/write_cnt is protected by the atomic_write_lock mutex */
 	if (tty->write_cnt < chunk) {
-		unsigned char *buf_chunk;
-
 		if (chunk < 1024)
 			chunk = 1024;
 
@@ -1082,6 +1081,9 @@ static inline ssize_t do_tty_write(
 		inode->i_mtime = current_fs_time(inode->i_sb);
 		ret = written;
 	}
+
+	if (buf_chunk)
+		kfree(buf_chunk);
 out:
 	tty_write_unlock(tty);
 	return ret;

Re: [PATCH] drivers/tty/tty_io.c: fix a potential memleak at do_tty_write()

[Greg KH]

On Mon, Jun 18, 2012 at 08:23:54PM +0800, Jeff Liu wrote:
> Hello,
> 
> Looks there is a potential memory leak at drivers/tty/tty_io.c: do_tty_write().
> It did allocate a buf_chunk if tty->write_cnt < chunk, however, buf_chunk was not
> freed after the writing is done.  Below tiny patch could fix it.
> 
> Thanks,
> -Jeff
> 

Thanks for the patch, but can you resend it with a signed-off-by: line
as described in Documentation/SubmittingPatches so that we can properly
apply the patch?

thanks,

greg k-h

Re: [PATCH] drivers/tty/tty_io.c: fix a potential memleak at do_tty_write()

[Alan Cox]

On Mon, 18 Jun 2012 20:23:54 +0800
Jeff Liu <jeff.liu@oracle.com> wrote:

> Hello,
> 
> Looks there is a potential memory leak at drivers/tty/tty_io.c: do_tty_write().
> It did allocate a buf_chunk if tty->write_cnt < chunk, however, buf_chunk was not
> freed after the writing is done.  Below tiny patch could fix it.

Why should it be freed, we still have a reference to it.


Alan

Re: [PATCH] drivers/tty/tty_io.c: fix a potential memleak at do_tty_write()

[Paul Fulghum]

On 6/18/2012 11:04 AM, Alan Cox wrote:
> On Mon, 18 Jun 2012 20:23:54 +0800
> Jeff Liu <jeff.liu@oracle.com> wrote:
> 
>> Hello,
>>
>> Looks there is a potential memory leak at drivers/tty/tty_io.c: do_tty_write().
>> It did allocate a buf_chunk if tty->write_cnt < chunk, however, buf_chunk was not
>> freed after the writing is done.  Below tiny patch could fix it.
> 
> Why should it be freed, we still have a reference to it.

Yeah, it would be messy on the next write()
when the now freed tty->write_buf is accessed ;-)
*boom*

-- 
Paul Fulghum
MicroGate Systems, Ltd.

Re: [PATCH] drivers/tty/tty_io.c: fix a potential memleak at do_tty_write()

[Jeff Liu]

On 06/19/2012 02:00 AM, Paul Fulghum wrote:

> On 6/18/2012 11:04 AM, Alan Cox wrote:
>> On Mon, 18 Jun 2012 20:23:54 +0800
>> Jeff Liu <jeff.liu@oracle.com> wrote:
>>
>>> Hello,
>>>
>>> Looks there is a potential memory leak at drivers/tty/tty_io.c: do_tty_write().
>>> It did allocate a buf_chunk if tty->write_cnt < chunk, however, buf_chunk was not
>>> freed after the writing is done.  Below tiny patch could fix it.
>>
>> Why should it be freed, we still have a reference to it.
> 
> Yeah, it would be messy on the next write()
> when the now freed tty->write_buf is accessed ;-)
> *boom*

Oops! I took for granted previously. Duh. :(

Sorry for the noise!

-Jeff