Skip to content

Fix inheriting standard descriptors #13

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: crac
Choose a base branch
from
Open

Fix inheriting standard descriptors #13

wants to merge 2 commits into from

Conversation

rvansa
Copy link
Member

@rvansa rvansa commented Oct 19, 2023

The snprintf-fix addresses invalid merge with crac 3.14; the code was comparing recorded FDs to be inherited with uninitalized on-stack memory.

Using reopen_fd_as_nocheck needs a bit more insight to validate; maybe a proper fix would be elsewhere. When stdout (FD 1) is redirected to a file, this code receives new_fd set to FD 3 and fails as fle->fe->fd == 1 is open FD.

@rvansa rvansa mentioned this pull request Oct 19, 2023
Closed
1 task
@rvansa rvansa marked this pull request as draft October 24, 2023 10:04
@rvansa
Copy link
Member Author

rvansa commented Oct 24, 2023

Looks like this gets stuck in IgnoredFileDescriptorsTest; CRIU gets stuck in

#0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x0000564ab912cc6b in sys_futex (addr2=0x0, val3=0, timeout=0x7fff7f70e840, val1=<optimized out>, op=0, addr1=0x7f8f8e9750b0) at include/common/lock.h:28
#2  wait_fds_event () at criu/files.c:225
#3  0x0000564ab912ebab in open_fdinfos (me=0x7f8f8e975058) at criu/files.c:1243
#4  prepare_fds (me=0x7f8f8e975058) at criu/files.c:1334
#5  0x0000564ab911e717 in restore_one_alive_task (core=0x564aba6aeec0, pid=2638870) at criu/cr-restore.c:905
#6  restore_one_task (core=0x564aba6aeec0, pid=2638870) at criu/cr-restore.c:1256
#7  restore_task_with_children (_arg=<optimized out>) at criu/cr-restore.c:1976
#8  0x0000564ab910eb92 in clone3_with_pid_noasan (fn=fn@entry=0x564ab911dcd0 <restore_task_with_children>, arg=arg@entry=0x7fff7f70eaa0, flags=0, exit_signal=exit_signal@entry=17, pid=<optimized out>)
    at criu/clone-noasan.c:82
#9  0x0000564ab911a580 in fork_with_pid (item=item@entry=0x7f8f8e975058) at criu/cr-restore.c:1465
#10 0x0000564ab911b3d3 in restore_root_task (init=0x7f8f8e975058) at criu/cr-restore.c:2338
#11 0x0000564ab911ca90 in cr_restore_tasks () at criu/cr-restore.c:2705
#12 0x0000564ab90f35f1 in main (argc=<optimized out>, argv=0x7fff7f70ed78, envp=<optimized out>) at criu/crtools.c:315

@rvansa rvansa mentioned this pull request Oct 30, 2023
Closed
1 task
@rvansa rvansa marked this pull request as ready for review December 7, 2023 09:40
@rvansa
Copy link
Member Author

rvansa commented Dec 7, 2023

The IgnoredFileDescriptorsTest checkpointed a process with FD 43 mapped to stdout. CRIU selected FD 1 to create the pipe going to the new stdout, but then it was not created but rather inherited, and FD 43 waited indefinitely for that.
With the current solution the process won't be stuck, CRIU will create a new pipe for FD 43 (haven't found the other end, though) but it won't go to new stdout, as we ask specifically for inheriting FDs 0..2. Therefore if the user is in such a weird situation where he would actually want to use FD 43 (note that he must have already marked this FD to be ignored by JVM!) he would also have to use CRAC_CRIU_OPTS="--inherit-fd 'fd[43]:fd[1]'" to inherit new stdout as well.

Anyway, the issue was resolved and this can be reviewed/integrated.

pushkarnk pushed a commit to canonical/crac-criu that referenced this pull request May 28, 2025
@adrianreber @avagin
files-reg: fix buffer overflow on aarch64
2b74924 
Running the zdtm/static/unlink_regular00 test on Ubuntu 24.04 on aarch64
results in following error:

    # ./zdtm.py run -t zdtm/static/unlink_regular00 -k always
    userns is supported
    === Run 1/1 ================ zdtm/static/unlink_regular00
    ==================== Run zdtm/static/unlink_regular00 in ns ====================
    Skipping rtc at root
    Start test
    Test is SUID
    ./unlink_regular00 --pidfile=unlink_regular00.pid --outfile=unlink_regular00.out --dirname=unlink_regular00.test
    Run criu dump
    *** buffer overflow detected ***: terminated
    ############# Test zdtm/static/unlink_regular00 FAIL at CRIU dump ##############
    Test output: ================================

     <<< ================================
    Send the 9 signal to  47
    Wait for zdtm/static/unlink_regular00(47) to die for 0.100000
    ##################################### FAIL #####################################

