diff mbox series

fs/read_write.c: Fix memory leak in read_write.c

Message ID 1592968023-20383-1-git-send-email-fanpeng@loongson.cn (mailing list archive)
State New, archived
Headers show
Series fs/read_write.c: Fix memory leak in read_write.c | expand

Commit Message

Peng Fan June 24, 2020, 3:07 a.m. UTC
kmemleak report:
unreferenced object 0x98000002bb591d00 (size 256):
  comm "ftest03", pid 24778, jiffies 4301603810 (age 490.665s)
  hex dump (first 32 bytes):
    00 01 04 20 01 00 00 00 80 00 00 00 00 00 00 00  ... ............
    f0 02 04 20 01 00 00 00 80 00 00 00 00 00 00 00  ... ............
  backtrace:
    [<0000000050b162cb>] __kmalloc+0x234/0x438
    [<00000000491da9c7>] rw_copy_check_uvector+0x1ac/0x1f0
    [<00000000b0dddb43>] import_iovec+0x50/0xe8
    [<00000000ae843d73>] vfs_readv+0x50/0xb0
    [<00000000c7216b06>] do_readv+0x80/0x160
    [<00000000cad79c3f>] syscall_common+0x34/0x58

This is because "iov" allocated by kmalloc() is not destroyed. Under normal
circumstances, "ret_pointer" should be equal to "iov". But if the previous 
statements fails to execute, and the allocation is successful, then the
block of memory will not be released, because it is necessary to 
determine whether they are equal. So we need to change the order.

Signed-off-by: Peng Fan <fanpeng@loongson.cn>
---
 fs/read_write.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Matthew Wilcox (Oracle) June 24, 2020, 3:24 a.m. UTC | #1
On Wed, Jun 24, 2020 at 11:07:03AM +0800, Peng Fan wrote:
> kmemleak report:
> unreferenced object 0x98000002bb591d00 (size 256):
>   comm "ftest03", pid 24778, jiffies 4301603810 (age 490.665s)
>   hex dump (first 32 bytes):
>     00 01 04 20 01 00 00 00 80 00 00 00 00 00 00 00  ... ............
>     f0 02 04 20 01 00 00 00 80 00 00 00 00 00 00 00  ... ............
>   backtrace:
>     [<0000000050b162cb>] __kmalloc+0x234/0x438
>     [<00000000491da9c7>] rw_copy_check_uvector+0x1ac/0x1f0
>     [<00000000b0dddb43>] import_iovec+0x50/0xe8
>     [<00000000ae843d73>] vfs_readv+0x50/0xb0
>     [<00000000c7216b06>] do_readv+0x80/0x160
>     [<00000000cad79c3f>] syscall_common+0x34/0x58
> 
> This is because "iov" allocated by kmalloc() is not destroyed. Under normal
> circumstances, "ret_pointer" should be equal to "iov". But if the previous 
> statements fails to execute, and the allocation is successful, then the
> block of memory will not be released, because it is necessary to 
> determine whether they are equal. So we need to change the order.

This patch doesn't make sense.  It will _introduce_ a memory leak,
not fix one.
diff mbox series

Patch

diff --git a/fs/read_write.c b/fs/read_write.c
index bbfa9b1..aa4f7c5 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -832,8 +832,8 @@  ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,
 		}
 		ret += len;
 	}
-out:
 	*ret_pointer = iov;
+out:
 	return ret;
 }