Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: build & test util-linux on Fedora Rawhide via Packit #1924

Merged
merged 1 commit into from Jan 2, 2023

Conversation

mrc0mmand
Copy link
Member

Let's run at least the unit-tests on Fedora Rawhide via Packit, as it supports a couple of alternative architectures (aarch64, ppc64le, s390x) as well as i*86, which were source of issues in the past.

@mrc0mmand
Copy link
Member Author

So far it seems to be doing the right thing, even though the collection of seds looks like one out of the seven deadly sins. For now I enabled all architectures which are available for Rawhide and there's a couple of test fails:

aarch64:

diff-{{{
--- /builddir/build/BUILD/util-linux-2.38/tests/expected/lsfd/column-name	2022-11-24 18:05:15.000000000 +0000
+++ /builddir/build/BUILD/util-linux-2.38/tests/output/lsfd/column-name	2022-11-24 18:20:56.878764719 +0000
@@ -2,5 +2,5 @@
 ro-regular-file:ASSOC,KNAME,NAME: 0
     3 anon_inode:[pidfd] pid=1 comm= nspid=1
 pidfd:ASSOC,KNAME,NAME: 0
-    3 socket:[INODENUM] state=connected type=dgram
+    3 socket:[INODENUM] state=unconnected type=dgram
 socketpair:ASSOC,KNAME,NAME: 0
}}}-diff

         lsfd: NAME and KNAME column                         ... FAILED (lsfd/column-name)
diff-{{{
--- /builddir/build/BUILD/util-linux-2.38/tests/expected/lsfd/mkfds-unix-dgram	2022-11-24 18:05:15.000000000 +0000
+++ /builddir/build/BUILD/util-linux-2.38/tests/output/lsfd/mkfds-unix-dgram	2022-11-24 18:20:58.458769684 +0000
@@ -1,6 +1,6 @@
-    3   SOCK state=connected path=test_mkfds-unix-dgram type=dgram  connected     dgram              0 test_mkfds-unix-dgram
-    4   SOCK state=connected type=dgram                             connected     dgram              0 
+    3   SOCK state=unconnected path=test_mkfds-unix-dgram type=dgram unconnected     dgram              0 test_mkfds-unix-dgram
+    4   SOCK state=unconnected type=dgram                            unconnected     dgram              0 
 ASSOC,STTYPE,NAME,SOCK.STATE,SOCK.TYPE,SOCK.LISTENING,UNIX.PATH: 0
-    3   SOCK state=connected path=@test_mkfds-unix-dgram type=dgram  connected     dgram              0 @test_mkfds-unix-dgram
-    4   SOCK state=connected type=dgram                              connected     dgram              0 
+    3   SOCK state=unconnected path=@test_mkfds-unix-dgram type=dgram unconnected     dgram              0 @test_mkfds-unix-dgram
+    4   SOCK state=unconnected type=dgram                             unconnected     dgram              0 
 ASSOC,STTYPE,NAME,SOCK.STATE,SOCK.TYPE,SOCK.LISTENING,UNIX.PATH: 0
}}}-diff

         lsfd: UNIX dgram sockets                            ... FAILED (lsfd/mkfds-unix-dgram)
