[gentoo-dev] Re: portage reliance on GNU objcopy ownership perseverance behavior in strip

[Fāng-ruì Sòng]

(I replied via https://groups.google.com/g/linux.gentoo.dev/c/WG-OLQe3yng
"Reply all" (which only replied to the list AFAICT) but I did not
subscribe to gentoo-dev via the official
https://www.gentoo.org/get-involved/mailing-lists/ so my reply is
missing)

On Thu, Feb 4, 2021 at 4:09 PM Manoj Gupta <manojgupta@××××××.com> wrote:
>
> Hi gentoo devs,
>
> This question is regarding interaction of fowners [1] and estrip functionality in portage.
> fowners is used on various binaries and files to assign the ownership to specific users or group.
>
> GNU objcopy and strip do not change the file ownership when run as root. However, llvm's versions do not preserve it and instead make root the owner of the modified file.
> e.g.
> sudo strip <file> keeps the original ownership .
> sudo llvm-strip <file> will change ownership to root.
>
> We were trying to have llvm objcopy with a patch [3] to have the same behavior as GNU but LLVM developers pointed out that GNU implementation is thought to have a security issue: https://sourceware.org/bugzilla/show_bug.cgi?id=26945
> We have modified the LLVM patch to avoid chown on the final file and rather doing it on the temporay file but I am not sure if that will be enough to placate the llvm devs.
>
> What does everyone think of modifying usages of calls to strip and objcopy inside estrip so that file ownership is manually restored. e.g
>
> owner=$(stat -U file)
> group=$(stat -G file)
> strip <file>
> chown owner:group file
>
> [1] https://devmanual.gentoo.org/function-reference/install-functions/
> [2] https://gitweb.gentoo.org/proj/portage.git/tree/bin/estrip
> [3] https://reviews.llvm.org/D93881
>
> Thanks,
> Manoj

> This is probably safe in portage because the temporary directory is no
> longer user-accessible, but it seems perverse to seek feature parity by
> adding the security vulnerability into the implementations that don't
> have it, rather than by removing it from the ones that do. Hopefully
> LLVM just accepts the patch.

In binutils, objcopy/strip call 'smart_rename', which has a
vulnerability: https://sourceware.org/bugzilla/show_bug.cgi?id=26945
The first resolution used renameat2, which is not intercepted by
fakeroot (Arch Linux runs strip and tar under fakeroot, then
extracting the tar to the system /).
After discussion with binutils upstream, Arch Linux's resolution is to
use `strip a -o b` instead of `strip a`:
https://git.archlinux.org/pacman.git/commit/?id=88d054093c1c99a697d95b26bd9aad5bc4d8e170

For some unclear reason the first binutils resolution was reverted in
the 2.36 branch. The latest attempt is to use open(O_TRUNC)+write
instead of atomic rename
https://sourceware.org/pipermail/binutils/2021-February/115282.html
That preserves security context/xattr (not used by any distribution
package)/owner/group/mode and avoids fchmod/chown.

However, like other non-atomic operations, this approach has one
wasteful write (`strip a` writes two files) and can potentially
corrupt the output in the case of full disk situation and other
erroneous cases.

The root cause is that `sudo strip a` (or `strip a` under fakeroot)
restoring owner/group is a questionable interface and distributions
read too much from the behavior.
Personally I'd like llvm-objcopy to keep the behavior as is (no
chown/fchown). Neither https://reviews.llvm.org/D96308 nor
https://reviews.llvm.org/D93881 is appealing.

Doing this (get_owner_group; {strip,llvm-strip} file -o temp && chown
owner:group file && mv temp file) on Portage side is doing a favor to
LLVM binary utilities.
It benefits GNU binutils as well - with newer binutils, the current
`strip a` will have a wasteful write and 2x dirty pages (write a
temporary file and copy that content to the original file).