Skip to contentSkip to conversation

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

PATCH In review Jeff Liu 4 participants 5 messages Jun 18, 2012 – Jun 19, 2012 TTY layer and serial drivers

lore
Share & tools
lore

Jeff Liu

author
#0 lore raw

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

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

drivers/tty/tty_io.c +4 -2
index b425c79..f09e73e 100644
@@ -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;
maintainer
#1 lore raw
Quoted 9 lines · 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

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

#2 lore raw
In reply to Jeff Liu
Quoted 5 lines · On Mon, 18 Jun 2012 20:23:54 +0800 Jeff Liu <[email protected]> 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

#3 lore raw
Quoted 10 lines · Alan Cox
Quoted 5 lines · On Mon, 18 Jun 2012 20:23:54 +0800 Jeff Liu <[email protected]> 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*

Signature · 2 lines
Paul Fulghum
MicroGate Systems, Ltd.
author
#4 lore raw
Quoted 10 lines · Paul Fulghum
Quoted 9 lines · Alan Cox
Quoted 5 lines · On Mon, 18 Jun 2012 20:23:54 +0800 Jeff Liu <[email protected]> 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.

Quoted 5 lines
Quoted 1 line

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