Hi Forian,
Thanks for your time!
On Wed, Nov 20, 2024 at 09:33:53PM +0100, Florian Weimer wrote:
> * Salvatore Bonaccorso:
>
> > Hi Florian,
> >
> > On Wed, Nov 20, 2024 at 02:05:46PM +0100, Florian Weimer wrote:
> >> * Salvatore Bonaccorso:
> >>
> >> > [ Changes ]
> >> > Quoting the upsream commit is likely the best to explain the 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 <fwei...@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 <fwei...@redhat.com>
> >
> > 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.
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
