FS#71578 - setuid bit is lost

Attached to Project: Pacman
Opened by Yu Kou (cky) - Wednesday, 21 July 2021, 23:49 GMT
Last edited by Laurent Carlier (lordheavy) - Wednesday, 04 August 2021, 08:14 GMT
Task Type Bug Report
Category makepkg
Status Closed
Assigned To No-one
Architecture All
Severity High
Priority Normal
Reported Version 6.0.0
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Summary and Info:

The setuid bit is lost for the installed files whose setuid bit is supposed to be set, e.g., /usr/lib/Xorg.wrap.

This is a bug of makepkg. Precisely, it is a bug of /usr/share/makepkg/tidy/strip.sh. And I believe it was introduced by git commit 88d054093c1c99a697d95b26bd9aad5bc4d8e170 of pacman.

Here goes a part of the changes in that commit,

- strip $@ "$binary"
+ local tempfile=$(mktemp "$binary.XXXXXX")
+ if strip "$@" "$binary" -o "$tempfile"; then
+ cat "$tempfile" > "$binary"
+ fi
+ rm -f "$tempfile"

The original 'strip $@ "$binary"' can keep the setuid bit, but the new 'cat "$tempfile" > "$binary"' is the cause of the setuid bit being lost -- the kernel would drop the setuid bit if the file content has been changed. Fakeroot has nothing to do with this issue.




Steps to Reproduce:

```
asp export xorg-server # the current version is 1.20.12
cd xorg-server
makepkg --skipinteg -sf
ls -l pkg/xorg-server/usr/lib/Xorg.wrap # there is no setuid bit
```

The line 91 of PKGBUILD of xorg-server-1.20.12 is 'mv -v "${src}" "${dir}/"'. If change it to 'cp -av "${src}" "${dir}/"', then run the following command, you would get the original installed files under src/fakeroot/. Check the original Xorg.wrap, and you would see the setuid bit is there

```
makepkg -sef
ls -l src/fakeinstall/usr/lib/Xorg.wrap # the setuid bit is set
```
This task depends upon

Closed by  Laurent Carlier (lordheavy)
Wednesday, 04 August 2021, 08:14 GMT
Reason for closing:  Not a bug
Additional comments about closing:  Not a bug of makepkg
Comment by Eli Schwartz (eschwartz) - Wednesday, 28 July 2021, 05:11 GMT
I originally tested the git commit 88d054093c1c99a697d95b26bd9aad5bc4d8e170 using the shadow package, if I recall correctly. setuid works just fine.

Thank you for the report and explanation, but...

> Fakeroot has nothing to do with this issue.

Actually it does. Looking at the xorg-server PKGBUILD, it runs ninja install during build() and the setuid bit is set during build... outside of fakeroot. I suspect when the file is modified under fakeroot, it loses the setuid bit specifically because fakeroot doesn't know to restore it.

So first off, I believe you should open a separate bug report against the xorg-server package.

Beyond that, if you have suggestions for what makepkg can do here, I'd be all ears... but the original commit performs a valuable and needed role, and we can't revert it just to work around what IMHO is misuse of the format anyway.
Comment by Yu Kou (cky) - Wednesday, 04 August 2021, 00:30 GMT
Yes, you are right. If a file is originally created in fakeroot environment, the setuid bit could be preserved even if later the file content is modified (like using "cat A > B"), because the file's owner is "root" (faked though). And finally it will be recorded in tarball (as well as in the .MTREE), thus, later when pacman installs the package with that tarball, the right file mode will be set to each file.

The problem of xorg-server package is that it runs the "install" command (ninja install) in build() stage, which is not fakeroot'ed yet. So the files are not created originally in fakeroot environment.

This is not makepkg's bug. It is a bug in the PKGBUILD of xorg-server package.

Good thing is that, IMO, the issue can be fixed with a little effort. There is a function named "_install()" in PKGBUILD of xorg-server package. This function MOVES files from "fakeinstall" dir to "$pkgdir". ("fakeinstall" is where the "ninja install" goes to). To fix the issue, this function needs to COPY (with "-a" option) files from "fakeinstall" dir to "$pkgdir" (then remove the source files in "fakeinstall" dir). Because COPY would result in the files being created originally in fakeroot environment, which could have the setuid bit preserved later when the file content is modified.

Loading...