(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 <manojgu...@google.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).

Reply via email to