| Message ID | 20240207152155.10146-1-andrea.cervesato@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] Add mkdir10 test | expand |
Hi Andrea, I was thinking if this is filesystem specific (struct inode_operations mkdir member is in fs/*/*.c - all filesystems), but it looks to me that code which checks for EEXIST is in may_create() in fs/namei.c, which is VFS. Therefore there is really no point to use .all_filesystems = 1. LGTM, just few nits below. Reviewed-by: Petr Vorel <pvorel@suse.cz> > From: Andrea Cervesato <andrea.cervesato@suse.com> > This test has been extracted from symlink01 and it verifies that > mkdir() can't overwrite certain types of files, such as simlinks, ^ symlinks > directories, pipes, devices, etc. > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> > --- > runtest/syscalls | 2 +- > testcases/kernel/syscalls/mkdir/.gitignore | 1 + > testcases/kernel/syscalls/mkdir/mkdir10.c | 57 ++++++++++++++++++++++ > 3 files changed, 59 insertions(+), 1 deletion(-) > create mode 100644 testcases/kernel/syscalls/mkdir/mkdir10.c > diff --git a/runtest/syscalls b/runtest/syscalls > index 2af7ade9c..7f4edb901 100644 > --- a/runtest/syscalls > +++ b/runtest/syscalls > @@ -757,8 +757,8 @@ mkdir02 mkdir02 > mkdir03 mkdir03 > mkdir04 mkdir04 > mkdir05 mkdir05 > -mkdir05A symlink01 -T mkdir05 This is not the case, but beware runtest/smoketest also use some of symlink tests you recently rewrite. > mkdir09 mkdir09 > +mkdir10 mkdir10 ... > --- /dev/null > +++ b/testcases/kernel/syscalls/mkdir/mkdir10.c > @@ -0,0 +1,57 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2000 Silicon Graphics, Inc. All Rights Reserved. > + * Author: David Fenner > + * Copilot: Jon Hendrickson very nit: Authors: David Fenner, Jon Hendrickson > + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com > + */ > + > +/*\ > + * [Description] > + * > + * This test verifies that mkdir() can't overwrite certain generated files, such > + * as symlinks, pipes, devices, folders, etc. > + */ > + > +#include "tst_test.h" > + > +#define FILE_FOLDER "myfolder" > +#define FILE_MYFILE "myfile" > +#define FILE_FIFO "mypipe" > +#define FILE_SYMLINK "mylink" > +#define FILE_DEVICE "/dev/null" nit: _PATH_DEVNULL from <paths.h> defines "/dev/null". > + > +struct tcase { > + char *file; > + char *msg; > +}; > + > +static struct tcase tcases[] = { > + {FILE_FOLDER, "folder already exists"}, > + {FILE_MYFILE, "file already exists"}, > + {FILE_FIFO, "fifo already exists"}, > + {FILE_SYMLINK, "symlink already exists"}, > + {FILE_DEVICE, "device already exists"}, very nit: "folder", "file", ... TST_EXP_FAIL(mkdir(tc->file, 0777), EEXIST, "%s already exists", tc->msg); Kind regards, Petr
Hi! This test can be actually be merged with mkdir01, so I will send an another patch for this use case. Please ignore the patch. Andrea On 2/9/24 23:41, Petr Vorel wrote: > Hi Andrea, > > I was thinking if this is filesystem specific (struct inode_operations mkdir > member is in fs/*/*.c - all filesystems), but it looks to me that code which > checks for EEXIST is in may_create() in fs/namei.c, which is VFS. Therefore > there is really no point to use .all_filesystems = 1. > > LGTM, just few nits below. > Reviewed-by: Petr Vorel <pvorel@suse.cz> > >> From: Andrea Cervesato <andrea.cervesato@suse.com> >> This test has been extracted from symlink01 and it verifies that >> mkdir() can't overwrite certain types of files, such as simlinks, > ^ symlinks > >> directories, pipes, devices, etc. >> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> >> --- >> runtest/syscalls | 2 +- >> testcases/kernel/syscalls/mkdir/.gitignore | 1 + >> testcases/kernel/syscalls/mkdir/mkdir10.c | 57 ++++++++++++++++++++++ >> 3 files changed, 59 insertions(+), 1 deletion(-) >> create mode 100644 testcases/kernel/syscalls/mkdir/mkdir10.c >> diff --git a/runtest/syscalls b/runtest/syscalls >> index 2af7ade9c..7f4edb901 100644 >> --- a/runtest/syscalls >> +++ b/runtest/syscalls >> @@ -757,8 +757,8 @@ mkdir02 mkdir02 >> mkdir03 mkdir03 >> mkdir04 mkdir04 >> mkdir05 mkdir05 >> -mkdir05A symlink01 -T mkdir05 > This is not the case, but beware runtest/smoketest also use some of symlink > tests you recently rewrite. > >> mkdir09 mkdir09 >> +mkdir10 mkdir10 > ... >> --- /dev/null >> +++ b/testcases/kernel/syscalls/mkdir/mkdir10.c >> @@ -0,0 +1,57 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (c) 2000 Silicon Graphics, Inc. All Rights Reserved. >> + * Author: David Fenner >> + * Copilot: Jon Hendrickson > very nit: > Authors: David Fenner, Jon Hendrickson > >> + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com >> + */ >> + >> +/*\ >> + * [Description] >> + * >> + * This test verifies that mkdir() can't overwrite certain generated files, such >> + * as symlinks, pipes, devices, folders, etc. >> + */ >> + >> +#include "tst_test.h" >> + >> +#define FILE_FOLDER "myfolder" >> +#define FILE_MYFILE "myfile" >> +#define FILE_FIFO "mypipe" >> +#define FILE_SYMLINK "mylink" >> +#define FILE_DEVICE "/dev/null" > nit: _PATH_DEVNULL from <paths.h> defines "/dev/null". >> + >> +struct tcase { >> + char *file; >> + char *msg; >> +}; >> + >> +static struct tcase tcases[] = { >> + {FILE_FOLDER, "folder already exists"}, >> + {FILE_MYFILE, "file already exists"}, >> + {FILE_FIFO, "fifo already exists"}, >> + {FILE_SYMLINK, "symlink already exists"}, >> + {FILE_DEVICE, "device already exists"}, > very nit: "folder", "file", ... > > TST_EXP_FAIL(mkdir(tc->file, 0777), EEXIST, "%s already exists", tc->msg); > > Kind regards, > Petr
diff --git a/runtest/syscalls b/runtest/syscalls index 2af7ade9c..7f4edb901 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -757,8 +757,8 @@ mkdir02 mkdir02 mkdir03 mkdir03 mkdir04 mkdir04 mkdir05 mkdir05 -mkdir05A symlink01 -T mkdir05 mkdir09 mkdir09 +mkdir10 mkdir10 #mkdirat test cases mkdirat01 mkdirat01 diff --git a/testcases/kernel/syscalls/mkdir/.gitignore b/testcases/kernel/syscalls/mkdir/.gitignore index 880ff50c0..416fbbd7c 100644 --- a/testcases/kernel/syscalls/mkdir/.gitignore +++ b/testcases/kernel/syscalls/mkdir/.gitignore @@ -3,3 +3,4 @@ /mkdir04 /mkdir05 /mkdir09 +/mkdir10 diff --git a/testcases/kernel/syscalls/mkdir/mkdir10.c b/testcases/kernel/syscalls/mkdir/mkdir10.c new file mode 100644 index 000000000..87f419916 --- /dev/null +++ b/testcases/kernel/syscalls/mkdir/mkdir10.c @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2000 Silicon Graphics, Inc. All Rights Reserved. + * Author: David Fenner + * Copilot: Jon Hendrickson + * Copyright (C) 2024 Andrea Cervesato andrea.cervesato@suse.com + */ + +/*\ + * [Description] + * + * This test verifies that mkdir() can't overwrite certain generated files, such + * as symlinks, pipes, devices, folders, etc. + */ + +#include "tst_test.h" + +#define FILE_FOLDER "myfolder" +#define FILE_MYFILE "myfile" +#define FILE_FIFO "mypipe" +#define FILE_SYMLINK "mylink" +#define FILE_DEVICE "/dev/null" + +struct tcase { + char *file; + char *msg; +}; + +static struct tcase tcases[] = { + {FILE_FOLDER, "folder already exists"}, + {FILE_MYFILE, "file already exists"}, + {FILE_FIFO, "fifo already exists"}, + {FILE_SYMLINK, "symlink already exists"}, + {FILE_DEVICE, "device already exists"}, +}; + +static void run(unsigned int i) +{ + struct tcase *tc = &tcases[i]; + + TST_EXP_FAIL(mkdir(tc->file, 0777), EEXIST, "%s", tc->msg); +} + +static void setup(void) +{ + SAFE_SYMLINK(tst_get_tmpdir(), FILE_SYMLINK); + SAFE_MKFIFO(FILE_FIFO, 0777); + SAFE_MKDIR(FILE_FOLDER, 0777); + SAFE_TOUCH(FILE_MYFILE, 0777, NULL); +} + +static struct tst_test test = { + .test = run, + .tcnt = ARRAY_SIZE(tcases), + .setup = setup, + .needs_tmpdir = 1, +};