[PATCH] Fix kfree bug in sendmsg and recvmsg

[PATCH] Fix kfree bug in sendmsg and recvmsg

[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.

Signed-off-by: Joe Korty <joe.korty@ccur.com>

Index: b/net/socket.c
===================================================================
--- a/net/socket.c
+++ 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;
 }
 

Re: [PATCH] Fix kfree bug in sendmsg and recvmsg

[Al Viro]

On Wed, Feb 17, 2016 at 11:38:05AM -0500, Joe Korty wrote:
> 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.

Re: [PATCH] Fix kfree bug in sendmsg and recvmsg

[Al Viro]

On Wed, Feb 17, 2016 at 04:44:07PM +0000, Al Viro wrote:
> On Wed, Feb 17, 2016 at 11:38:05AM -0500, Joe Korty wrote:
> > 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.

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.

Re: [PATCH] Fix kfree bug in sendmsg and recvmsg

[David Miller]

From: Joe Korty <joe.korty@ccur.com>
Date: Wed, 17 Feb 2016 11:38:05 -0500

> 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.
> 
> Signed-off-by: Joe Korty <joe.korty@ccur.com>

Please submit networking patches to netdev@vger.kernel.org, thank you.