[LTP] [PATCH 0/2] Fix for syscalls/utimensat01 test on Ubuntu 4.4 kernel

[LTP] [PATCH 0/2] Fix for syscalls/utimensat01 test on Ubuntu 4.4 kernel

[Po-Hsu Lin]

Ubuntu 4.4 kernel got the backported kernel patch (b3b4283) for
syscalls/utimensat01 since 4.4.0-48.69, therefore it will return
EPERM instead of EACCES.

Without this fix, the test will fail on Ubuntu 4.4 kernel with:
  FAIL: 18: utimensat() failed with incorrect error, expected EACCES: EPERM (1)
  FAIL: 19: utimensat() failed with incorrect error, expected EACCES: EPERM (1)

Also add support to get the distname for Ubuntu by using grep for
^ID=ubuntu from /etc/os-release in tst_kvcmp_distname()

There is no need to know the codename of the release, as we just need
distname UBUTNU and the detailed kernel version for tst_kvercmp2() to
compare the kernel version in syscalls/utimensat01.

Po-Hsu Lin (2):
  lib/tst_kvercmp: Add support to get distname for Ubuntu in
    tst_kvcmp_distname
  syscalls/utimensat01: add exception for Ubuntu 4.4 kernel

 lib/tst_kvercmp.c                                 |  5 +++++
 testcases/kernel/syscalls/utimensat/utimensat01.c | 11 +++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

-- 
2.17.1

[LTP] [PATCH 1/2] lib/tst_kvercmp: Add support to get distname for Ubuntu in tst_kvcmp_distname

[Po-Hsu Lin]

The kver on Ubuntu will be something like these:
* 4.4.0-187-generic
* 5.4.0-1021-kvm
* 4.15.0-1093-azure

So it's better to grep for ^ID=ubuntu in /etc/os-release to determine
the distname, instead of doing this from checking kver substring like
what we did for RHEL / Oracle Linux

Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 lib/tst_kvercmp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/tst_kvercmp.c b/lib/tst_kvercmp.c
index dc3bb669b..349d45543 100644
--- a/lib/tst_kvercmp.c
+++ b/lib/tst_kvercmp.c
@@ -139,6 +139,11 @@ const char *tst_kvcmp_distname(const char *kver)
 	if (strstr(kver, ".el6"))
 		return "RHEL6";
 
+	// Special case for Ubuntu, kernel version cannot reveal the dist_name
+	int rc = WEXITSTATUS(system("grep -q ^ID=ubuntu /etc/os-release 2>/dev/null"));
+	if (rc == 0)
+		return "UBUNTU";
+
 	return NULL;
 }
 
-- 
2.17.1

[LTP] [PATCH 2/2] syscalls/utimensat01: add exception for Ubuntu 4.4 kernel

[Po-Hsu Lin]

Ubuntu 4.4 kernel got this patch (b3b4283) since 4.4.0-48.69,
therefore it will return EPERM instead of EACCES.

Without this fix, the test will fail on Ubuntu 4.4 kernel with:
  FAIL: 18: utimensat() failed with incorrect error, expected EACCES: EPERM (1)
  FAIL: 19: utimensat() failed with incorrect error, expected EACCES: EPERM (1)

Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 testcases/kernel/syscalls/utimensat/utimensat01.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/utimensat/utimensat01.c b/testcases/kernel/syscalls/utimensat/utimensat01.c
index 7dabfed6d..469cb61c5 100644
--- a/testcases/kernel/syscalls/utimensat/utimensat01.c
+++ b/testcases/kernel/syscalls/utimensat/utimensat01.c
@@ -158,6 +158,10 @@ static void tst_multi_set_time(enum tst_ts_type type, struct mytime *mytime)
 
 static void update_error(struct test_case *tc)
 {
+	static struct tst_kern_exv kvers[] = {
+		{ "UBUNTU", "4.4.0-48.69" },
+	};
+
 	if (tc->exp_err != -1)
 		return;
 
@@ -167,9 +171,12 @@ static void update_error(struct test_case *tc)
 	 * This patch has also been merged to stable 4.4 with
 	 * b3b4283 ("vfs: move permission checking into notify_change() for utimes(NULL)")
 	 */
-	if (tst_kvercmp(4, 4, 27) < 0)
+	if (tst_kvercmp(4, 4, 27) < 0) {
 		tc->exp_err = EACCES;
-	else
+		// Special case for Ubuntu kernel, which got this patch since 4.4.0-48.69
+		if (tst_kvercmp2(4, 4, 0, kvers))
+			tc->exp_err = EPERM;
+	} else
 		tc->exp_err = EPERM;
 }
 
-- 
2.17.1

[LTP] [PATCH 1/2] lib/tst_kvercmp: Add support to get distname for Ubuntu in tst_kvcmp_distname

[Cyril Hrubis]

Hi!
> * 4.4.0-187-generic
> * 5.4.0-1021-kvm
> * 4.15.0-1093-azure
> 
> So it's better to grep for ^ID=ubuntu in /etc/os-release to determine
> the distname, instead of doing this from checking kver substring like
> what we did for RHEL / Oracle Linux
> 
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> ---
>  lib/tst_kvercmp.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/tst_kvercmp.c b/lib/tst_kvercmp.c
> index dc3bb669b..349d45543 100644
> --- a/lib/tst_kvercmp.c
> +++ b/lib/tst_kvercmp.c
> @@ -139,6 +139,11 @@ const char *tst_kvcmp_distname(const char *kver)
>  	if (strstr(kver, ".el6"))
>  		return "RHEL6";
>  
> +	// Special case for Ubuntu, kernel version cannot reveal the dist_name
> +	int rc = WEXITSTATUS(system("grep -q ^ID=ubuntu /etc/os-release 2>/dev/null"));
> +	if (rc == 0)
> +		return "UBUNTU";

