binpash / try

Inspect a command's effects before modifying your live system
MIT License
5.22k stars 68 forks source link

Potential issue with `rename` system call #170

Closed SleepyMug closed 1 week ago

SleepyMug commented 4 months ago

If the command being run has rename in it, while the renaming target is an existing directory, try gives EXDEV (Invalid cross-device link)

Potentially related to redirect_dir feature (https://docs.kernel.org/filesystems/overlayfs.html#renaming-directories)

To recreate:

Create rename.c:

#include <stdio.h>
#include <unistd.h>

int main(void)
{
    int ret;
    ret = link("x", "y");
    printf("link ret=%d\n", ret);
    ret = unlink("x");
    printf("unlink ret=%d\n", ret);
}

Then do the following:

$ gcc rename.c -o rename
$ mkdir x
$ ./rename
rename: Success
$ rmdir y; mkdir x
$ try ./rename
rename: Invalid cross-device link
kwakubiney commented 3 weeks ago

Can i pick this up? Assign to me if possible :)

ezrizhu commented 3 weeks ago

Can i pick this up? Assign to me if possible :)

Sure thing, please make a PR when it's ready.

kwakubiney commented 2 weeks ago

Which Linux environment (probably version?) did you run the rename binary on? @SleepyMug Can you also share your kernel config options for overlay? Perhaps the output of cat /boot/config-$(uname -r) | grep OVERLAY_FS

SleepyMug commented 2 weeks ago

Sorry for the late reply. I think the key here is that x/ and y/ lives on a different FS as /tmp.

For some reason I could not recreate the problem with link anymore, but rename does trigger the issue.

#include <stdio.h>
#include <unistd.h>

int main(void)
{
    int ret;
    ret = rename("x", "y");
    if (ret < 0) {
        perror("rename");
    }
}

This is the kernel config in case it's useful.

$ grep /boot/config-$(uname -r) -e 'OVERLAY'
CONFIG_EFI_CUSTOM_SSDT_OVERLAYS=y
CONFIG_OVERLAY_FS=m
# CONFIG_OVERLAY_FS_REDIRECT_DIR is not set
CONFIG_OVERLAY_FS_REDIRECT_ALWAYS_FOLLOW=y
# CONFIG_OVERLAY_FS_INDEX is not set
CONFIG_OVERLAY_FS_XINO_AUTO=y
# CONFIG_OVERLAY_FS_METACOPY is not set
kwakubiney commented 2 weeks ago

Haha, yes. Thanks for the reply. I was able to reproduce your issue with rename and came across this in the kernel

 * Directory renames only allowed on "pure upper" (already created on
 * upper filesystem, never copied up).  Directories which are on lower or
 * are merged may not be renamed.  For these -EXDEV is returned and
 * userspace has to deal with it.  This means, when copying up a
 * directory we can rely on it and ancestors being stable.

Attempting a fix on #184

mgree commented 1 week ago

Closing this as WONTFIX. This is an applicaiton-level issue and there's nothing we can do to fix broken applications.

kwakubiney commented 3 days ago

I've been thinking about this, and doesn't try sort of guarantee that whatever outcome you'd get on your machine without try would be the same you'd get on try? The usage of overlays is an implementation detail the user shouldn't really care about or?

mgree commented 1 day ago

An application using rename would have the same issue if the user tried to work across multiple partitions, not just in an overlay. Basically, rename is not mv and an application that thinks so is simply broken---and trying to fix it for them could break things worse.