linux: Check for null value msghdr struct before use

Message ID 20210702202810.2779765-1-raj.khem@gmail.com
State Committed
Delegated to: Adhemerval Zanella Netto
Headers
Series linux: Check for null value msghdr struct before use |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Khem Raj July 2, 2021, 8:28 p.m. UTC
  This avoids crashes in libc when cmsg is null and refrencing msg
structure when it is null

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 sysdeps/unix/sysv/linux/convert_scm_timestamps.c | 2 ++
 sysdeps/unix/sysv/linux/recvmsg.c                | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)
  

Comments

Adhemerval Zanella July 5, 2021, 5:51 p.m. UTC | #1
On 02/07/2021 17:28, Khem Raj wrote:
> This avoids crashes in libc when cmsg is null and refrencing msg

s/refrencing/referencing

> structure when it is null
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>

Patch looks ok just some nits below, thank for catching it.  
I will commit it shortly for you.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/unix/sysv/linux/convert_scm_timestamps.c | 2 ++
>  sysdeps/unix/sysv/linux/recvmsg.c                | 4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> index d75a4618dd6..5af71847f57 100644
> --- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> +++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> @@ -87,6 +87,8 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>  
>    msg->msg_controllen += CMSG_SPACE (sizeof tvts);
>    cmsg = CMSG_NXTHDR(msg, last);
> +  if (cmsg == NULL)
> +    return;
>    cmsg->cmsg_level = SOL_SOCKET;
>    cmsg->cmsg_type = type;
>    cmsg->cmsg_len = CMSG_LEN (sizeof tvts);
> diff --git a/sysdeps/unix/sysv/linux/recvmsg.c b/sysdeps/unix/sysv/linux/recvmsg.c
> index a2a600228ba..19c49e2a85c 100644
> --- a/sysdeps/unix/sysv/linux/recvmsg.c
> +++ b/sysdeps/unix/sysv/linux/recvmsg.c
> @@ -25,7 +25,7 @@ __libc_recvmsg (int fd, struct msghdr *msg, int flags)
>  {
>    ssize_t r;
>  #ifndef __ASSUME_TIME64_SYSCALLS
> -  socklen_t orig_controllen = msg->msg_controllen;
> +  socklen_t orig_controllen = (msg) ? msg->msg_controllen : 0;
>  #endif
>  

No implicit checks.

>  #ifdef __ASSUME_RECVMSG_SYSCALL
> @@ -35,7 +35,7 @@ __libc_recvmsg (int fd, struct msghdr *msg, int flags)
>  #endif
>  
>  #ifndef __ASSUME_TIME64_SYSCALLS
> -  if (r >= 0)
> +  if (r >= 0 && orig_controllen)
>      __convert_scm_timestamps (msg, orig_controllen);
>  #endif
>  
> 

Same as before.
  
Khem Raj July 6, 2021, 4:22 p.m. UTC | #2
On Mon, Jul 5, 2021 at 10:51 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 02/07/2021 17:28, Khem Raj wrote:
> > This avoids crashes in libc when cmsg is null and refrencing msg
>
> s/refrencing/referencing
>
> > structure when it is null
> >
> > Signed-off-by: Khem Raj <raj.khem@gmail.com>
>
> Patch looks ok just some nits below, thank for catching it.
> I will commit it shortly for you.

Thank you Adhemerval

>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>
> > ---
> >  sysdeps/unix/sysv/linux/convert_scm_timestamps.c | 2 ++
> >  sysdeps/unix/sysv/linux/recvmsg.c                | 4 ++--
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> > index d75a4618dd6..5af71847f57 100644
> > --- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> > +++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> > @@ -87,6 +87,8 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
> >
> >    msg->msg_controllen += CMSG_SPACE (sizeof tvts);
> >    cmsg = CMSG_NXTHDR(msg, last);
> > +  if (cmsg == NULL)
> > +    return;
> >    cmsg->cmsg_level = SOL_SOCKET;
> >    cmsg->cmsg_type = type;
> >    cmsg->cmsg_len = CMSG_LEN (sizeof tvts);
> > diff --git a/sysdeps/unix/sysv/linux/recvmsg.c b/sysdeps/unix/sysv/linux/recvmsg.c
> > index a2a600228ba..19c49e2a85c 100644
> > --- a/sysdeps/unix/sysv/linux/recvmsg.c
> > +++ b/sysdeps/unix/sysv/linux/recvmsg.c
> > @@ -25,7 +25,7 @@ __libc_recvmsg (int fd, struct msghdr *msg, int flags)
> >  {
> >    ssize_t r;
> >  #ifndef __ASSUME_TIME64_SYSCALLS
> > -  socklen_t orig_controllen = msg->msg_controllen;
> > +  socklen_t orig_controllen = (msg) ? msg->msg_controllen : 0;
> >  #endif
> >
>
> No implicit checks.
>
> >  #ifdef __ASSUME_RECVMSG_SYSCALL
> > @@ -35,7 +35,7 @@ __libc_recvmsg (int fd, struct msghdr *msg, int flags)
> >  #endif
> >
> >  #ifndef __ASSUME_TIME64_SYSCALLS
> > -  if (r >= 0)
> > +  if (r >= 0 && orig_controllen)
> >      __convert_scm_timestamps (msg, orig_controllen);
> >  #endif
> >
> >
>
> Same as before.
  

Patch

diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
index d75a4618dd6..5af71847f57 100644
--- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
+++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
@@ -87,6 +87,8 @@  __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
 
   msg->msg_controllen += CMSG_SPACE (sizeof tvts);
   cmsg = CMSG_NXTHDR(msg, last);
+  if (cmsg == NULL)
+    return;
   cmsg->cmsg_level = SOL_SOCKET;
   cmsg->cmsg_type = type;
   cmsg->cmsg_len = CMSG_LEN (sizeof tvts);
diff --git a/sysdeps/unix/sysv/linux/recvmsg.c b/sysdeps/unix/sysv/linux/recvmsg.c
index a2a600228ba..19c49e2a85c 100644
--- a/sysdeps/unix/sysv/linux/recvmsg.c
+++ b/sysdeps/unix/sysv/linux/recvmsg.c
@@ -25,7 +25,7 @@  __libc_recvmsg (int fd, struct msghdr *msg, int flags)
 {
   ssize_t r;
 #ifndef __ASSUME_TIME64_SYSCALLS
-  socklen_t orig_controllen = msg->msg_controllen;
+  socklen_t orig_controllen = (msg) ? msg->msg_controllen : 0;
 #endif
 
 #ifdef __ASSUME_RECVMSG_SYSCALL
@@ -35,7 +35,7 @@  __libc_recvmsg (int fd, struct msghdr *msg, int flags)
 #endif
 
 #ifndef __ASSUME_TIME64_SYSCALLS
-  if (r >= 0)
+  if (r >= 0 && orig_controllen)
     __convert_scm_timestamps (msg, orig_controllen);
 #endif