[Openvpn-devel] [PATCH] Zero initialise msghdr prior to calling sendmesg

[Openvpn-devel] [PATCH] Zero initialise msghdr prior to calling sendmesg

[Arne Schwabe]

This ensure that all unused fields in msg are zero.

Spotted by Coverity:

Using uninitialized value "msg". Field "msg.msg_flags" is uninitialized
when calling "sendmsg".
---
 src/openvpn/manage.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index a4f99c9a..103ccadc 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -2092,7 +2092,7 @@ man_io_error(struct management *man, const char *prefix)
 static ssize_t
 man_send_with_fd(int fd, void *ptr, size_t nbytes, int flags, int sendfd)
 {
-    struct msghdr msg;
+    struct msghdr msg = {0};
     struct iovec iov[1];
 
     union {
@@ -2124,7 +2124,7 @@ man_send_with_fd(int fd, void *ptr, size_t nbytes, int flags, int sendfd)
 static ssize_t
 man_recv_with_fd(int fd, void *ptr, size_t nbytes, int flags, int *recvfd)
 {
-    struct msghdr msghdr;
+    struct msghdr msghdr = {0};
     struct iovec iov[1];
     ssize_t n;
 
-- 
2.26.2

Re: [Openvpn-devel] [PATCH] Zero initialise msghdr prior to calling sendmesg

[Antonio Quartulli]

Hi,


On 05/01/2021 14:17, Arne Schwabe wrote:
> This ensure that all unused fields in msg are zero.
> 
> Spotted by Coverity:
> 
> Using uninitialized value "msg". Field "msg.msg_flags" is uninitialized
> when calling "sendmsg".

No signed-off-by ?

> ---
>  src/openvpn/manage.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
> index a4f99c9a..103ccadc 100644
> --- a/src/openvpn/manage.c
> +++ b/src/openvpn/manage.c
> @@ -2092,7 +2092,7 @@ man_io_error(struct management *man, const char *prefix)
>  static ssize_t
>  man_send_with_fd(int fd, void *ptr, size_t nbytes, int flags, int sendfd)
>  {
> -    struct msghdr msg;
> +    struct msghdr msg = {0};

In the rest of the code we have spaces around the 0, like "{ 0 }"
I suggest using the same style.

>      struct iovec iov[1];
>  
>      union {
> @@ -2124,7 +2124,7 @@ man_send_with_fd(int fd, void *ptr, size_t nbytes, int flags, int sendfd)
>  static ssize_t
>  man_recv_with_fd(int fd, void *ptr, size_t nbytes, int flags, int *recvfd)
>  {
> -    struct msghdr msghdr;
> +    struct msghdr msghdr = {0};

same as above.

>      struct iovec iov[1];
>      ssize_t n;
>  
> 

Other than that it's Feature-ACK.
We should always fully initialize objects that we pass around.

Cheers,

-- 
Antonio Quartulli

[Openvpn-devel] [PATCH applied] Re: Zero initialise msghdr prior to calling sendmesg

[Gert Doering]

I have adjusted the whitespace as requested by Antonio, and added your 
SoB-Line (as agreed on IRC).

Your patch has been applied to the master, and release/2.5 branch (bugfix).

I have not merged it to 2.4 - it would nicely fit, but since this is all
TARGET_ANDROID, and nobody builds the Android client with an old tree, this
would just be "repo commit noise".

commit aa58035a955a2ae7ffa2b93ca2c8d2c6e5472695 (master)
commit a65d39bfd27cf5612e55dd536dd189e1a62624c2 (release/2.5)
Author: Arne Schwabe
Date:   Tue Jan 5 14:17:58 2021 +0100

     Zero initialise msghdr prior to calling sendmesg

     Signed-off-by: Arne Schwabe <arne@...1227...>
     Acked-by: Antonio Quartulli <antonio@...515...>
     Message-Id: <20210105131758.20311-1-arne@...1227...>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21418.html
     Signed-off-by: Gert Doering <gert@...1296...>


--
kind regards,

Gert Doering