[LTP] [PATCH v3 0/2] LTP: adjust socket xattr tests for kernel 7.1.0+

[LTP] [PATCH v3 0/2] LTP: adjust socket xattr tests for kernel 7.1.0+

[Jan Polensky]

Linux 7.1.0 allows user.* xattrs on sockets (dc0876b9846d). Update
fsetxattr02/setxattr02 to expect success on 7.1.0+ (instead of -EPERM).

Changes since v2:
- Move kernel check into setup() & add helper (thanks Petr) 
- Alter test description
Changes since v1:
- Fix kernel version 7.0+ -> 7.1.0+ (thanks Petr)
- Fix comments

Jan Polensky (2):
  fsetxattr02: Adapt test for kernel 7.1.0+ socket xattr support
  setxattr02: Adapt test for kernel 7.1.0+ socket xattr support

 .../kernel/syscalls/fsetxattr/fsetxattr02.c   | 22 ++++++++++++++++++-
 .../kernel/syscalls/setxattr/setxattr02.c     | 20 ++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)

-- 
2.53.0


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

[LTP] [PATCH v3 1/2] fsetxattr02: Adapt test for kernel 7.1.0+ socket xattr support

[Jan Polensky]

Starting with kernel 7.1.0, sockets support extended attributes in the
user.* namespace. This behavior was enabled by dc0876b9846d ("xattr: support
extended attributes on sockets"), which permits user.* xattrs on S_IFSOCK
inodes (previously rejected with -EPERM).

Adapts the test to expect success (exp_err = 0) instead of EPERM when
testing fsetxattr(2) on sockets with kernel 7.1.0+.

Behavior:
- Kernel < 7.1.0: fsetxattr() on socket returns -EPERM (original behavior)
- Kernel >= 7.1.0: fsetxattr() on socket returns 0 (success, new behavior)

Signed-off-by: Jan Polensky <japo@linux.ibm.com>
---
 .../kernel/syscalls/fsetxattr/fsetxattr02.c   | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/fsetxattr/fsetxattr02.c b/testcases/kernel/syscalls/fsetxattr/fsetxattr02.c
index 18490a865871..be6e7f489816 100644
--- a/testcases/kernel/syscalls/fsetxattr/fsetxattr02.c
+++ b/testcases/kernel/syscalls/fsetxattr/fsetxattr02.c
@@ -19,7 +19,10 @@
  * - Set attribute to a block special file, fsetxattr(2) should
  *   return -1 and set errno to EPERM.
  * - Set attribute to a UNIX domain socket, fsetxattr(2) should
- *   return -1 and set errno to EPERM.
+ *   return -1 and set errno to EPERM on kernels < 7.1.0.
+ *   On kernel 7.1.0+ (dc0876b9846d "xattr: support extended 
+ *   attributes on sockets"), returns 0 (success) as sockets now 
+ *   support user.* xattrs.
  */
 
 /*
@@ -62,6 +65,8 @@
 #define BLK      MNTPOINT"/fsetxattr02blk"
 #define SOCK     "fsetxattr02sock"
 
+static bool socket_xattr_supported;
+
 struct test_case {
 	char *fname;
 	int fd;
@@ -73,7 +78,16 @@ struct test_case {
 	int exp_err;
 	int issocket;
 	int needskeyset;
+	void (*fix_errno)(struct test_case *tc);
 };
+
+static void fix_sock(struct test_case *tc)
+{
+	/* dc0876b9846d ("xattr: support extended attributes on sockets") */
+	if (socket_xattr_supported)
+		tc->exp_err = 0;
+}
+
 static struct test_case tc[] = {
 	{			/* case 00, set attr to reg */
 	 .fname = FILENAME,
@@ -139,6 +153,7 @@ static struct test_case tc[] = {
 	 .flags = XATTR_CREATE,
 	 .exp_err = EPERM,
 	 .issocket = 1,
+	 .fix_errno = fix_sock,
 	 },
 };
 
@@ -152,6 +167,9 @@ static void verify_fsetxattr(unsigned int i)
 				XATTR_CREATE);
 	}
 
+	if (tc[i].fix_errno)
+		tc[i].fix_errno(&tc[i]);
+
 	TEST(fsetxattr(tc[i].fd, tc[i].key, tc[i].value, tc[i].size,
 			tc[i].flags));
 
@@ -233,6 +251,8 @@ static void setup(void)
 		SAFE_BIND(tc[i].fd, (const struct sockaddr *) &sun,
 				sizeof(struct sockaddr_un));
 	}
+
+	socket_xattr_supported = (tst_kvercmp(7, 1, 0) >= 0);
 }
 
 static void cleanup(void)
