[PATCH] Fix kfree() corruption in sock_read_buffer_sendmsg()

[PATCH] Fix kfree() corruption in sock_read_buffer_sendmsg()

[Dan Smith]

The memcpy_from_iovec() function that the unix sendmsg functions use modifies
the struct msghdr.  Since the current code uses the msg.iovec_base pointer
in the msghdr for the kmalloc() and kfree(), we end up freeing the wrong
pointer.  This patch stores the original address in a separate pointer and
corrects the kfree() call to use it.

Cc: serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org
Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 net/unix/checkpoint.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/unix/checkpoint.c b/net/unix/checkpoint.c
index 841d25d..65b7025 100644
--- a/net/unix/checkpoint.c
+++ b/net/unix/checkpoint.c
@@ -118,6 +118,7 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sock)
 {
 	struct msghdr msg;
 	struct kvec kvec;
+	void *buf;
 	int ret = 0;
 	int len;
 
@@ -134,8 +135,9 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sock)
 	}
 
 	kvec.iov_len = len;
-	kvec.iov_base = kmalloc(len, GFP_KERNEL);
-	if (!kvec.iov_base)
+	buf = kmalloc(len, GFP_KERNEL);
+	kvec.iov_base = buf;
+	if (!buf)
 		return -ENOMEM;
 
 	ret = ckpt_kread(ctx, kvec.iov_base, len);
@@ -147,7 +149,7 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sock)
 	if ((ret > 0) && (ret != len))
 		ret = -ENOMEM;
  out:
-	kfree(kvec.iov_base);
+	kfree(buf);
 
 	return ret;
 }
-- 
1.6.2.5

Re: [PATCH] Fix kfree() corruption in sock_read_buffer_sendmsg()

[Serge E. Hallyn]

Quoting Dan Smith (danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org):
> The memcpy_from_iovec() function that the unix sendmsg functions use modifies
> the struct msghdr.  Since the current code uses the msg.iovec_base pointer
> in the msghdr for the kmalloc() and kfree(), we end up freeing the wrong
> pointer.  This patch stores the original address in a separate pointer and
> corrects the kfree() call to use it.
> 
> Cc: serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org
> Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Tested-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

> ---
>  net/unix/checkpoint.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/net/unix/checkpoint.c b/net/unix/checkpoint.c
> index 841d25d..65b7025 100644
> --- a/net/unix/checkpoint.c
> +++ b/net/unix/checkpoint.c
> @@ -118,6 +118,7 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sock)
>  {
>  	struct msghdr msg;
>  	struct kvec kvec;
> +	void *buf;
>  	int ret = 0;
>  	int len;
> 
> @@ -134,8 +135,9 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sock)
>  	}
> 
>  	kvec.iov_len = len;
> -	kvec.iov_base = kmalloc(len, GFP_KERNEL);
> -	if (!kvec.iov_base)
> +	buf = kmalloc(len, GFP_KERNEL);
> +	kvec.iov_base = buf;
> +	if (!buf)
>  		return -ENOMEM;
> 
>  	ret = ckpt_kread(ctx, kvec.iov_base, len);
> @@ -147,7 +149,7 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sock)
>  	if ((ret > 0) && (ret != len))
>  		ret = -ENOMEM;
>   out:
> -	kfree(kvec.iov_base);
> +	kfree(buf);
> 
>  	return ret;
>  }
> -- 
> 1.6.2.5

Re: [PATCH] Fix kfree() corruption in sock_read_buffer_sendmsg()

[Oren Laadan]

Serge E. Hallyn wrote:
> Quoting Dan Smith (danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org):
>> The memcpy_from_iovec() function that the unix sendmsg functions use modifies
>> the struct msghdr.  Since the current code uses the msg.iovec_base pointer
>> in the msghdr for the kmalloc() and kfree(), we end up freeing the wrong
>> pointer.  This patch stores the original address in a separate pointer and
>> corrects the kfree() call to use it.
>>
>> Cc: serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org
>> Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> 
> Tested-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Pulled.

Oren.