diff-{{{
--- /builddir/build/BUILD/util-linux-2.38/tests/expected/lsfd/mkfds-unix-stream	2022-11-24 18:05:15.000000000 +0000
+++ /builddir/build/BUILD/util-linux-2.38/tests/output/lsfd/mkfds-unix-stream	2022-11-24 18:20:58.908771098 +0000
@@ -1,22 +1,22 @@
-    3   SOCK state=listen path=test_mkfds-unix-stream        listen    stream              1 test_mkfds-unix-stream
-    4   SOCK state=connected                              connected    stream              0 
-    5   SOCK state=connected path=test_mkfds-unix-stream  connected    stream              0 test_mkfds-unix-stream
+    3   SOCK state=listen path=test_mkfds-unix-stream type=stream        listen    stream              1 test_mkfds-unix-stream
+    4   SOCK state=connected type=stream                              connected    stream              0 
+    5   SOCK state=connected path=test_mkfds-unix-stream type=stream  connected    stream              0 test_mkfds-unix-stream
 ASSOC,STTYPE,NAME,SOCK.STATE,SOCK.TYPE,SOCK.LISTENING,UNIX.PATH: 0
-    3   SOCK state=listen path=@test_mkfds-unix-stream-abs        listen    stream              1 @test_mkfds-unix-stream-abs
-    4   SOCK state=connected                                   connected    stream              0 
-    5   SOCK state=connected path=@test_mkfds-unix-stream-abs  connected    stream              0 @test_mkfds-unix-stream-abs
+    3   SOCK state=listen path=@test_mkfds-unix-stream-abs type=stream        listen    stream              1 @test_mkfds-unix-stream-abs
+    4   SOCK state=connected type=stream                                   connected    stream              0 
+    5   SOCK state=connected path=@test_mkfds-unix-stream-abs type=stream  connected    stream              0 @test_mkfds-unix-stream-abs
 (abs) ASSOC,STTYPE,NAME,SOCK.STATE,SOCK.TYPE,SOCK.LISTENING,UNIX.PATH: 0
-    3   SOCK state=listen path=test_mkfds-unix-stream-shutdown        listen    stream              1 test_mkfds-unix-stream-shutdown
-    4   SOCK state=connected                                       connected    stream              0 
-    5   SOCK state=connected path=test_mkfds-unix-stream-shutdown  connected    stream              0 test_mkfds-unix-stream-shutdown
+    3   SOCK state=listen path=test_mkfds-unix-stream-shutdown type=stream        listen    stream              1 test_mkfds-unix-stream-shutdown
+    4   SOCK state=connected type=stream                                       connected    stream              0 
+    5   SOCK state=connected path=test_mkfds-unix-stream-shutdown type=stream  connected    stream              0 test_mkfds-unix-stream-shutdown
 (shutdown) ASSOC,STTYPE,NAME,SOCK.STATE,SOCK.TYPE,SOCK.LISTENING,UNIX.PATH: 0
     3   SOCK state=listen path=test_mkfds-unix-seqpacket type=seqpacket        listen seqpacket              1 test_mkfds-unix-seqpacket
     4   SOCK state=connected type=seqpacket                                 connected seqpacket              0 
     5   SOCK state=connected path=test_mkfds-unix-seqpacket type=seqpacket  connected seqpacket              0 test_mkfds-unix-seqpacket
 ASSOC,STTYPE,NAME,SOCK.STATE,SOCK.TYPE,SOCK.LISTENING,UNIX.PATH: 0
-    3   SOCK state=listen path=@test_mkfds-unix-seqpacket-abs        listen    stream              1 @test_mkfds-unix-seqpacket-abs
-    4   SOCK state=connected                                      connected    stream              0 
-    5   SOCK state=connected path=@test_mkfds-unix-seqpacket-abs  connected    stream              0 @test_mkfds-unix-seqpacket-abs
+    3   SOCK state=listen path=@test_mkfds-unix-seqpacket-abs type=stream        listen    stream              1 @test_mkfds-unix-seqpacket-abs
+    4   SOCK state=connected type=stream                                      connected    stream              0 
+    5   SOCK state=connected path=@test_mkfds-unix-seqpacket-abs type=stream  connected    stream              0 @test_mkfds-unix-seqpacket-abs
 (abs) ASSOC,STTYPE,NAME,SOCK.STATE,SOCK.TYPE,SOCK.LISTENING,UNIX.PATH: 0
     3   SOCK state=listen path=test_mkfds-unix-seqpacket-shutdown type=seqpacket        listen seqpacket              1 test_mkfds-unix-seqpacket-shutdown
     4   SOCK state=connected type=seqpacket                                          connected seqpacket              0 
}}}-diff

         lsfd: UNIX stream sockets                           ... FAILED (lsfd/mkfds-unix-stream)

i*86:


userns expected: 4026532328 4026531837 4026531837
userns actual: 

diff-{{{
--- /builddir/build/BUILD/util-linux-2.38/tests/expected/lsns/ioctl_ns	2022-11-24 18:05:15.000000000 +0000
+++ /builddir/build/BUILD/util-linux-2.38/tests/output/lsns/ioctl_ns	2022-11-24 18:15:49.525278143 +0000
@@ -1 +1 @@
-0
+1
}}}-diff

         lsns: ownership and hierarchy                       ... FAILED (lsns/ioctl_ns)

@masatake
Copy link
Member

