Discussion:
Processed: bookworm-pu: package criu/3.17.1-2+deb12u1
(too old to reply)
Debian Bug Tracking System
2024-11-20 12:40:01 UTC
Permalink
affects -1 + src:criu
Bug #1087931 [release.debian.org] bookworm-pu: package criu/3.17.1-2+deb12u1
Added indication that 1087931 affects src:criu
--
1087931: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1087931
Debian Bug Tracking System
Contact ***@bugs.debian.org with problems
Florian Weimer
2024-11-20 13:20:01 UTC
Permalink
[ Changes ]
| cr-restore: rseq: dynamically handle *libc with rseq
| Before this patch we assumed that CRIU is compiled against
| the same GLibc as it runs with. But as we see from real
| world examples like #1935 it's not always true.
|
| The idea of this patch is to detect rseq configuration
| for the main CRIU process and use it to unregister
| rseq for all further child processes. It's correct,
| because we restore pstree using clone*() syscalls,
| don't use exec*() (!) syscalls, so rseq gets inherited
| in the kernel and rseq configuration remains the same
| for all children processes.
There's are further commit you should consider picking up:

commit 089345f77a34d1bc7ef146d650636afcd3cdda21
Author: Florian Weimer <***@redhat.com>
Date: Wed Jul 10 18:34:50 2024 +0200

Adjust to glibc __rseq_size semantic change

