Skip to content
Snippets Groups Projects
  • Colin Walters's avatar
    673f48f6
    fdio: Use O_TMPFILE + rename-overwrite for regfile copies · 673f48f6
    Colin Walters authored
    I was working on rpm-ostree unified core, and hit the fact that
    `glnx_file_copy_at()` had the same bug with `fsetxattr()` and files whose mode
    is <= `0400` (e.g. `000` in the case of `/etc/shadow`) that libostree did a
    while ago.  Basically, Linux currently allows `write()` on non-writable open files
    but not `fsetxattr()`.  This situation is masked for privileged (i.e.
    `CAP_DAC_OVERRIDE`) code.
    
    Looking at this, I think it's cleaner to convert to `O_TMPFILE` here,
    since that code already handles setting the tmpfile to mode `0600`.  Now,
    this *is* a behavior change in the corner case of existing files which
    are symbolic links.  Previously we'd do an `open(O_TRUNC)` which would follow
    the link.
    
    But in the big picture, I think the use cases for `open(O_TRUNC)` are really
    rare - I audited all callers of this in ostree/rpm-ostree/flatpak, and all of
    them will be fine with this behavior change. For example, the ostree `/etc`
    merge code already explicitly unlinks the target beforehand. Other cases like
    supporting `repo/pubring.gpg` in an ostree repo being a symlink...eh, just no.
    
    Making this change allows us to convert to new style, and brings all of the
    general benefits of using `O_TMPFILE` too.
    673f48f6
    History
    fdio: Use O_TMPFILE + rename-overwrite for regfile copies
    Colin Walters authored
    I was working on rpm-ostree unified core, and hit the fact that
    `glnx_file_copy_at()` had the same bug with `fsetxattr()` and files whose mode
    is <= `0400` (e.g. `000` in the case of `/etc/shadow`) that libostree did a
    while ago.  Basically, Linux currently allows `write()` on non-writable open files
    but not `fsetxattr()`.  This situation is masked for privileged (i.e.
    `CAP_DAC_OVERRIDE`) code.
    
    Looking at this, I think it's cleaner to convert to `O_TMPFILE` here,
    since that code already handles setting the tmpfile to mode `0600`.  Now,
    this *is* a behavior change in the corner case of existing files which
    are symbolic links.  Previously we'd do an `open(O_TRUNC)` which would follow
    the link.
    
    But in the big picture, I think the use cases for `open(O_TRUNC)` are really
    rare - I audited all callers of this in ostree/rpm-ostree/flatpak, and all of
    them will be fine with this behavior change. For example, the ostree `/etc`
    merge code already explicitly unlinks the target beforehand. Other cases like
    supporting `repo/pubring.gpg` in an ostree repo being a symlink...eh, just no.
    
    Making this change allows us to convert to new style, and brings all of the
    general benefits of using `O_TMPFILE` too.