masatake commented Nov 24, 2022

I will take a look. Thank you for reporting.


(I will use this comment to note my analysis.)

  • builddir/build/BUILD/util-linux-2.38/tests/output/lsfd/mkfds-unix-stream
    The kernel looks older than I assumed. type=stream should not be printed if
    the kernel reports "UNIX-STREAM" as the "protoname" of the socket.
    unix_get_name() in misc-utils/lsfd-sock-xinfo.c.
    Nov 25.

@karelzak
Copy link
Collaborator

That's cool. @mrc0mmand thanks!

@masatake
Copy link
Member

@mrc0mmand is there a way to know the version of kernel used for running the test cases?

@mrc0mmand
Copy link
Member Author

@mrc0mmand is there a way to know the version of kernel used for running the test cases?

Yup, in this case its kernel: 5.14.10-300.fc35.aarch64 (unfortunately, as the build runs in an nspawn container, so it shares host's kernel).

@karelzak
Copy link
Collaborator

@mrc0mmand is there a way to know the version of kernel used for running the test cases?

The test suite prints the version at the beginning,

-------------------- util-linux regression tests --------------------

                    For development purpose only.                    
                 Don't execute on production system!                 

       kernel: 5.14.10-300.fc35.aarch64          

See rpm-build:fedora-rawhide-aarch64 in the list of PR checks, then "Details" -> "Dashboard URL" -> "Build Logs" where is complete output.

@mrc0mmand
Copy link
Member Author

@karelzak this should be almost ready, and thanks to #1936 I was able to drop one of the seds.

There's one remaining fail on i*86 I can't really wrap my head around - lsns on i*86 doesn't return anything (at least in the nspawn container, that's used by Copr/Packit), which breaks the lsns/ioctl_ns test:

userns expected: 4026532440 4026531837 4026531837
userns actual: 

diff-{{{
--- /builddir/build/BUILD/util-linux-2.38/tests/expected/lsns/ioctl_ns	2022-12-01 18:12:53.000000000 +0000
+++ /builddir/build/BUILD/util-linux-2.38/tests/output/lsns/ioctl_ns	2022-12-01 18:22:05.510001764 +0000
@@ -1 +1 @@
-0
+1
}}}-diff

         lsns: ownership and hierarchy                       ... FAILED (lsns/ioctl_ns)

It can be reproduced locally using mock:

$ mock --init --install util-linux -r fedora-rawhide-i386
...
$ mock --shell -r fedora-rawhide-i386
# stat -c %i -L /proc/self/ns/user
4026531837
# stat -c %i -L /proc/self/ns/pid
4026538401
# lsns
<no output>

This seems to work fine in an x86_64 container:

$ mock --init --install util-linux -r fedora-rawhide-x86_64
...
$ mock --shell -r fedora-rawhide-x86_64
# stat -c %i -L /proc/self/ns/user
4026531837
# stat -c %i -L /proc/self/ns/pid
4026538407
# lsns
        NS TYPE   NPROCS PID USER COMMAND
4026531834 time        3   1 root (sd-stubinit)
4026531837 user        3   1 root (sd-stubinit)
4026538031 net         3   1 root (sd-stubinit)
4026538403 cgroup      3   1 root (sd-stubinit)
4026538404 mnt         3   1 root (sd-stubinit)
4026538405 uts         3   1 root (sd-stubinit)
4026538406 ipc         3   1 root (sd-stubinit)
4026538407 pid         3   1 root (sd-stubinit)

@karelzak
Copy link
Collaborator

karelzak commented Dec 9, 2022

On fedora-rawhide-i386, it ends on NS_GET_USERNS:

openat(4, "ns/mnt", O_RDONLY|O_LARGEFILE) = 6
ioctl(6, NS_GET_USERNS)                 = -1 ENOTTY (Inappropriate ioctl for device)

Linux returns this errno when not able to identify the ioctl. IMHO the problem is the mismatch between the 64bit kernel and 32bit userspace.

I guess the best would be to detect this situation and mark the test as KNOWN_FAIL.

karelzak added a commit that referenced this pull request Dec 9, 2022
It seems 32bit userspace on 64bit kernel return ENOTTY for
NS_GET_USERNS ioctl (for example when execute tests in mock
environment).

Addresses: #1924
Signed-off-by: Karel Zak <kzak@redhat.com>
@karelzak
Copy link
Collaborator

