diff mbox series

[v2] open08: rewrite to newlib

Message ID 787996724f38cfa88d0a8c9a0a713d056ba01307.1528980323.git.jstancek@redhat.com
State Accepted
Headers show
Series [v2] open08: rewrite to newlib | expand

Commit Message

Jan Stancek June 14, 2018, 12:48 p.m. UTC
Fixes: https://github.com/linux-test-project/ltp/issues/330

EACCES testcase changed:
Test now creates file (0600) in tmpdir, that is owned by privileged user,
and unprivileged user tries to open it for writing.

EFAULT testcase changed:
Comments say the intent is to use address outside of process mapped space.
So, test is changed to map/unmap rather than map as PROT_NONE.

uclinux ifdefs dropped.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/open/open08.c | 140 +++++++++++++-------------------
 1 file changed, 58 insertions(+), 82 deletions(-)

Comments

Li Wang June 14, 2018, 1:47 p.m. UTC | #1
On Thu, Jun 14, 2018 at 8:48 PM, Jan Stancek <jstancek@redhat.com> wrote:
> Fixes: https://github.com/linux-test-project/ltp/issues/330
>
> EACCES testcase changed:
> Test now creates file (0600) in tmpdir, that is owned by privileged user,
> and unprivileged user tries to open it for writing.
>
> EFAULT testcase changed:
> Comments say the intent is to use address outside of process mapped space.
> So, test is changed to map/unmap rather than map as PROT_NONE.
>
> uclinux ifdefs dropped.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  testcases/kernel/syscalls/open/open08.c | 140 +++++++++++++-------------------
>  1 file changed, 58 insertions(+), 82 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/open/open08.c b/testcases/kernel/syscalls/open/open08.c
> index d55d8e6bdd53..d749d1e7100e 100644
> --- a/testcases/kernel/syscalls/open/open08.c
> +++ b/testcases/kernel/syscalls/open/open08.c
> @@ -1,5 +1,6 @@
>  /*
>   *   Copyright (c) 2013 Wanlong Gao <gaowanlong@cn.fujitsu.com>
> + *   Copyright (c) 2018 Linux Test Project
>   *
>   *   This program is free software;  you can redistribute it and/or modify
>   *   it under the terms of the GNU General Public License as published by
> @@ -39,7 +40,7 @@
>   *     4. Attempt to open() a filename which is more than VFS_MAXNAMLEN, and
>   *        check for errno to be ENAMETOOLONG.
>   *
> - *     5. Attempt to open a test executable in WRONLY mode,
> + *     5. Attempt to open a (0600) file owned by different user in WRONLY mode,
>   *        open(2) should fail with EACCES.
>   *
>   *     6. Attempt to pass an invalid pathname with an address pointing outside
> @@ -56,113 +57,88 @@
>  #include <fcntl.h>
>  #include <signal.h>
>  #include <pwd.h>
> -#include "test.h"
> -#include "safe_macros.h"
> -
> -static void setup(void);
> -static void cleanup(void);
> -
> -char *TCID = "open08";
> -
> -static char nobody_uid[] = "nobody";
> -static struct passwd *ltpuser;
> -
> -static char *bad_addr;
> +#include "tst_test.h"
>
>  static char filename[40] = "";
> -static char fname[] = "/bin/cat";
> +static char user2_file[] = "user2_0600";
>  static char bad_file[] = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
>
> +struct test_case_t;
> +static void setup_badfname_buf(struct test_case_t *t);
> +
>  static struct test_case_t {
> +       void (*setup)(struct test_case_t *t);

This function pointer named setup make me a little bit confused at first sight.
It makes me thinking about what's the relationship to ltp steup()
function for a while, actually it does not have.

To avoid the situation to other person, how about just using
   void (*func) (struct test_case_t *t);
here?

And my propose change like:

--- a/testcases/kernel/syscalls/open/open08.c
+++ b/testcases/kernel/syscalls/open/open08.c
@@ -64,10 +64,10 @@ static char user2_file[] = "user2_0600";
 static char bad_file[] =
"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghi

 struct test_case_t;
-static void setup_badfname_buf(struct test_case_t *t);
+static void get_badfname_buf(struct test_case_t *t);

 static struct test_case_t {
-       void (*setup)(struct test_case_t *t);
+       void (*func)(struct test_case_t *t);
        char *fname;
        int flags;
        int error;
@@ -77,13 +77,14 @@ static struct test_case_t {
        {NULL, filename, O_DIRECTORY, ENOTDIR},
        {NULL, bad_file, O_RDWR, ENAMETOOLONG},
        {NULL, user2_file, O_WRONLY, EACCES},
-       {setup_badfname_buf, NULL, O_CREAT, EFAULT}
+       {get_badfname_buf, NULL, O_CREAT, EFAULT}
 };

+
 void verify_open(unsigned int i)
 {
-       if (tcases[i].setup)
-               tcases[i].setup(&tcases[i]);
+       if (tcases[i].func)
+               tcases[i].func(&tcases[i]);

        TEST(open(tcases[i].fname, tcases[i].flags,
                S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
@@ -104,7 +105,7 @@ void verify_open(unsigned int i)
        }
 }

-static void setup_badfname_buf(struct test_case_t *t)
+static void get_badfname_buf(struct test_case_t *t)
 {
        char *bad_addr;
        int len = getpagesize();


And, I have to say this is a nice cleanup work. Others looks good to me.
Li Wang June 15, 2018, 3:04 a.m. UTC | #2
On Thu, Jun 14, 2018 at 9:47 PM, Li Wang <liwang@redhat.com> wrote:
> On Thu, Jun 14, 2018 at 8:48 PM, Jan Stancek <jstancek@redhat.com> wrote:
>> Fixes: https://github.com/linux-test-project/ltp/issues/330
>>
>> EACCES testcase changed:
>> Test now creates file (0600) in tmpdir, that is owned by privileged user,
>> and unprivileged user tries to open it for writing.
>>
>> EFAULT testcase changed:
>> Comments say the intent is to use address outside of process mapped space.
>> So, test is changed to map/unmap rather than map as PROT_NONE.
>>
>> uclinux ifdefs dropped.
>>
>> Signed-off-by: Jan Stancek <jstancek@redhat.com>
>> ---
>>  testcases/kernel/syscalls/open/open08.c | 140 +++++++++++++-------------------
>>  1 file changed, 58 insertions(+), 82 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/open/open08.c b/testcases/kernel/syscalls/open/open08.c
>> index d55d8e6bdd53..d749d1e7100e 100644
>> --- a/testcases/kernel/syscalls/open/open08.c
>> +++ b/testcases/kernel/syscalls/open/open08.c
>> @@ -1,5 +1,6 @@
>>  /*
>>   *   Copyright (c) 2013 Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> + *   Copyright (c) 2018 Linux Test Project
>>   *
>>   *   This program is free software;  you can redistribute it and/or modify
>>   *   it under the terms of the GNU General Public License as published by
>> @@ -39,7 +40,7 @@
>>   *     4. Attempt to open() a filename which is more than VFS_MAXNAMLEN, and
>>   *        check for errno to be ENAMETOOLONG.
>>   *
>> - *     5. Attempt to open a test executable in WRONLY mode,
>> + *     5. Attempt to open a (0600) file owned by different user in WRONLY mode,
>>   *        open(2) should fail with EACCES.
>>   *
>>   *     6. Attempt to pass an invalid pathname with an address pointing outside
>> @@ -56,113 +57,88 @@
>>  #include <fcntl.h>
>>  #include <signal.h>
>>  #include <pwd.h>
>> -#include "test.h"
>> -#include "safe_macros.h"
>> -
>> -static void setup(void);
>> -static void cleanup(void);
>> -
>> -char *TCID = "open08";
>> -
>> -static char nobody_uid[] = "nobody";
>> -static struct passwd *ltpuser;
>> -
>> -static char *bad_addr;
>> +#include "tst_test.h"
>>
>>  static char filename[40] = "";
>> -static char fname[] = "/bin/cat";
>> +static char user2_file[] = "user2_0600";
>>  static char bad_file[] = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
>>
>> +struct test_case_t;
>> +static void setup_badfname_buf(struct test_case_t *t);
>> +
>>  static struct test_case_t {
>> +       void (*setup)(struct test_case_t *t);
>
> This function pointer named setup make me a little bit confused at first sight.
> It makes me thinking about what's the relationship to ltp steup()
> function for a while, actually it does not have.
>
> To avoid the situation to other person, how about just using
>    void (*func) (struct test_case_t *t);
> here?

Or, maybe we could drop the func pointer in the test structure, and
reset the badfname only in verify_open(). It's hard to say which
method is better, but seems this can make things more easier I guess.

Which something differ with your patch too:

--- a/testcases/kernel/syscalls/open/open08.c
+++ b/testcases/kernel/syscalls/open/open08.c
@@ -63,27 +63,30 @@ static char filename[40] = "";
 static char user2_file[] = "user2_0600";
 static char bad_file[] =
"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";

-struct test_case_t;
-static void setup_badfname_buf(struct test_case_t *t);
-
 static struct test_case_t {
-    void (*setup)(struct test_case_t *t);
     char *fname;
     int flags;
     int error;
 } tcases[] = {
-    {NULL, filename, O_CREAT | O_EXCL, EEXIST},
-    {NULL, "/tmp", O_RDWR, EISDIR},
-    {NULL, filename, O_DIRECTORY, ENOTDIR},
-    {NULL, bad_file, O_RDWR, ENAMETOOLONG},
-    {NULL, user2_file, O_WRONLY, EACCES},
-    {setup_badfname_buf, NULL, O_CREAT, EFAULT}
+    {filename, O_CREAT | O_EXCL, EEXIST},
+    {"/tmp", O_RDWR, EISDIR},
+    {filename, O_DIRECTORY, ENOTDIR},
+    {bad_file, O_RDWR, ENAMETOOLONG},
+    {user2_file, O_WRONLY, EACCES},
+    {NULL, O_CREAT, EFAULT}
 };

 void verify_open(unsigned int i)
 {
-    if (tcases[i].setup)
-        tcases[i].setup(&tcases[i]);
+    if (i == ARRAY_SIZE(tcases) - 1) {
+        char *bad_addr;
+        int len = getpagesize();
+
+        bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
+                MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+        tcases[i].fname = bad_addr;
+        SAFE_MUNMAP(bad_addr, len);
+    }

     TEST(open(tcases[i].fname, tcases[i].flags,
         S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
@@ -104,17 +107,6 @@ void verify_open(unsigned int i)
     }
 }

-static void setup_badfname_buf(struct test_case_t *t)
-{
-    char *bad_addr;
-    int len = getpagesize();
-
-    bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
-        MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
-    t->fname = bad_addr;
-    SAFE_MUNMAP(bad_addr, len);
-}
-
 static void setup(void)
 {
     int fildes;
Jan Stancek June 15, 2018, 6:11 a.m. UTC | #3
----- Original Message -----
> On Thu, Jun 14, 2018 at 9:47 PM, Li Wang <liwang@redhat.com> wrote:
> > On Thu, Jun 14, 2018 at 8:48 PM, Jan Stancek <jstancek@redhat.com> wrote:
> >> Fixes: https://github.com/linux-test-project/ltp/issues/330
> >>
> >> EACCES testcase changed:
> >> Test now creates file (0600) in tmpdir, that is owned by privileged user,
> >> and unprivileged user tries to open it for writing.
> >>
> >> EFAULT testcase changed:
> >> Comments say the intent is to use address outside of process mapped space.
> >> So, test is changed to map/unmap rather than map as PROT_NONE.
> >>
> >> uclinux ifdefs dropped.
> >>
> >> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> >> ---
> >>  testcases/kernel/syscalls/open/open08.c | 140
> >>  +++++++++++++-------------------
> >>  1 file changed, 58 insertions(+), 82 deletions(-)
> >>
> >> diff --git a/testcases/kernel/syscalls/open/open08.c
> >> b/testcases/kernel/syscalls/open/open08.c
> >> index d55d8e6bdd53..d749d1e7100e 100644
> >> --- a/testcases/kernel/syscalls/open/open08.c
> >> +++ b/testcases/kernel/syscalls/open/open08.c
> >> @@ -1,5 +1,6 @@
> >>  /*
> >>   *   Copyright (c) 2013 Wanlong Gao <gaowanlong@cn.fujitsu.com>
> >> + *   Copyright (c) 2018 Linux Test Project
> >>   *
> >>   *   This program is free software;  you can redistribute it and/or
> >>   modify
> >>   *   it under the terms of the GNU General Public License as published by
> >> @@ -39,7 +40,7 @@
> >>   *     4. Attempt to open() a filename which is more than VFS_MAXNAMLEN,
> >>   and
> >>   *        check for errno to be ENAMETOOLONG.
> >>   *
> >> - *     5. Attempt to open a test executable in WRONLY mode,
> >> + *     5. Attempt to open a (0600) file owned by different user in WRONLY
> >> mode,
> >>   *        open(2) should fail with EACCES.
> >>   *
> >>   *     6. Attempt to pass an invalid pathname with an address pointing
> >>   outside
> >> @@ -56,113 +57,88 @@
> >>  #include <fcntl.h>
> >>  #include <signal.h>
> >>  #include <pwd.h>
> >> -#include "test.h"
> >> -#include "safe_macros.h"
> >> -
> >> -static void setup(void);
> >> -static void cleanup(void);
> >> -
> >> -char *TCID = "open08";
> >> -
> >> -static char nobody_uid[] = "nobody";
> >> -static struct passwd *ltpuser;
> >> -
> >> -static char *bad_addr;
> >> +#include "tst_test.h"
> >>
> >>  static char filename[40] = "";
> >> -static char fname[] = "/bin/cat";
> >> +static char user2_file[] = "user2_0600";
> >>  static char bad_file[] =
> >>  "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
> >>
> >> +struct test_case_t;
> >> +static void setup_badfname_buf(struct test_case_t *t);
> >> +
> >>  static struct test_case_t {
> >> +       void (*setup)(struct test_case_t *t);
> >
> > This function pointer named setup make me a little bit confused at first
> > sight.
> > It makes me thinking about what's the relationship to ltp steup()
> > function for a while, actually it does not have.
> >
> > To avoid the situation to other person, how about just using
> >    void (*func) (struct test_case_t *t);
> > here?
> 
> Or, maybe we could drop the func pointer in the test structure, and
> reset the badfname only in verify_open(). It's hard to say which
> method is better, but seems this can make things more easier I guess.
> 
> Which something differ with your patch too:
> 
> --- a/testcases/kernel/syscalls/open/open08.c
> +++ b/testcases/kernel/syscalls/open/open08.c
> @@ -63,27 +63,30 @@ static char filename[40] = "";
>  static char user2_file[] = "user2_0600";
>  static char bad_file[] =
> "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
> 
> -struct test_case_t;
> -static void setup_badfname_buf(struct test_case_t *t);
> -
>  static struct test_case_t {
> -    void (*setup)(struct test_case_t *t);
>      char *fname;
>      int flags;
>      int error;
>  } tcases[] = {
> -    {NULL, filename, O_CREAT | O_EXCL, EEXIST},
> -    {NULL, "/tmp", O_RDWR, EISDIR},
> -    {NULL, filename, O_DIRECTORY, ENOTDIR},
> -    {NULL, bad_file, O_RDWR, ENAMETOOLONG},
> -    {NULL, user2_file, O_WRONLY, EACCES},
> -    {setup_badfname_buf, NULL, O_CREAT, EFAULT}
> +    {filename, O_CREAT | O_EXCL, EEXIST},
> +    {"/tmp", O_RDWR, EISDIR},
> +    {filename, O_DIRECTORY, ENOTDIR},
> +    {bad_file, O_RDWR, ENAMETOOLONG},
> +    {user2_file, O_WRONLY, EACCES},
> +    {NULL, O_CREAT, EFAULT}
>  };
> 
>  void verify_open(unsigned int i)
>  {
> -    if (tcases[i].setup)
> -        tcases[i].setup(&tcases[i]);
> +    if (i == ARRAY_SIZE(tcases) - 1) {

It's same as hardcoding index to array - if someone adds
new testcase it breaks.

But we could set it to some special value (NULL) and check all cases against that.
If it's NULL, then make a bad_addr.

> +        char *bad_addr;
> +        int len = getpagesize();
> +
> +        bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
> +                MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> +        tcases[i].fname = bad_addr;
> +        SAFE_MUNMAP(bad_addr, len);
> +    }
> 
>      TEST(open(tcases[i].fname, tcases[i].flags,
>          S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
> @@ -104,17 +107,6 @@ void verify_open(unsigned int i)
>      }
>  }
> 
> -static void setup_badfname_buf(struct test_case_t *t)
> -{
> -    char *bad_addr;
> -    int len = getpagesize();
> -
> -    bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
> -        MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> -    t->fname = bad_addr;
> -    SAFE_MUNMAP(bad_addr, len);
> -}
> -
>  static void setup(void)
>  {
>      int fildes;
> 
> 
> 
> --
> Regards,
> Li Wang
>
Li Wang June 15, 2018, 8:16 a.m. UTC | #4
On Fri, Jun 15, 2018 at 2:11 PM, Jan Stancek <jstancek@redhat.com> wrote:
>
>>  void verify_open(unsigned int i)
>>  {
>> -    if (tcases[i].setup)
>> -        tcases[i].setup(&tcases[i]);
>> +    if (i == ARRAY_SIZE(tcases) - 1) {
>
> It's same as hardcoding index to array - if someone adds
> new testcase it breaks.

Yes, to avoid that, new testcase should not be as the last one.

>
> But we could set it to some special value (NULL) and check all cases against that.
> If it's NULL, then make a bad_addr.

Agreed.
Cyril Hrubis June 26, 2018, 1:49 p.m. UTC | #5
Hi!
> > But we could set it to some special value (NULL) and check all cases against that.
> > If it's NULL, then make a bad_addr.
> 
> Agreed.

Either we do that or make the addr pointer to a pointer in the test
structure and make it to point to a global variable initialized in the
test setup as we do in some tests as well.
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/open/open08.c b/testcases/kernel/syscalls/open/open08.c
index d55d8e6bdd53..d749d1e7100e 100644
--- a/testcases/kernel/syscalls/open/open08.c
+++ b/testcases/kernel/syscalls/open/open08.c
@@ -1,5 +1,6 @@ 
 /*
  *   Copyright (c) 2013 Wanlong Gao <gaowanlong@cn.fujitsu.com>
+ *   Copyright (c) 2018 Linux Test Project
  *
  *   This program is free software;  you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -39,7 +40,7 @@ 
  *	4. Attempt to open() a filename which is more than VFS_MAXNAMLEN, and
  *	   check for errno to be ENAMETOOLONG.
  *
- *	5. Attempt to open a test executable in WRONLY mode,
+ *	5. Attempt to open a (0600) file owned by different user in WRONLY mode,
  *	   open(2) should fail with EACCES.
  *
  *	6. Attempt to pass an invalid pathname with an address pointing outside
@@ -56,113 +57,88 @@ 
 #include <fcntl.h>
 #include <signal.h>
 #include <pwd.h>
-#include "test.h"
-#include "safe_macros.h"
-
-static void setup(void);
-static void cleanup(void);
-
-char *TCID = "open08";
-
-static char nobody_uid[] = "nobody";
-static struct passwd *ltpuser;
-
-static char *bad_addr;
+#include "tst_test.h"
 
 static char filename[40] = "";
-static char fname[] = "/bin/cat";
+static char user2_file[] = "user2_0600";
 static char bad_file[] = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
 
+struct test_case_t;
+static void setup_badfname_buf(struct test_case_t *t);
+
 static struct test_case_t {
+	void (*setup)(struct test_case_t *t);
 	char *fname;
 	int flags;
 	int error;
-} TC[] = {
-	{filename, O_CREAT | O_EXCL, EEXIST},
-	{"/tmp", O_RDWR, EISDIR},
-	{filename, O_DIRECTORY, ENOTDIR},
-	{bad_file, O_RDWR, ENAMETOOLONG},
-	{fname, O_WRONLY, EACCES},
-#if !defined(UCLINUX)
-	{(char *)-1, O_CREAT, EFAULT}
-#endif
+} tcases[] = {
+	{NULL, filename, O_CREAT | O_EXCL, EEXIST},
+	{NULL, "/tmp", O_RDWR, EISDIR},
+	{NULL, filename, O_DIRECTORY, ENOTDIR},
+	{NULL, bad_file, O_RDWR, ENAMETOOLONG},
+	{NULL, user2_file, O_WRONLY, EACCES},
+	{setup_badfname_buf, NULL, O_CREAT, EFAULT}
 };
 
-int TST_TOTAL = sizeof(TC) / sizeof(TC[0]);
-
-int main(int ac, char **av)
+void verify_open(unsigned int i)
 {
-	int lc;
-	int i;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		for (i = 0; i < TST_TOTAL; i++) {
-			TEST(open(TC[i].fname, TC[i].flags,
-				  S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
-
-			if (TEST_RETURN != -1) {
-				tst_resm(TFAIL, "call succeeded unexpectedly");
-				continue;
-			}
-
-			if (TEST_ERRNO == TC[i].error) {
-				tst_resm(TPASS, "expected failure - "
-					 "errno = %d : %s", TEST_ERRNO,
-					 strerror(TEST_ERRNO));
-			} else {
-				tst_resm(TFAIL, "unexpected error - %d : %s - "
-					 "expected %d", TEST_ERRNO,
-					 strerror(TEST_ERRNO), TC[i].error);
-			}
-		}
+	if (tcases[i].setup)
+		tcases[i].setup(&tcases[i]);
+
+	TEST(open(tcases[i].fname, tcases[i].flags,
+		S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
+
+	if (TEST_RETURN != -1) {
+		tst_res(TFAIL, "call succeeded unexpectedly");
+		return;
 	}
 
-	cleanup();
-	tst_exit();
+	if (TEST_ERRNO == tcases[i].error) {
+		tst_res(TPASS, "expected failure - "
+				"errno = %d : %s", TEST_ERRNO,
+				strerror(TEST_ERRNO));
+	} else {
+		tst_res(TFAIL, "unexpected error - %d : %s - "
+				"expected %d", TEST_ERRNO,
+				strerror(TEST_ERRNO), tcases[i].error);
+	}
 }
 
-static void setup(void)
+static void setup_badfname_buf(struct test_case_t *t)
 {
-	int fildes;
+	char *bad_addr;
+	int len = getpagesize();
 
-	tst_require_root();
+	bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
+		MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+	t->fname = bad_addr;
+	SAFE_MUNMAP(bad_addr, len);
+}
 
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+static void setup(void)
+{
+	int fildes;
+	char nobody_uid[] = "nobody";
+	struct passwd *ltpuser;
 
 	umask(0);
 
-	TEST_PAUSE;
+	SAFE_CREAT(user2_file, 0600);
 
 	/* Switch to nobody user for correct error code collection */
 	ltpuser = getpwnam(nobody_uid);
-	SAFE_SETGID(NULL, ltpuser->pw_gid);
-	SAFE_SETUID(NULL, ltpuser->pw_uid);
-
-	tst_tmpdir();
+	SAFE_SETGID(ltpuser->pw_gid);
+	SAFE_SETUID(ltpuser->pw_uid);
 
 	sprintf(filename, "open3.%d", getpid());
-
-	fildes = SAFE_CREAT(cleanup, filename, 0600);
-
+	fildes = SAFE_CREAT(filename, 0600);
 	close(fildes);
-
-#if !defined(UCLINUX)
-	bad_addr = mmap(0, 1, PROT_NONE,
-			MAP_PRIVATE_EXCEPT_UCLINUX | MAP_ANONYMOUS, 0, 0);
-	if (bad_addr == MAP_FAILED)
-		tst_brkm(TBROK, cleanup, "mmap failed");
-
-	TC[5].fname = bad_addr;
-#endif
 }
 
-static void cleanup(void)
-{
-	tst_rmdir();
-}
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.setup = setup,
+	.test = verify_open,
+};