Fix: getsockopt usage for SO_PASSCRED/SO_PASSSEC on Linux 6.16+Fix socket opts

[DongSunchao]

Linux 6.16 changed the behavior of getsockopt for credential-related options and it restricted SO_PASS{CRED,PIDFD,SEC} to AF_{UNIX,NETLINK,BLUETOOTH} .
This patch updates the logic to correctly handle SO_PASSCRED and SO_PASSSEC,
ensuring compatibility with newer kernels while preserving support for older ones.

Besides, /test/zdtm/static/sock_opts00.c has been changed to check the kernel version so that PF_INET can skip the SO_PASSCRED and SO_PASSSEC options.

Tested with: ./test/zdtm.py run -t zdtm/static/sock_opts00 -f ns

Fixes: #2705

[mihalicyn]

Hi,

1. you don't need to change anything in the test code. Test code is correct.
2. we shouldn't introduce any checks based on a kernel version in CRIU code, except a very-very rare cases
3. please, squash your commits and add Signed-off-by tag

[DongSunchao]

Hi,

1. you don't need to change anything in the test code. Test code is correct.
2. we shouldn't introduce any checks based on a kernel version in CRIU code, except a very-very rare cases
3. please, squash your commits and add Signed-off-by tag

Thank you for your advice.

1. I have a question about the test code in /test/zdtm/static/sock_opts00.c. It currently usesPF_INETto set OPT(SO_PASSCRED) and OPT(SO_PASSSEC).However, these options are restricted to AF_{UNIX,NETLINK,BLUETOOTH}. Additionally, getsockopt() is used, which would cause an error if used to set an option like OPT(SO_PASSCRED) for a PF_INET socket. Is this the intended behavior?
2. I have removed all kernel version checks. This raises a quesion: Should we allow PF_INET or other non- AF_{UNIX,NETLINK,BLUETOOTH} sockets to set these kernel version older than 6.16, or should this be disallowed across all kernel version? If we need different behaviors according to different kernel versions, how can we implement it without kernel version checks?
3. Sorry, I haven't used this before. Thank you for pointing this out.

[adrianreber]

Take a look at kerndat.c. That is where CRIU checks for runtime features. It is important to check for runtime features and not during compile time.

[avagin]

Hi @DongSunchao , please read the contribution guidelines at https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html and reformat your commits. The command git rebase -i origin/criu-dev should be helpful.

[Snorch]

I somehow overlooked this one, you can look into #2719 where I have some improving ideas. Like:

• using existing family from upper function instead of rereading it via dump_opt
• spliting zdtm hunk to separate commit
• using unix socket to test SO_PASSCRED/SO_PASSSEC

[DongSunchao]

I somehow overlooked this one, you can look into #2719 where I have some improving ideas. Like:

• using existing family from upper function instead of rereading it via dump_opt
• spliting zdtm hunk to separate commit
• using unix socket to test SO_PASSCRED/SO_PASSSEC

I've seen your code, and I really think your approach is better than mine. I'll change my code soon.

[rst0git]

@DongSunchao Would it be possible to reset the author of your commits to yourself (instead of root) and use messages similar to the commits in #2719?

The following blog post provides more information on how to write good commit messages and why this is important: https://cbea.ms/git-commit/#seven-rules

[DongSunchao]

@DongSunchao Would it be possible to reset the author of your commits to yourself (instead of root) and use messages similar to the commits in #2719?

The following blog post provides more information on how to write good commit messages and why this is important: https://cbea.ms/git-commit/#seven-rules

I've made the requested changes and have done my best to follow the guidelines. However, I've run into a new issue: my code passes the Fedora Rawhide test in my repository action but fails the Vagrant Fedora Rawhide test on this PR. I'm not sure how to resolve it.

[Snorch]

Having a hunk adding lines in first patch

--- a/test/zdtm/static/sock_opts00.c
+++ b/test/zdtm/static/sock_opts00.c
@@ -83,8 +83,15 @@ int main(int argc, char **argv)
                        pr_perror("can't verify %s", vname[i].name);
                        return 1;
                }