@mrc0mmand so, are we ready to merge it? ;-)

@mrc0mmand
Copy link
Member Author

@mrc0mmand so, are we ready to merge it? ;-)

Indeed, thanks! I've rebased the PR branch, so let's see how it goes :)

@mrc0mmand
Copy link
Member Author

@karelzak unfortunately, the i386 build still fails:

        lsmem: lsmem                                         ... OK (all 2 sub-tests PASSED)

userns expected: 4026532250 4026531837 4026531837
userns actual: 

diff-{{{
--- /builddir/build/BUILD/util-linux-2.38/tests/expected/lsns/ioctl_ns	2022-12-12 09:14:00.000000000 +0000
+++ /builddir/build/BUILD/util-linux-2.38/tests/output/lsns/ioctl_ns	2022-12-12 09:25:02.031688284 +0000
@@ -1 +1 @@
-0
+1
}}}-diff

         lsns: ownership and hierarchy                       ... FAILED (lsns/ioctl_ns)

I'll sprinkle the test with some debug logs.

Also, it looks like 825feef broke build in the CodeQL Action:

   [295/878] Compiling C object test_path.p/lib_cpuset.c.o
  [296/878] Linking target test_path
  [297/878] Linking target chsh
  FAILED: chsh 
  cc  -o chsh chsh.p/login-utils_chsh.c.o chsh.p/login-utils_ch-common.c.o chsh.p/login-utils_auth.c.o chsh.p/login-utils_islocal.c.o chsh.p/login-utils_setpwnam.c.o -Wl,--as-needed -Wl,--no-undefined -Wl,--start-group lib/libcommon.a /usr/lib/x86_64-linux-gnu/libreadline.so -lpam -lpam_misc -Wl,--end-group
  /usr/bin/ld: chsh.p/login-utils_chsh.c.o: in function `check_shell':
  /home/runner/work/util-linux/util-linux/_lgtm_build_dir/../login-utils/chsh.c:181: undefined reference to `is_known_shell'
  /usr/bin/ld: chsh.p/login-utils_chsh.c.o: in function `main':
  /home/runner/work/util-linux/util-linux/_lgtm_build_dir/../login-utils/chsh.c:265: undefined reference to `is_known_shell'
  collect2: error: ld returned 1 exit status
  [298/878] Compiling C object test_pty.p/lib_pty-session.c.o
  [299/878] Compiling C object test-consoles.p/login-utils_sulogin-consoles.c.o
  ninja: build stopped: subcommand failed.

Full log: https://github.com/util-linux/util-linux/actions/runs/3674306157/jobs/6212366513#step:5:937

@mrc0mmand
Copy link
Member Author

Ah, I see, the workaround from 857038d doesn't work, since uname -m in this case reports i686:

<mock-chroot> sh-5.2# uname -a
Linux df503e9072134a32b4a352f3de48271a 5.18.5-200.fc36.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Jun 16 14:51:11 UTC 2022 i686 GNU/Linux
# uname -m
i686

@karelzak
Copy link
Collaborator

There is no elegant way to detect kernel wordsize (32/64 bit) in an environment like mock. There is possible to use /proc/sys/kernel/osrelease (or uname -a), but this is a distribution-specific string. Things like getconf LONG_BIT returns info about userspace rather than the kernel, lscpu returns info about CPU, etc. It seems for now only reliable way is to try any 64-bit syscall.

@t-8ch, what about adding to the /proc something like wordsize file with 32 or 64 value? You have already been successful with LE/BE. What do you think? ;-)

@t-8ch
Copy link
Member

t-8ch commented Dec 12, 2022

@karelzak I'll look into it in a bit.

@t-8ch
Copy link
Member

t-8ch commented Dec 14, 2022

While it would be possible to add a new sysfs file to skip the test, would it not be better to fix the ioctls 32bit compat?

For the time being we would still need a workaround in any case.
It would probably be best to report the failing ioctl from lsns and skip the test based on that.

@t-8ch
Copy link
Member

t-8ch commented Dec 15, 2022

I have a patch that makes the ioctl work.
I'll submit it at some point next week.

This was referenced Dec 15, 2022
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this pull request Dec 19, 2022
As all parameters and return values of the ioctls have the same
representation on both 32bit and 64bit we can reuse the normal ioctl
handler for the compat handler via compat_ptr_ioctl().

