RE: [PATCH] Tools: hv: correct payload size in netlink_send

[KY Srinivasan <kys@microsoft.com>]

> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Wednesday, August 07, 2013 9:07 AM
> To: KY Srinivasan; gregkh@linuxfoundation.org
> Cc: linux-kernel@vger.kernel.org; Olaf Hering
> Subject: [PATCH] Tools: hv: correct payload size in netlink_send
> 
> netlink_send is supposed to send just the cn_msg+hv_kvp_msg via netlink.
> Currently it sets an incorrect iovec size, as reported by valgrind.
> 
> In the case of registering with the kernel the allocated buffer is large
> enough to hold nlmsghdr+cn_msg+hv_kvp_msg, no overrun happens. In the
> case of responding to the kernel the cn_msg is located in the middle of
> recv_buffer, after the nlmsghdr. Currently the code in netlink_send adds
> also the size of nlmsghdr to the payload. But nlmsghdr is a separate
> iovec. This leads to an (harmless) out-of-bounds access when the kernel
> processes the iovec. Correct the iovec size of the cn_msg to be just
> cn_msg + its payload.

Thanks Olaf.

> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

> ---
>  tools/hv/hv_kvp_daemon.c | 2 +-
>  tools/hv/hv_vss_daemon.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index d3bcb84..1bd1ad1 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -1399,7 +1399,7 @@ netlink_send(int fd, struct cn_msg *msg)
>  	char buffer[64];
>  	struct iovec iov[2];
> 
> -	size = NLMSG_SPACE(sizeof(struct cn_msg) + msg->len);
> +	size = sizeof(struct cn_msg) + msg->len;
> 
>  	nlh = (struct nlmsghdr *)buffer;
>  	nlh->nlmsg_seq = 0;
> diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
> index 6b4f2fa..2f1f53f 100644
> --- a/tools/hv/hv_vss_daemon.c
> +++ b/tools/hv/hv_vss_daemon.c
> @@ -111,7 +111,7 @@ static int netlink_send(int fd, struct cn_msg *msg)
>  	char buffer[64];
>  	struct iovec iov[2];
> 
> -	size = NLMSG_SPACE(sizeof(struct cn_msg) + msg->len);
> +	size = sizeof(struct cn_msg) + msg->len;
> 
>  	nlh = (struct nlmsghdr *)buffer;
>  	nlh->nlmsg_seq = 0;