According to the backtrace:

    #0  __pthread_kill_implementation (threadid=281473158467616, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
    #1  0x0000ffff93477690 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
    #2  0x0000ffff9342cb3c in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
    #3  0x0000ffff93417e00 in __GI_abort () at ./stdlib/abort.c:79
    #4  0x0000ffff9346abf0 in __libc_message_impl (fmt=fmt@entry=0xffff93552a78 "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:132
    #5  0x0000ffff934e81a8 in __GI___fortify_fail (msg=msg@entry=0xffff93552a28 "buffer overflow detected") at ./debug/fortify_fail.c:24
    #6  0x0000ffff934e79e4 in __GI___chk_fail () at ./debug/chk_fail.c:28
    #7  0x0000ffff934e9070 in ___snprintf_chk (s=s@entry=0xffffc6ed04a3 "testfile", maxlen=maxlen@entry=4056, flag=flag@entry=2, slen=slen@entry=4053,
        format=format@entry=0xaaaacffe3888 "link_remap.%d") at ./debug/snprintf_chk.c:29
    #8  0x0000aaaacff4b8b8 in snprintf (__fmt=0xaaaacffe3888 "link_remap.%d", __n=4056, __s=0xffffc6ed04a3 "testfile")
        at /usr/include/aarch64-linux-gnu/bits/stdio2.h:54
    CRaC#9  create_link_remap (path=path@entry=0xffffc6ed2901 "/zdtm/static/unlink_regular00.test/subdir/testfile", len=len@entry=60, lfd=lfd@entry=20,
        idp=idp@entry=0xffffc6ed14ec, nsid=nsid@entry=0xaaaada2bac00, parms=parms@entry=0xffffc6ed2808, fallback=0xaaaacff4c6c0 <dump_linked_remap+96>,
        fallback@entry=0xffffc6ed2797) at criu/files-reg.c:1164
    CRaC#10 0x0000aaaacff4c6c0 in dump_linked_remap (path=path@entry=0xffffc6ed2901 "/zdtm/static/unlink_regular00.test/subdir/testfile", len=len@entry=60,
        parms=parms@entry=0xffffc6ed2808, lfd=lfd@entry=20, id=id@entry=12, nsid=nsid@entry=0xaaaada2bac00, fallback=fallback@entry=0xffffc6ed2797)
        at criu/files-reg.c:1198
    CRaC#11 0x0000aaaacff4d8b0 in check_path_remap (nsid=0xaaaada2bac00, id=12, lfd=20, parms=0xffffc6ed2808, link=<optimized out>) at criu/files-reg.c:1426
    CRaC#12 dump_one_reg_file (lfd=20, id=12, p=0xffffc6ed2808) at criu/files-reg.c:1827
    CRaC#13 0x0000aaaacff51078 in dump_one_file (pid=<optimized out>, fd=4, lfd=20, opts=opts@entry=0xaaaada2ba2c0, ctl=ctl@entry=0xaaaada2c4d50,
        e=e@entry=0xffffc6ed39c8, dfds=dfds@entry=0xaaaada2c3d40) at criu/files.c:581
    CRaC#14 0x0000aaaacff5176c in dump_task_files_seized (ctl=ctl@entry=0xaaaada2c4d50, item=item@entry=0xaaaada2b8f80, dfds=dfds@entry=0xaaaada2c3d40)
        at criu/files.c:657
    CRaC#15 0x0000aaaacff3d3c0 in dump_one_task (parent_ie=0x0, item=0xaaaada2b8f80) at criu/cr-dump.c:1679
    CRaC#16 cr_dump_tasks (pid=<optimized out>) at criu/cr-dump.c:2224
    CRaC#17 0x0000aaaacff163a0 in main (argc=<optimized out>, argv=0xffffc6ed40e8, envp=<optimized out>) at criu/crtools.c:293

This line is the problem:

    snprintf(tmp + 1, sizeof(link_name) - (size_t)(tmp - link_name - 1), "link_remap.%d", rfe.id);

The problem was that the `-1` was on the inside of the braces and not on
the outside. This way the destination size was increase by 1 instead of
being decreased by 1 which triggered the buffer overflow detection.

Signed-off-by: Adrian Reber <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@rvansa