All nsfs ioctls return a plain "int" filedescriptor which is a signed
4-byte integer type on both 32bit and 64bit.
The only parameter taken is by NS_GET_OWNER_UID and is a pointer to a
"uid_t" which is a 4-byte unsigned integer type on both 32bit and 64bit.

Fixes: 6786741 ("nsfs: add ioctl to get an owning user namespace for ns file descriptor")
Reported-by: Karel Zak <kzak@redhat.com>
Link: util-linux/util-linux#1924 (comment)
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
t-8ch added a commit to t-8ch/linux that referenced this pull request Dec 21, 2022
As all parameters and return values of the ioctls have the same
representation on both 32bit and 64bit we can reuse the normal ioctl
handler for the compat handler via compat_ptr_ioctl().

All nsfs ioctls return a plain "int" filedescriptor which is a signed
4-byte integer type on both 32bit and 64bit.
The only parameter taken is by NS_GET_OWNER_UID and is a pointer to a
"uid_t" which is a 4-byte unsigned integer type on both 32bit and 64bit.

Fixes: 6786741 ("nsfs: add ioctl to get an owning user namespace for ns file descriptor")
Reported-by: Karel Zak <kzak@redhat.com>
Link: util-linux/util-linux#1924 (comment)
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
@karelzak
Copy link
Collaborator

karelzak commented Dec 21, 2022

Yes, I've thought about the ioctl too, and it's definitely a way how to fix the core of the problem, but I still think that we need a way how to detect 32 vs. 64 bits. (I mean not for this issue, but as a generic thing.)

@karelzak
Copy link
Collaborator

@mrc0mmand can you rebase this PR? I guess with recent @t-8ch's changes; it's ready now.

Let's run at least the unit-tests on Fedora Rawhide via Packit, as it
supports a couple of alternative architectures (aarch64, ppc64le, s390x)
as well as i*86, which were source of issues in the past.
@mrc0mmand
Copy link
Member Author

@mrc0mmand can you rebase this PR? I guess with recent @t-8ch's changes; it's ready now.

That indeed seems to have done the trick, thanks!

@t-8ch
Copy link
Member

t-8ch commented Dec 21, 2022

@karelzak Do we have a concrete usecase to argue for the new sysfs file?
I think it could be used by setarch to replace this logic:

unsigned long wrdsz = CHAR_BIT * sizeof(void *);

Anything else?

PS: all tests on this PR are green.

@t-8ch
Copy link
Member

t-8ch commented Dec 21, 2022

The packit logs currently all talk about "util-linux-2.38". It would be clearer to have some indication that is actually a development version.

@karelzak karelzak merged commit 45e0307 into util-linux:master Jan 2, 2023
@mrc0mmand mrc0mmand deleted the packit branch January 2, 2023 12:45
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this pull request Jan 11, 2023
As all parameters and return values of the ioctls have the same
representation on both 32bit and 64bit we can reuse the normal ioctl
handler for the compat handler via compat_ptr_ioctl().

All nsfs ioctls return a plain "int" filedescriptor which is a signed
4-byte integer type on both 32bit and 64bit.
The only parameter taken is by NS_GET_OWNER_UID and is a pointer to a
"uid_t" which is a 4-byte unsigned integer type on both 32bit and 64bit.

Fixes: 6786741 ("nsfs: add ioctl to get an owning user namespace for ns file descriptor")
Reported-by: Karel Zak <kzak@redhat.com>
Link: util-linux/util-linux#1924 (comment)
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this pull request Jan 11, 2023
As all parameters and return values of the ioctls have the same
representation on both 32bit and 64bit we can reuse the normal ioctl
handler for the compat handler via compat_ptr_ioctl().

All nsfs ioctls return a plain "int" filedescriptor which is a signed
4-byte integer type on both 32bit and 64bit.
The only parameter taken is by NS_GET_OWNER_UID and is a pointer to a
"uid_t" which is a 4-byte unsigned integer type on both 32bit and 64bit.

Fixes: 6786741 ("nsfs: add ioctl to get an owning user namespace for ns file descriptor")
Reported-by: Karel Zak <kzak@redhat.com>
Link: util-linux/util-linux#1924 (comment)
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants