| Message ID | b987a73550937e5d5652e4a86e591d72334d8fda.1635244875.git.jstancek@redhat.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [v2] finit_module02: fix exp. errno for O_WRONLY testcase | expand |
Hi! > commit 032146cda855 ("vfs: check fd has read access in > kernel_read_file_from_fd()") changed errno back to EBADF, > which is correct value according to man page. Drop the > workaround and always expect EBADF for O_WRONLY testcase. Reviewed-by: Cyril Hrubis <chrubis@suse.cz> Also I'm starting to wonder if the errno from dir_setup() should be fixed in the kernel as well. I guess that EISDIR sounds much better error than EINVAL. But in this case the manual page seems to be silent on what is going to happen if you pass a directory fd to the finit_module().
On Tue, Oct 26, 2021 at 12:52 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > commit 032146cda855 ("vfs: check fd has read access in > > kernel_read_file_from_fd()") changed errno back to EBADF, > > which is correct value according to man page. Drop the > > workaround and always expect EBADF for O_WRONLY testcase. > > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> Pushed. > > Also I'm starting to wonder if the errno from dir_setup() should be > fixed in the kernel as well. I guess that EISDIR sounds much better > error than EINVAL. It does. > But in this case the manual page seems to be silent > on what is going to happen if you pass a directory fd to the > finit_module(). > > -- > Cyril Hrubis > chrubis@suse.cz >
Hi! > > Also I'm starting to wonder if the errno from dir_setup() should be > > fixed in the kernel as well. I guess that EISDIR sounds much better > > error than EINVAL. > > It does. Will you send a kernel patch or should I do it?
On Tue, Oct 26, 2021 at 5:07 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > > Also I'm starting to wonder if the errno from dir_setup() should be > > > fixed in the kernel as well. I guess that EISDIR sounds much better > > > error than EINVAL. > > > > It does. > > Will you send a kernel patch or should I do it? Since you found it, I'll leave it to you :-). On second thought, EBADF might be better, since it covers other fd types as well. > > -- > Cyril Hrubis > chrubis@suse.cz >
diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c index 0d2bf917ea64..47b5edbfb527 100644 --- a/testcases/kernel/syscalls/finit_module/finit_module02.c +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c @@ -50,14 +50,6 @@ static void bad_fd_setup(struct tcase *tc) tc->exp_errno = EBADF; } -static void wo_file_setup(struct tcase *tc) -{ - if (tst_kvercmp(4, 6, 0) < 0) - tc->exp_errno = EBADF; - else - tc->exp_errno = ETXTBSY; -} - static void dir_setup(struct tcase *tc) { if (tst_kvercmp(4, 6, 0) < 0) @@ -78,8 +70,8 @@ static struct tcase tcases[] = { {"no-perm", &fd, "", O_RDONLY | O_CLOEXEC, 0, 1, EPERM, 0, NULL}, {"module-exists", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EEXIST, 1, NULL}, - {"file-not-readable", &fd, "", O_WRONLY | O_CLOEXEC, 0, 0, 0, 0, - wo_file_setup}, + {"file-not-readable", &fd, "", O_WRONLY | O_CLOEXEC, 0, 0, EBADF, 0, + NULL}, {"directory", &fd_dir, "", O_RDONLY | O_CLOEXEC, 0, 0, 0, 0, dir_setup}, }; @@ -140,6 +132,10 @@ static void run(unsigned int n) } static struct tst_test test = { + .tags = (const struct tst_tag[]) { + {"linux-git", "032146cda855"}, + {} + }, .test = run, .tcnt = ARRAY_SIZE(tcases), .setup = setup,
commit 032146cda855 ("vfs: check fd has read access in kernel_read_file_from_fd()") changed errno back to EBADF, which is correct value according to man page. Drop the workaround and always expect EBADF for O_WRONLY testcase. Signed-off-by: Jan Stancek <jstancek@redhat.com> --- .../syscalls/finit_module/finit_module02.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)