From mboxrd@z Thu Jan  1 00:00:00 1970
From: Richard Palethorpe <rpalethorpe@suse.com>
Date: Mon, 20 Mar 2017 15:42:19 +0100
Subject: [LTP] [RFC 1/1] Test for vulnerability cve-2016-7117 in
 recvmmsg error return path
In-Reply-To: <20170320112338.GB3322@rei.lan>
References: <20170317103911.1e9e0e25@linux-v3j5>
 <20170320112338.GB3322@rei.lan>
Message-ID: <20170320154219.54b4fed9@linux-v3j5>
List-Id: <ltp.lists.linux.it>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: ltp@lists.linux.it

Hello Metan,

On Mon, 20 Mar 2017 12:23:38 +0100
"Cyril Hrubis" <chrubis@suse.cz> wrote:

> Hi!
> > --- /dev/null
> > +++ b/testcases/cve/2016-7117/cve-2016-7117.c  
> 
> Hmm, I would have just put this test directly into the cve/ directory,
> there is no point in having one directory per test here.
>

Good I like having a more flat hierarchy. I think it would be nice just to
have all the tests directly relating to CVEs in one big dir, although the
Makefile will be bigger than usual... when I have done a few more tests we
shall see what it looks like.

> > +// The bug was present in the kernel before recvmmsg was exposed by glibc
> > +#ifndef __NR_recvmmsg
> > +#ifdef __i386__
> > +#define __NR_recvmmsg 337
> > +#elif defined(__x86_64__)
> > +#define __NR_recvmmsg 299
> > +#endif
> > +#endif  
> 
> We have these for all architectures in autogenerated
> linux_syscall_numbers.h, just include that header instead of rolling
> your own definitions.

Nice.
> > +char rbuf[sizeof(MSG)] = {0};  
> 
> There is no need to initialize global variables to 0. Also global
> variables should be declared static.

Yep, sorry.

> > +
> > +void *send_and_close(void *arg)
> > +{
> > +	struct timespec *delay = (struct timespec *)arg;
> > +
> > +	send(socket_fds[0], MSG, sizeof(MSG), 0);
> > +	send(socket_fds[0], MSG, sizeof(MSG), 0);
> > +
> > +	nanosleep(delay, 0);
> > +
> > +	close(socket_fds[0]);
> > +	close(socket_fds[1]);
> > +	close_exit = exit_time();  
> 
> More usuall way of passing structures in C is by pointer, if you just
> did exit_time(&close_exit) here you could just pass the pointer to
> clock_gettime() call instead of copying the value on the stack...

Yep, I didn't think that through properly.

> 
> Otherwise the code looks fine.
> 
> It's missing Makefile, runtest file and .gitignore record though...
> 

Thank you,
Richard.