Skip to contentSkip to conversation

Fix kfree bug in sendmsg and recvmsg

PATCH In review Joe Korty 3 participants 4 messages Feb 17, 2016

Subsystems (2)
lore
Share & tools
lore

Joe Korty

author
#0 lore raw

PATCH 1/1 Fix kfree bug in sendmsg and recvmsg

Fix kfree bug in recvmsg and sendmsg.

We cannot kfree(iov) when iov points to an array on the stack, as that has the potential of corrupting memory.

So re-introduce the if-stmt that used to protect kfree from this condition, code that was removed as part of a larger set of changes made by git commit da184284.

net/socket.c +4 -2
Index: b/net/socket.c
===================================================================
@@ -1960,7 +1960,8 @@ out_freectl:
 	if (ctl_buf != ctl)
 		sock_kfree_s(sock->sk, ctl_buf, ctl_len);
 out_freeiov:
-	kfree(iov);
+	if (iov != iovstack)
+		kfree(iov);
 	return err;
 }
 
@@ -2125,7 +2126,8 @@ static int ___sys_recvmsg(struct socket 
 	err = len;
 
 out_freeiov:
-	kfree(iov);
+	if (iov != iovstack)
+		kfree(iov);
 	return err;
 }
 
#1 lore raw
Quoted 8 lines · Joe Korty

Fix kfree bug in recvmsg and sendmsg.

We cannot kfree(iov) when iov points to an array on the stack, as that has the potential of corrupting memory.

So re-introduce the if-stmt that used to protect kfree from this condition, code that was removed as part of a larger set of changes made by git commit da184284.

NAK. You are misreading import_iovec():

        *iov = p == *iov ? NULL : p;

in the end will have iov replaced with NULL if we ended up using what it originally pointed to.

#2 lore raw
Quoted 6 lines · Al Viro
Quoted 5 lines · Joe Korty

Fix kfree bug in recvmsg and sendmsg.

We cannot kfree(iov) when iov points to an array on the stack, as that has the potential of corrupting memory.

Quoted 8 lines
Quoted 3 lines

So re-introduce the if-stmt that used to protect kfree from this condition, code that was removed as part of a larger set of changes made by git commit da184284.

NAK. You are misreading import_iovec():

        *iov = p == *iov ? NULL : p;

in the end will have iov replaced with NULL if we ended up using what it originally pointed to.

PS: that's absolutely deliberate - cleanup after import_iovec() (and its wrappers) is *always* of the same sort; that way we avoid the checks completely. The rules are much simpler that way: after the call of import_iovec() cleanup is always kfree(iov), no matter what and that's the only thing iov should be used after that call.

Your "restored" check won't harm anything, precisely because it'll never be satisfied. Note that this is the normal codepath - we go through out_freeiov on success as well. If iov remained pointing to on-stack array in case of short vector, we would've been ears-deep in oopsen. The reason why we are not is that in this case iov ends up being NULL.

maintainer
#3 lore raw
In reply to Joe Korty

From: Joe Korty <[email protected]>
Date: Wed, 17 Feb 2016 11:38:05 -0500

Quoted 10 lines

Fix kfree bug in recvmsg and sendmsg.

We cannot kfree(iov) when iov points to an array on the stack, as that has the potential of corrupting memory.

So re-introduce the if-stmt that used to protect kfree from this condition, code that was removed as part of a larger set of changes made by git commit da184284.

Please submit networking patches to [email protected], thank you.