Skip to contentSkip to conversation

ext4: fix memory leak in xattr code.

PATCH Applied Dave Jones 3 participants 3 messages Sep 20, 2013 – Sep 22, 2013 ext4 file system

lore
Share & tools
lore

Dave Jones

author
#0 lore raw

If we take the 2nd retry path in ext4_expand_extra_isize_ea, we potentionally return from the function without having freed these allocations. If we don't do the return, we over-write the previous allocation pointers, so we leak either way.

Spotted with Coverity.

Signed-off-by Dave Jones <[email protected]>
fs/ext4/xattr.c +2 -0
index c081e34..f3a6220 100644
@@ -1350,6 +1350,8 @@ retry:
 				    s_min_extra_isize) {
 					tried_min_extra_isize++;
 					new_extra_isize = s_min_extra_isize;
+					kfree(is);
+					kfree(bs);
 					goto retry;
 				}
 				error = -1;
Reviewed-by
#1 lore raw
Quoted 8 lines · Dave Jones

If we take the 2nd retry path in ext4_expand_extra_isize_ea, we potentionally return from the function without having freed these allocations. If we don't do the return, we over-write the previous allocation pointers, so we leak either way.

Spotted with Coverity.

Signed-off-by Dave Jones <[email protected]>

Looks right to me.

Reviewed-by Eric Sandeen <[email protected]>

Thanks!

-Eric

Quoted 18 lines
fs/ext4/xattr.c +2 -1
index c081e34..f3a6220 100644
@@ -1350,6 +1350,8 @@ retry:
 				    s_min_extra_isize) {
 					tried_min_extra_isize++;
 					new_extra_isize = s_min_extra_isize;
+					kfree(is);
+					kfree(bs);
 					goto retry;
 				}
 				error = -1;
--

To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer Reviewed-by Eric Sandeen Applied
#2 lore raw

Thanks, applied with a minor bug fix...

- Ted

commit 8484764f97a840df851f4586079f28715f13318e

Author: Dave Jones <[email protected]>
Date:   Sun Sep 22 15:31:00 2013 -0400

ext4: fix memory leak in xattr

If we take the 2nd retry path in ext4_expand_extra_isize_ea, we potentionally return from the function without having freed these allocations. If we don't do the return, we over-write the previous allocation pointers, so we leak either way.

Spotted with Coverity.

[ Fixed by tytso to set is and bs to NULL after freeing these
pointers, in case in the retry loop we later end up triggering an error causing a jump to cleanup, at which point we could have a double free bug. -- Ted ]

Signed-off-by Dave Jones <[email protected]>
Signed-off-by "Theodore Ts'o" <[email protected]>
Reviewed-by Eric Sandeen <[email protected]>
fs/ext4/xattr.c +3 -0
index c081e34..3c53192 100644
@@ -1350,6 +1350,9 @@ retry:
 				    s_min_extra_isize) {
 					tried_min_extra_isize++;
 					new_extra_isize = s_min_extra_isize;
+					kfree(is);
+					kfree(bs);
+					is = bs = NULL;
 					goto retry;
 				}
 				error = -1;