-
+               /*
+               * for kernel version >= 6.16.0 Restrict
+               * SO_PASS{CRED,PIDFD,SEC} to AF_{UNIX,NETLINK,BLUETOOTH}
+               */
                if (val[i] != rval) {
+                       if (vname[i].opt == SO_PASSCRED || vname[i].opt == SO_PASSSEC) {
+                               continue;
+                       }
+
                        errno = 0;
                        fail("%s changed: %d -> %d", vname[i].name, val[i], rval);
                        return 1;

And then having hunk removing those same lines in second patch:

@@ -78,28 +85,16 @@ int main(int argc, char **argv)
        test_waitsig();
 
        for (i = 0; i < NOPTS; i++) {
-               ret = getsockopt(sock, SOL_SOCKET, vname[i].opt, &rval, &len);
+               sk = vname[i].opt == SO_PASSCRED || vname[i].opt == SO_PASSSEC ? usock : sock;
+               ret = getsockopt(sk, SOL_SOCKET, vname[i].opt, &rval, &len);
                if (ret) {
                        pr_perror("can't verify %s", vname[i].name);
                        return 1;
                }
-               /*
-               * for kernel version >= 6.16.0 Restrict
-               * SO_PASS{CRED,PIDFD,SEC} to AF_{UNIX,NETLINK,BLUETOOTH}
-               */
-               if (val[i] != rval) {
-                       if (vname[i].opt == SO_PASSCRED || vname[i].opt == SO_PASSSEC) {
-                               continue;
-                       }
-
-                       errno = 0;
-                       fail("%s changed: %d -> %d", vname[i].name, val[i], rval);
-                       return 1;
-               }
        }
 
        pass();
        close(sock);

is generally discouraged. Please remove the hunk touching zdtm from the first patch completely.

Also by those hunks you remove important if (val[i] != rval) { value persistence check, which is not what we want, please return it.

[DongSunchao]

Having a hunk adding lines in first patch

--- a/test/zdtm/static/sock_opts00.c
+++ b/test/zdtm/static/sock_opts00.c
@@ -83,8 +83,15 @@ int main(int argc, char **argv)
                        pr_perror("can't verify %s", vname[i].name);
                        return 1;
                }
-
+               /*
+               * for kernel version >= 6.16.0 Restrict
+               * SO_PASS{CRED,PIDFD,SEC} to AF_{UNIX,NETLINK,BLUETOOTH}
+               */
                if (val[i] != rval) {
+                       if (vname[i].opt == SO_PASSCRED || vname[i].opt == SO_PASSSEC) {
+                               continue;
+                       }
+
                        errno = 0;
                        fail("%s changed: %d -> %d", vname[i].name, val[i], rval);
                        return 1;

And then having hunk removing those same lines in second patch:

@@ -78,28 +85,16 @@ int main(int argc, char **argv)
        test_waitsig();
 
        for (i = 0; i < NOPTS; i++) {
-               ret = getsockopt(sock, SOL_SOCKET, vname[i].opt, &rval, &len);
+               sk = vname[i].opt == SO_PASSCRED || vname[i].opt == SO_PASSSEC ? usock : sock;
+               ret = getsockopt(sk, SOL_SOCKET, vname[i].opt, &rval, &len);
                if (ret) {
                        pr_perror("can't verify %s", vname[i].name);
                        return 1;
                }
-               /*
-               * for kernel version >= 6.16.0 Restrict
-               * SO_PASS{CRED,PIDFD,SEC} to AF_{UNIX,NETLINK,BLUETOOTH}
-               */
-               if (val[i] != rval) {
-                       if (vname[i].opt == SO_PASSCRED || vname[i].opt == SO_PASSSEC) {
-                               continue;
-                       }
-
-                       errno = 0;
-                       fail("%s changed: %d -> %d", vname[i].name, val[i], rval);
-                       return 1;
-               }
        }
 
        pass();
        close(sock);

is generally discouraged. Please remove the hunk touching zdtm from the first patch completely.

Also by those hunks you remove important if (val[i] != rval) { value persistence check, which is not what we want, please return it.

My apologies. I caught a cold last night, so I rechecked my code and fixed the other errors. Please point out any other issues you find. I'd appreciate your help.

[Snorch]

Please point out any other issues you find.

I still see all the same problems with zdtm hunk and also new excess hunk 8402f8d#diff-d6b1349661211f7a79a28b685b951ce03f5919c95f404faacea6ce98692b8bf6

[Snorch]

Looks good. Thank you!

[DongSunchao]

Looks good. Thank you!

Hi, there is another issue related to the one I mentioned previously. The code fails the Vagrant Fedora Rawhide based test with an error in vdso-proxy. The specific issue is that it's creating three new VMAs and incorrectly incorporating one existing VMA. This behavior is unexpected.

However, I've tried running the same test on my local virtual machine (kernel version 6.17.0-0.rc1.250815gd7ee5bdce789.21.fc44.x86_64, Vagrant Fedora), and it passes successfully.

I find this very strange and suspect the issue might be related to the CI environment.