In commit 2e456ccf0c34a056e3ccafac4a0c7effef14d918 ("Linux: Make
__rseq_size useful for feature detection (bug 31965)") glibc 2.40
changed the meaning of __rseq_size slightly: it is now the size
of the active/feature area (20 bytes initially), and not the size
of the entire initially defined struct (32 bytes including padding).
The reason for the change is that the size including padding does not
allow detection of newly added features while previously unused
padding is consumed.

The prep_libc_rseq_info change in criu/cr-restore.c is not necessary
on kernels which have full ptrace support for obtaining rseq
information because the code is not used. On older kernels, it is
a correctness fix because with size 20 (the new value), rseq
registeration would fail.

The two other changes are required to make rseq unregistration work
in tests.

Signed-off-by: Florian Weimer <***@redhat.com>
Salvatore Bonaccorso
2024-11-20 13:30:01 UTC
Permalink
Hi Florian,
Post by Florian Weimer
[ Changes ]
| cr-restore: rseq: dynamically handle *libc with rseq
| Before this patch we assumed that CRIU is compiled against
| the same GLibc as it runs with. But as we see from real
| world examples like #1935 it's not always true.
|
| The idea of this patch is to detect rseq configuration
| for the main CRIU process and use it to unregister
| rseq for all further child processes. It's correct,
| because we restore pstree using clone*() syscalls,
| don't use exec*() (!) syscalls, so rseq gets inherited
| in the kernel and rseq configuration remains the same
| for all children processes.
commit 089345f77a34d1bc7ef146d650636afcd3cdda21
Date: Wed Jul 10 18:34:50 2024 +0200
Adjust to glibc __rseq_size semantic change
In commit 2e456ccf0c34a056e3ccafac4a0c7effef14d918 ("Linux: Make
__rseq_size useful for feature detection (bug 31965)") glibc 2.40
changed the meaning of __rseq_size slightly: it is now the size
of the active/feature area (20 bytes initially), and not the size
of the entire initially defined struct (32 bytes including padding).
The reason for the change is that the size including padding does not
allow detection of newly added features while previously unused
padding is consumed.
The prep_libc_rseq_info change in criu/cr-restore.c is not necessary
on kernels which have full ptrace support for obtaining rseq
information because the code is not used. On older kernels, it is
a correctness fix because with size 20 (the new value), rseq
registeration would fail.
The two other changes are required to make rseq unregistration work
in tests.
Do you consider this optional, or required for the fix to land in
bookrworm?

If it is required, I need to either first have v4.0 in unstable (or
cherry-pick the above as well on top of unstable and let it migrate to
testing), so we have the fix available in the upper suite before going
back to bookworm.

Regards,
Salvatore
Florian Weimer
2024-11-20 20:40:01 UTC
Permalink
Post by Salvatore Bonaccorso
Hi Florian,
Post by Florian Weimer
[ Changes ]
| cr-restore: rseq: dynamically handle *libc with rseq
| Before this patch we assumed that CRIU is compiled against
| the same GLibc as it runs with. But as we see from real
| world examples like #1935 it's not always true.
|
| The idea of this patch is to detect rseq configuration
| for the main CRIU process and use it to unregister
| rseq for all further child processes. It's correct,
| because we restore pstree using clone*() syscalls,
| don't use exec*() (!) syscalls, so rseq gets inherited
| in the kernel and rseq configuration remains the same
| for all children processes.
commit 089345f77a34d1bc7ef146d650636afcd3cdda21
Date: Wed Jul 10 18:34:50 2024 +0200
Adjust to glibc __rseq_size semantic change
In commit 2e456ccf0c34a056e3ccafac4a0c7effef14d918 ("Linux: Make
__rseq_size useful for feature detection (bug 31965)") glibc 2.40
changed the meaning of __rseq_size slightly: it is now the size
of the active/feature area (20 bytes initially), and not the size
of the entire initially defined struct (32 bytes including padding).
The reason for the change is that the size including padding does not
allow detection of newly added features while previously unused
padding is consumed.
The prep_libc_rseq_info change in criu/cr-restore.c is not necessary
on kernels which have full ptrace support for obtaining rseq
information because the code is not used. On older kernels, it is
a correctness fix because with size 20 (the new value), rseq
registeration would fail.
The two other changes are required to make rseq unregistration work
in tests.
Do you consider this optional, or required for the fix to land in
bookrworm?
The mentioned glibc change is probably in bookworm already. I think
it's this one from 2.36-9+deb12u8:

- Fixes rseq extension mechanism.

I think you need the test changes of the criu patch to get a clean
run. The non-test changes are dormant with sufficiently recent
kernels that contain full ptrace support.
Salvatore Bonaccorso
2024-11-20 22:10:01 UTC
Permalink
Hi Forian,

Thanks for your time!
Post by Florian Weimer
Post by Salvatore Bonaccorso
Hi Florian,
Post by Florian Weimer
[ Changes ]
| cr-restore: rseq: dynamically handle *libc with rseq
| Before this patch we assumed that CRIU is compiled against
| the same GLibc as it runs with. But as we see from real
| world examples like #1935 it's not always true.
|
| The idea of this patch is to detect rseq configuration
| for the main CRIU process and use it to unregister
| rseq for all further child processes. It's correct,
| because we restore pstree using clone*() syscalls,
| don't use exec*() (!) syscalls, so rseq gets inherited
| in the kernel and rseq configuration remains the same
| for all children processes.
commit 089345f77a34d1bc7ef146d650636afcd3cdda21
Date: Wed Jul 10 18:34:50 2024 +0200
Adjust to glibc __rseq_size semantic change
In commit 2e456ccf0c34a056e3ccafac4a0c7effef14d918 ("Linux: Make
__rseq_size useful for feature detection (bug 31965)") glibc 2.40
changed the meaning of __rseq_size slightly: it is now the size
of the active/feature area (20 bytes initially), and not the size
of the entire initially defined struct (32 bytes including padding).
The reason for the change is that the size including padding does not
allow detection of newly added features while previously unused
padding is consumed.
The prep_libc_rseq_info change in criu/cr-restore.c is not necessary
on kernels which have full ptrace support for obtaining rseq
information because the code is not used. On older kernels, it is
a correctness fix because with size 20 (the new value), rseq
registeration would fail.
The two other changes are required to make rseq unregistration work
in tests.
Do you consider this optional, or required for the fix to land in
bookrworm?
The mentioned glibc change is probably in bookworm already. I think
- Fixes rseq extension mechanism.
I think you need the test changes of the criu patch to get a clean
run. The non-test changes are dormant with sufficiently recent
kernels that contain full ptrace support.
Which, if I got the bit rights, should be since around upstream
90f093fa8ea4 ("rseq, ptrace: Add PTRACE_GET_RSEQ_CONFIGURATION
request") in v5.13-rc1.

https://criu.org/Rseq

Notabne: We do not run the testsuite during build for criu, and only
recently it got added as autopkgtest, which makes the whole now way
more confortable. For bookworm we almost have no coverage at all :-(

Regards,
Salvatore
Salvatore Bonaccorso
2024-11-23 13:00:01 UTC
Permalink
Hi,
Post by Salvatore Bonaccorso
Hi Forian,
Thanks for your time!
Post by Florian Weimer
Post by Salvatore Bonaccorso
Hi Florian,
Post by Florian Weimer
[ Changes ]
| cr-restore: rseq: dynamically handle *libc with rseq
| Before this patch we assumed that CRIU is compiled against
| the same GLibc as it runs with. But as we see from real
| world examples like #1935 it's not always true.
|
| The idea of this patch is to detect rseq configuration
| for the main CRIU process and use it to unregister
| rseq for all further child processes. It's correct,
| because we restore pstree using clone*() syscalls,
| don't use exec*() (!) syscalls, so rseq gets inherited
| in the kernel and rseq configuration remains the same
| for all children processes.
commit 089345f77a34d1bc7ef146d650636afcd3cdda21
Date: Wed Jul 10 18:34:50 2024 +0200
Adjust to glibc __rseq_size semantic change
In commit 2e456ccf0c34a056e3ccafac4a0c7effef14d918 ("Linux: Make
__rseq_size useful for feature detection (bug 31965)") glibc 2.40
changed the meaning of __rseq_size slightly: it is now the size
of the active/feature area (20 bytes initially), and not the size
of the entire initially defined struct (32 bytes including padding).
The reason for the change is that the size including padding does not
allow detection of newly added features while previously unused
padding is consumed.
The prep_libc_rseq_info change in criu/cr-restore.c is not necessary
on kernels which have full ptrace support for obtaining rseq
information because the code is not used. On older kernels, it is
a correctness fix because with size 20 (the new value), rseq
registeration would fail.
The two other changes are required to make rseq unregistration work
in tests.
Do you consider this optional, or required for the fix to land in
bookrworm?
The mentioned glibc change is probably in bookworm already. I think
- Fixes rseq extension mechanism.
I think you need the test changes of the criu patch to get a clean
run. The non-test changes are dormant with sufficiently recent
kernels that contain full ptrace support.
Which, if I got the bit rights, should be since around upstream
90f093fa8ea4 ("rseq, ptrace: Add PTRACE_GET_RSEQ_CONFIGURATION
request") in v5.13-rc1.
https://criu.org/Rseq
Notabne: We do not run the testsuite during build for criu, and only
recently it got added as autopkgtest, which makes the whole now way
more confortable. For bookworm we almost have no coverage at all :-(
To get some more confidence I did run the test suite similarly we do
now in the unstable autopkgtest (plus additionally first patching the
rseq test with the additional commit):

./zdtm.py run --criu-bin=/usr/sbin/criu --crit-bin=/usr/bin/crit --keep-going -a -x apparmor_stacking -x fd01 -x netns_lock_iptables

With that all tests pass (modulo the one skipped).

(Only on amd64 tested).

Regards,
Salvatore
Jonathan Wiltshire
2024-11-25 17:10:01 UTC
Permalink
package release.debian.org
tags 1087931 = bookworm pending
thanks

Hi,

The upload referenced by this bug report has been flagged for acceptance into the proposed-updates queue for Debian bookworm.

Thanks for your contribution!

Upload details
==============

Package: criu
Version: 3.17.1-2+deb12u1

Explanation: dynamically handle different libc at runtime than compilation time
Debian Bug Tracking System
2024-11-25 17:10:02 UTC
Permalink
Post by Jonathan Wiltshire
package release.debian.org
Limiting to bugs with field 'package' containing at least one of 'release.debian.org'
Limit currently set to 'package':'release.debian.org'
Post by Jonathan Wiltshire
tags 1087931 = bookworm pending
Bug #1087931 [release.debian.org] bookworm-pu: package criu/3.17.1-2+deb12u1
Added tag(s) pending.
Post by Jonathan Wiltshire
thanks
Stopping processing here.

Please contact me if you need assistance.
--
1087931: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1087931
Debian Bug Tracking System
Contact ***@bugs.debian.org with problems
Debian Bug Tracking System
2025-01-11 11:20:04 UTC
Permalink
Your message dated Sat, 11 Jan 2025 11:03:08 +0000
with message-id <E1tWZGm-009jXa-***@coccia.debian.org>
and subject line Close 1087931
has caused the Debian Bug report #1087931,
regarding bookworm-pu: package criu/3.17.1-2+deb12u1
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ***@bugs.debian.org
immediately.)
--
1087931: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1087931
Debian Bug Tracking System
Contact ***@bugs.debian.org with problems
Loading...