-- 
2.53.0


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

[LTP] [PATCH v3 2/2] setxattr02: Adapt test for kernel 7.1.0+ socket xattr support

[Jan Polensky]

Starting with kernel 7.1.0, sockets support extended attributes in the
user.* namespace. This behavior was enabled by dc0876b9846d ("xattr:
support extended attributes on sockets"), which permits user.* xattrs on
S_IFSOCK inodes (previously rejected with -EPERM).

Adapts the test to expect success (exp_err = 0) instead of EPERM when
testing setxattr(2) on sockets with kernel 7.1.0+.

Behavior:
- Kernel < 7.1.0: setxattr() on socket returns -EPERM (original behavior)
- Kernel >= 7.1.0: setxattr() on socket returns 0 (success, new behavior)

Signed-off-by: Jan Polensky <japo@linux.ibm.com>
---
 .../kernel/syscalls/setxattr/setxattr02.c     | 20 ++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/setxattr/setxattr02.c b/testcases/kernel/syscalls/setxattr/setxattr02.c
index 9f5f998da9ba..c1996fb60d73 100644
--- a/testcases/kernel/syscalls/setxattr/setxattr02.c
+++ b/testcases/kernel/syscalls/setxattr/setxattr02.c
@@ -15,7 +15,8 @@
  * - EPERM - set attribute to a FIFO
  * - EPERM - set attribute to a char special file
  * - EPERM - set attribute to a block special file
- * - EPERM - set attribute to a UNIX domain socket
+ * - EPERM/SUCCEED - set attribute to a UNIX domain socket (dc0876b9846d 
+ *   "xattr: support extended attributes on sockets")
  */
 
 #include "config.h"
@@ -49,6 +50,8 @@
 #define BLK      "setxattr02blk"
 #define SOCK     "setxattr02sock"
 
+static bool socket_xattr_supported;
+
 struct test_case {
 	char *fname;
 	char *key;
@@ -57,7 +60,16 @@ struct test_case {
 	int flags;
 	int exp_err;
 	int needskeyset;
+	void (*fix_errno)(struct test_case *tc);
 };
+
+static void fix_sock(struct test_case *tc)
+{
+	/* dc0876b9846d ("xattr: support extended attributes on sockets") */
+	if (socket_xattr_supported)
+		tc->exp_err = 0;
+}
+
 static struct test_case tc[] = {
 	{			/* case 00, set attr to reg */
 	 .fname = FILENAME,
@@ -115,6 +127,7 @@ static struct test_case tc[] = {
 	 .size = XATTR_TEST_VALUE_SIZE,
 	 .flags = XATTR_CREATE,
 	 .exp_err = EPERM,
+	 .fix_errno = fix_sock,
 	 },
 };
 
@@ -126,6 +139,9 @@ static void verify_setxattr(unsigned int i)
 				XATTR_CREATE);
 	}
 
+	if (tc[i].fix_errno)
+		tc[i].fix_errno(&tc[i]);
+
 	TEST(setxattr(tc[i].fname, tc[i].key, tc[i].value, tc[i].size,
 			tc[i].flags));
 
@@ -185,6 +201,8 @@ static void setup(void)
 	SAFE_MKNOD(CHR, S_IFCHR | 0777, dev);
 	SAFE_MKNOD(BLK, S_IFBLK | 0777, 0);
 	SAFE_MKNOD(SOCK, S_IFSOCK | 0777, 0);
+
+	socket_xattr_supported = (tst_kvercmp(7, 1, 0) >= 0);
 }
 
 static struct tst_test test = {
-- 
2.53.0


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

Re: [LTP] [PATCH v3 2/2] setxattr02: Adapt test for kernel 7.1.0+ socket xattr support

[Petr Vorel]

Hi Jan,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

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

Re: [LTP] [PATCH v3 2/2] setxattr02: Adapt test for kernel 7.1.0+ socket xattr support

[Andrea Cervesato via ltp]

Hi!

Sorry for jumping in a bit late, but...

> +static bool socket_xattr_supported;
> +
>  struct test_case {
>  	char *key;
> @@ -57,7 +60,16 @@ struct test_case {
>  	int flags;
>  	int exp_err;
>  	int needskeyset;
> +	void (*fix_errno)(struct test_case *tc);

Honestly I would have preferred a bool flag here instead of a function.
The code would have been more readable as:

static void verify_setxattr(unsigned int i)
{
..
    if (tc[i].check_xattr && socket_xattr_supported)
        tc->exp_err = 0;
..
}

And only 1 testcase would have been touched with `.check_xattr = 1`.

Also, please, always run `make check` before sending the patch. There are a
couple of issues to fix in the test description.

Same for the other patch.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

Re: [LTP] [PATCH v3 2/2] setxattr02: Adapt test for kernel 7.1.0+ socket xattr support

[Petr Vorel]

Hi Andrea,

> Hi!

> Sorry for jumping in a bit late, but...

> > +static bool socket_xattr_supported;
> > +
> >  struct test_case {
> >  	char *key;
> > @@ -57,7 +60,16 @@ struct test_case {
> >  	int flags;
> >  	int exp_err;
> >  	int needskeyset;
> > +	void (*fix_errno)(struct test_case *tc);

> Honestly I would have preferred a bool flag here instead of a function.
> The code would have been more readable as:

> static void verify_setxattr(unsigned int i)
> {
> ..
>     if (tc[i].check_xattr && socket_xattr_supported)
>         tc->exp_err = 0;
> ..
> }

> And only 1 testcase would have been touched with `.check_xattr = 1`.

OK, this is slightly simpler.

> Also, please, always run `make check` before sending the patch. There are a
> couple of issues to fix in the test description.

> Same for the other patch.

I wonder what error you mean. Modified tests are ok:

$ make check-setxattr02
CHECK testcases/kernel/syscalls/setxattr/setxattr02.c

$ make check-fsetxattr02
CHECK testcases/kernel/syscalls/fsetxattr/fsetxattr02.c

Also, I'm not aware that make check would check test description
(I guess it'd be part of checkpatch.pl check, but still no idea).

Kind regards,
Petr

> Regards,

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

Re: [LTP] [PATCH v3 2/2] setxattr02: Adapt test for kernel 7.1.0+ socket xattr support

[Jan Polensky]

Hi Petr and Andrea,
> Hi Andrea,
> 
> > Hi!
> 
> > Sorry for jumping in a bit late, but...
> 
> > > +static bool socket_xattr_supported;
> > > +
> > >  struct test_case {
> > >  	char *key;
> > > @@ -57,7 +60,16 @@ struct test_case {
> > >  	int flags;
> > >  	int exp_err;
> > >  	int needskeyset;
> > > +	void (*fix_errno)(struct test_case *tc);
> 
> > Honestly I would have preferred a bool flag here instead of a function.
> > The code would have been more readable as:
> 
> > static void verify_setxattr(unsigned int i)
> > {
> > ..
> >     if (tc[i].check_xattr && socket_xattr_supported)
> >         tc->exp_err = 0;
> > ..
> > }
> 
> > And only 1 testcase would have been touched with `.check_xattr = 1`.
> 
> OK, this is slightly simpler.
> 
[snip]
Yes, it's simpler. The `make check` command is a good thing, thanks for
pointing it out. I see what you mean.

I'll send v4 soon.

Kind regards,
Jan

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

Re: [LTP] [PATCH v3 1/2] fsetxattr02: Adapt test for kernel 7.1.0+ socket xattr support

[Andrea Cervesato via ltp]

Hi!

> +static void fix_sock(struct test_case *tc)
> +{
> +	/* dc0876b9846d ("xattr: support extended attributes on sockets") */
> +	if (socket_xattr_supported)
> +		tc->exp_err = 0;

There's a mix of `bool` and `int` here. I would just define tc->exp_err as `int`
and leave `bool` out of the test for consistency.

Also I was thinking that we shouldn't explicitly edit static variables meant
to describe the test scenario, so we probably need a local variable (also for
consistency).

    int exp_err = tc->exp_err;

    if (socket_xattr_supported)
        exp_err = 1;

I can add these two small things before merge if you agree with that.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

Re: [LTP] [PATCH v3 1/2] fsetxattr02: Adapt test for kernel 7.1.0+ socket xattr support

[Jan Polensky]

Hi Andrea:
> Hi!
>
> > +static void fix_sock(struct test_case *tc)
> > +{
> > +	/* dc0876b9846d ("xattr: support extended attributes on sockets") */
> > +	if (socket_xattr_supported)
> > +		tc->exp_err = 0;
>
> There's a mix of `bool` and `int` here. I would just define tc->exp_err as `int`
> and leave `bool` out of the test for consistency.
>
> Also I was thinking that we shouldn't explicitly edit static variables meant
> to describe the test scenario, so we probably need a local variable (also for
> consistency).
>
>     int exp_err = tc->exp_err;
>
>     if (socket_xattr_supported)
>         exp_err = 1;
>
> I can add these two small things before merge if you agree with that.
Yes, please! And thank you very much!
>
> Regards,
> --
> Andrea Cervesato
> SUSE QE Automation Engineer Linux
> andrea.cervesato@suse.com


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