Can we please properly parse the /etc/os-release file insetad of this
hackery?

It should be as easy as one SAFE_FILE_SCANF() in the case that the file
exists.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] syscalls/utimensat01: add exception for Ubuntu 4.4 kernel

[Cyril Hrubis]

Hi!
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> ---
>  testcases/kernel/syscalls/utimensat/utimensat01.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/utimensat/utimensat01.c b/testcases/kernel/syscalls/utimensat/utimensat01.c
> index 7dabfed6d..469cb61c5 100644
> --- a/testcases/kernel/syscalls/utimensat/utimensat01.c
> +++ b/testcases/kernel/syscalls/utimensat/utimensat01.c
> @@ -158,6 +158,10 @@ static void tst_multi_set_time(enum tst_ts_type type, struct mytime *mytime)
>  
>  static void update_error(struct test_case *tc)
>  {
> +	static struct tst_kern_exv kvers[] = {
> +		{ "UBUNTU", "4.4.0-48.69" },
> +	};
> +
>  	if (tc->exp_err != -1)
>  		return;
>  
> @@ -167,9 +171,12 @@ static void update_error(struct test_case *tc)
>  	 * This patch has also been merged to stable 4.4 with
>  	 * b3b4283 ("vfs: move permission checking into notify_change() for utimes(NULL)")
>  	 */
> -	if (tst_kvercmp(4, 4, 27) < 0)
> +	if (tst_kvercmp(4, 4, 27) < 0) {
>  		tc->exp_err = EACCES;
> -	else
> +		// Special case for Ubuntu kernel, which got this patch since 4.4.0-48.69
> +		if (tst_kvercmp2(4, 4, 0, kvers))
> +			tc->exp_err = EPERM;
> +	} else
>  		tc->exp_err = EPERM;

This whole thing looks broken, this is not how the tst_kvercmp2() is
supposed to work. The generic kernel version is supposed to be passed in
the first parameters and the kvers overrides that option.

So this should be something like:

	if (tst_kvercmp2(4, 4, 27, kvers) < 0)
		tc->exp_err = EACCESS;
	else
		tc->exp_err = EPERM;


And in a case that the distro matches to UBUNTU the kernel version is
compared against the "4.4.0-48.69" instead of the generic one.

>  }
>  
> -- 
> 2.17.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/2] lib/tst_kvercmp: Add support to get distname for Ubuntu in tst_kvcmp_distname

[Cyril Hrubis]

Hi!
> It should be as easy as one SAFE_FILE_SCANF() in the case that the file
> exists.

It's SAFE_FILE_LINES_SCANF()

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/2] lib/tst_kvercmp: Add support to get distname for Ubuntu in tst_kvcmp_distname

[Po-Hsu Lin]

On Mon, Aug 17, 2020 at 8:44 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > It should be as easy as one SAFE_FILE_SCANF() in the case that the file
> > exists.
>
> It's SAFE_FILE_LINES_SCANF()

Hello Cyril,
thanks for your suggestion, I will submit V2 for this.

>
> --
> Cyril Hrubis
> chrubis@suse.cz

[LTP] [PATCH 2/2] syscalls/utimensat01: add exception for Ubuntu 4.4 kernel

[Po-Hsu Lin]

On Mon, Aug 17, 2020 at 8:41 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > ---
> >  testcases/kernel/syscalls/utimensat/utimensat01.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/utimensat/utimensat01.c b/testcases/kernel/syscalls/utimensat/utimensat01.c
> > index 7dabfed6d..469cb61c5 100644
> > --- a/testcases/kernel/syscalls/utimensat/utimensat01.c
> > +++ b/testcases/kernel/syscalls/utimensat/utimensat01.c
> > @@ -158,6 +158,10 @@ static void tst_multi_set_time(enum tst_ts_type type, struct mytime *mytime)
> >
> >  static void update_error(struct test_case *tc)
> >  {
> > +     static struct tst_kern_exv kvers[] = {
> > +             { "UBUNTU", "4.4.0-48.69" },
> > +     };
> > +
> >       if (tc->exp_err != -1)
> >               return;
> >
> > @@ -167,9 +171,12 @@ static void update_error(struct test_case *tc)
> >        * This patch has also been merged to stable 4.4 with
> >        * b3b4283 ("vfs: move permission checking into notify_change() for utimes(NULL)")
> >        */
> > -     if (tst_kvercmp(4, 4, 27) < 0)
> > +     if (tst_kvercmp(4, 4, 27) < 0) {
> >               tc->exp_err = EACCES;
> > -     else
> > +             // Special case for Ubuntu kernel, which got this patch since 4.4.0-48.69
> > +             if (tst_kvercmp2(4, 4, 0, kvers))
> > +                     tc->exp_err = EPERM;
> > +     } else
> >               tc->exp_err = EPERM;
>
> This whole thing looks broken, this is not how the tst_kvercmp2() is
> supposed to work. The generic kernel version is supposed to be passed in
> the first parameters and the kvers overrides that option.
>
> So this should be something like:
>
>         if (tst_kvercmp2(4, 4, 27, kvers) < 0)
>                 tc->exp_err = EACCESS;
>         else
>                 tc->exp_err = EPERM;
>
>
> And in a case that the distro matches to UBUNTU the kernel version is
> compared against the "4.4.0-48.69" instead of the generic one.
>
Ah!
I think I can update the doc/test-writing-guidelines.txt with this later.

I will send V2 for this patch.
Thanks for your detailed explanation.

> >  }
> >
> > --
> > 2.17.1
> >
>
> --
> Cyril Hrubis
> chrubis@suse.cz