[LTP] [PATCH] finit_module02: fix exp. errno for O_WRONLY testcase

[LTP] [PATCH] finit_module02: fix exp. errno for O_WRONLY testcase

[Jan Stancek]

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, so tweak
the test to expect it for kernels >= 5.15.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/finit_module/finit_module02.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
index 0d2bf917ea64..9b5d3563b74e 100644
--- a/testcases/kernel/syscalls/finit_module/finit_module02.c
+++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
@@ -52,7 +52,7 @@ static void bad_fd_setup(struct tcase *tc)
 
 static void wo_file_setup(struct tcase *tc)
 {
-	if (tst_kvercmp(4, 6, 0) < 0)
+	if (tst_kvercmp(4, 6, 0) < 0 || tst_kvercmp(5, 15, 0) >= 0)
 		tc->exp_errno = EBADF;
 	else
 		tc->exp_errno = ETXTBSY;
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] finit_module02: fix exp. errno for O_WRONLY testcase

[Cyril Hrubis]

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, so tweak
> the test to expect it for kernels >= 5.15.

Shouldn't we drop the check completely and rather than working around
the bug add this commit to the test metadata?

> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  testcases/kernel/syscalls/finit_module/finit_module02.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
> index 0d2bf917ea64..9b5d3563b74e 100644
> --- a/testcases/kernel/syscalls/finit_module/finit_module02.c
> +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
> @@ -52,7 +52,7 @@ static void bad_fd_setup(struct tcase *tc)
>  
>  static void wo_file_setup(struct tcase *tc)
>  {
> -	if (tst_kvercmp(4, 6, 0) < 0)
> +	if (tst_kvercmp(4, 6, 0) < 0 || tst_kvercmp(5, 15, 0) >= 0)
>  		tc->exp_errno = EBADF;
>  	else
>  		tc->exp_errno = ETXTBSY;
> -- 
> 2.27.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] finit_module02: fix exp. errno for O_WRONLY testcase

[Jan Stancek]

On Tue, Oct 26, 2021 at 10:03 AM 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, so tweak
> > the test to expect it for kernels >= 5.15.
>
> Shouldn't we drop the check completely and rather than working around
> the bug add this commit to the test metadata?

Sure, I'll post v2.

>
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > ---
> >  testcases/kernel/syscalls/finit_module/finit_module02.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
> > index 0d2bf917ea64..9b5d3563b74e 100644
> > --- a/testcases/kernel/syscalls/finit_module/finit_module02.c
> > +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
> > @@ -52,7 +52,7 @@ static void bad_fd_setup(struct tcase *tc)
> >
> >  static void wo_file_setup(struct tcase *tc)
> >  {
> > -     if (tst_kvercmp(4, 6, 0) < 0)
> > +     if (tst_kvercmp(4, 6, 0) < 0 || tst_kvercmp(5, 15, 0) >= 0)
> >               tc->exp_errno = EBADF;
> >       else
> >               tc->exp_errno = ETXTBSY;
> > --
> > 2.27.0
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] finit_module02: fix exp. errno for O_WRONLY testcase

[Jan Stancek]

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(-)

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,
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] finit_module02: fix exp. errno for O_WRONLY testcase

[Cyril Hrubis]

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().

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] finit_module02: fix exp. errno for O_WRONLY testcase

[Jan Stancek]

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
>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] finit_module02: fix exp. errno for O_WRONLY testcase

[Cyril Hrubis]

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?

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] finit_module02: fix exp. errno for O_WRONLY testcase

[Jan Stancek]

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
>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] finit_module02: fix exp. errno for O_WRONLY testcase

[xuyang2018.jy]

Hi Jan
> 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, so tweak
> the test to expect it for kernels>= 5.15.
It seems we still can hit ETXTBSY error if we  use RDWR fd on kernels >= 
5.15.

This error existed since the following commit because of 
deny_write_access api
commit 39d637af5aa7577f655c58b9e55587566c63a0af
Author: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
Date:   Sun Oct 26 12:42:07 2014 +0200

     vfs: forbid write access when reading a file into memory

I suggest add a RDWR test and send a patch to record a ETXTBSY error( 
Since linux 4.7).

What do you think about this?

Best Regards
Yang Xu
>
> Signed-off-by: Jan Stancek<jstancek@redhat.com>
> ---
>   testcases/kernel/syscalls/finit_module/finit_module02.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
> index 0d2bf917ea64..9b5d3563b74e 100644
> --- a/testcases/kernel/syscalls/finit_module/finit_module02.c
> +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
> @@ -52,7 +52,7 @@ static void bad_fd_setup(struct tcase *tc)
>
>   static void wo_file_setup(struct tcase *tc)
>   {
> -	if (tst_kvercmp(4, 6, 0)<  0)
> +	if (tst_kvercmp(4, 6, 0)<  0 || tst_kvercmp(5, 15, 0)>= 0)
>   		tc->exp_errno = EBADF;
>   	else
>   		tc->exp_errno = ETXTBSY;

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] finit_module02: fix exp. errno for O_WRONLY testcase

[Jan Stancek]

On Wed, Nov 17, 2021 at 8:54 AM xuyang2018.jy@fujitsu.com
<xuyang2018.jy@fujitsu.com> wrote:
>
> Hi Jan
> > 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, so tweak
> > the test to expect it for kernels>= 5.15.
> It seems we still can hit ETXTBSY error if we  use RDWR fd on kernels >=
> 5.15.
>
> This error existed since the following commit because of
> deny_write_access api
> commit 39d637af5aa7577f655c58b9e55587566c63a0af
> Author: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
> Date:   Sun Oct 26 12:42:07 2014 +0200
>
>      vfs: forbid write access when reading a file into memory
>
> I suggest add a RDWR test and send a patch to record a ETXTBSY error(
> Since linux 4.7).
>
> What do you think about this?

(Sorry for delay, I'm catching up with email after holidays)
Since it's also not documented, I'd wait for Cyril's patch first. But
we can always change it later (I see patch was already merged).


>
> Best Regards
> Yang Xu
> >
> > Signed-off-by: Jan Stancek<jstancek@redhat.com>
> > ---
> >   testcases/kernel/syscalls/finit_module/finit_module02.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
> > index 0d2bf917ea64..9b5d3563b74e 100644
> > --- a/testcases/kernel/syscalls/finit_module/finit_module02.c
> > +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
> > @@ -52,7 +52,7 @@ static void bad_fd_setup(struct tcase *tc)
> >
> >   static void wo_file_setup(struct tcase *tc)
> >   {
> > -     if (tst_kvercmp(4, 6, 0)<  0)
> > +     if (tst_kvercmp(4, 6, 0)<  0 || tst_kvercmp(5, 15, 0)>= 0)
> >               tc->exp_errno = EBADF;
> >       else
> >               tc->exp_errno = ETXTBSY;


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] finit_module02: fix exp. errno for O_WRONLY testcase

[xuyang2018.jy]

Hi Jan
> On Wed, Nov 17, 2021 at 8:54 AM xuyang2018.jy@fujitsu.com
> <xuyang2018.jy@fujitsu.com>  wrote:
>>
>> Hi Jan
>>> 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, so tweak
>>> the test to expect it for kernels>= 5.15.
>> It seems we still can hit ETXTBSY error if we  use RDWR fd on kernels>=
>> 5.15.
>>
>> This error existed since the following commit because of
>> deny_write_access api
>> commit 39d637af5aa7577f655c58b9e55587566c63a0af
>> Author: Dmitry Kasatkin<dmitry.kasatkin@huawei.com>
>> Date:   Sun Oct 26 12:42:07 2014 +0200
>>
>>       vfs: forbid write access when reading a file into memory
>>
>> I suggest add a RDWR test and send a patch to record a ETXTBSY error(
>> Since linux 4.7).
>>
>> What do you think about this?
>
> (Sorry for delay, I'm catching up with email after holidays)
> Since it's also not documented, I'd wait for Cyril's patch first. But
> we can always change it later (I see patch was already merged).
Yes, I have merged my patch with Richard's review.
Also, I  have documented this error by sending a patch[1](still in 
review) to man-pages community.
[1]https://lore.kernel.org/linux-man/1637136967-13028-1-git-send-email-xuyang2018.jy@fujitsu.com/T/#u

Best Regards
Yang Xu
>
>
>>
>> Best Regards
>> Yang Xu
>>>
>>> Signed-off-by: Jan Stancek<jstancek@redhat.com>
>>> ---
>>>    testcases/kernel/syscalls/finit_module/finit_module02.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
>>> index 0d2bf917ea64..9b5d3563b74e 100644
>>> --- a/testcases/kernel/syscalls/finit_module/finit_module02.c
>>> +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
>>> @@ -52,7 +52,7 @@ static void bad_fd_setup(struct tcase *tc)
>>>
>>>    static void wo_file_setup(struct tcase *tc)
>>>    {
>>> -     if (tst_kvercmp(4, 6, 0)<   0)
>>> +     if (tst_kvercmp(4, 6, 0)<   0 || tst_kvercmp(5, 15, 0)>= 0)
>>>                tc->exp_errno = EBADF;
>>>        else
>>>                tc->exp_errno = ETXTBSY;
>

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp