public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] Add regression test for CVE-2017-16939
Date: Fri, 9 Mar 2018 11:50:06 +0100	[thread overview]
Message-ID: <20180309105006.GA24656@rei> (raw)
In-Reply-To: <20180309103310.qx3oac43twii7wvh@linux.suse.de>

Hi!
> > Also we should probably allocate these once in the test setup so that we
> > do not waste memory when the test is executed with the -i option.
> I was trying to keep as close as possible to the reproducer. I can check if
> this can be moved to setup, and maybe only initialize once. However, I think
> it is not a big waste of memory, after all the test does not loop or fork 
> or anything like this.

Note that all testcases can take -i or -I parameter which causes that
the test() function is called in a loop. That is the reason why we
should care for allocating memory out of the test() function, or at
least freeing it at the end of it.

> > > +	pid = SAFE_FORK();
> > > +	if (pid == 0) {
> > > +		do_run();
> > > +	} else {
> > > +		usleep(250000);
> >                 ^
> > 		Why the usleep here? Should just the waitpid() below
> > 		suffice in waiting for the child to exit?
> > 
> Well, this is something asynchronous happening in the kernel. When we don't 
> wait for a while, the crash happens way after we reported a PASS.. but maybe
> the usleep makes more sense after the waitpid(). 

Well that's a good question, the problem is that if we add the usleep to
the test it will slow down the testsuite in all cases, which is bad as
the longer the testsuite takes the less useful it is. But if we don't
add it will be a bit more complicated to to figure out which test caused
the crash.

But at least the usleep() requires proper comment about why it's there,
something as "the corruption usually manifests within a short time frame
hence we sleep here for a while" or something like that.

Or we may try to help it by running the actuall test in a loop for
several iterations instead of idling which may increase the probability
of failing earlier.

-- 
Cyril Hrubis
chrubis@suse.cz

  reply	other threads:[~2018-03-09 10:50 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-06 14:03 [LTP] [PATCH] Add regression test for CVE-2017-16939 Michael Moese
2018-03-07 17:05 ` Cyril Hrubis
2018-03-09 10:33   ` Michael Moese
2018-03-09 10:50     ` Cyril Hrubis [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-02-14 14:12 Michael Moese

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180309105006.GA24656@rei \
    --to=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox