chown: link to chgrp in the same bin folder #133

skhal · 2018-03-21T13:02:41Z

chown creates a link from /usr/sbin/chown to /usr/bin/chgrp
instead of staying in the same bin-folder, e.g. /usr/sbin/chgrp.

it makes install world fail if one creates sepearate ZFS datasets
for /usr/bin and /usr/sbin -- cross-filesystem link fails.

BUG

% find /usr/src -type f -name Makefile | \
	xargs grep --color '^LINKS' | \
	grep -v '\(\(BIN\|LIB\|FILES\|SCRIPTS\)DIR\).*\1'
./usr.sbin/chown/Makefile:LINKS=        ${BINDIR}/chown /usr/bin/chgrp

stesser · 2018-03-21T15:51:20Z

Moving chgrp into /usr/sbin will break scripts that use it with a hard-coded path (/usr/bin/chgrp).

I had the same problem and have used a symbolic link to resolve it (i.e. SYMLINKS= instead of LINKS= in the Makefile).

chown creates a link from /usr/sbin/chown to /usr/bin/chgrp instead of staying in the same bin-folder, e.g. /usr/sbin/chgrp. it makes install world fail if one creates sepearate ZFS datasets for /usr/bin and /usr/sbin -- cross-filesystem link fails. BUG % find /usr/src -type f -name Makefile | \ xargs grep --color '^LINKS' | \ grep -v '$\(BIN\|LIB\|FILES\|SCRIPTS$DIR\).*\1' ./usr.sbin/chown/Makefile:LINKS= ${BINDIR}/chown /usr/bin/chgrp

skhal · 2018-03-21T16:02:34Z

@stesser , very good point. I updated the PR with LINKS > SYMLINKS:

To keep history clean:

commit is amended
branch re-pushed

skhal · 2018-03-21T16:12:11Z

I assume symlink may not work either as some scripts may try to resolve it or check if chgrp is an actual file.

We could simply copy the executable in this case (the executable is ~10KB):

% ( eval $(stat -s /usr/sbin/chown); echo $st_size)
10352

stesser · 2018-03-21T18:22:44Z

Am 21.03.18 um 17:12 schrieb Samvel Khalatyan:

I assume symlink may not work either as some scripts may try to resolve it or check if |chgrp| is an actual file.

Possible but not likely. I've been using the sym-link for some 10 years without problems. But you've got a point: A script could perform such tests, thought I do not see for what purpose.

We could simply copy the executable in this case (the executable is ~10KB): % ( eval $(stat -s /usr/sbin/chown); echo $st_size) 10352

The size is 19352 bytes on my amd64 system (typo: 0 instead of 9?). If LINKS was defined to try the hard link and create a copy if that fails, the copy could be made conditional on the /usr/bin and /usr/sbin being on different file systems ... Regards, STefan

skhal · 2018-03-21T18:45:43Z

Thanks for sharing the feedback.

The size is 10K (there is no typo). I run a FreeBSD virtual machine in VirtualBox:

% uname -a
FreeBSD freebsd.skhal.net 11.1-RELEASE FreeBSD 11.1-RELEASE #0 r321309: Fri Jul 21 02:08:28 UTC 2017     root@releng2.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC  amd64

cemeyer · 2018-03-29T14:58:19Z

Since small size is not an object, why use a separate FS for /usr/bin and /usr/sbin? On my CURRENT system, /usr/bin is 203MB and /usr/sbin is only 19MB more. What's the compelling motivation? Thanks.

stesser · 2018-03-29T15:32:24Z

Am 29.03.18 um 16:58 schrieb Conrad Meyer:

Since small size is not an object, why use a separate FS for /usr/bin and /usr/sbin? On my CURRENT system, /usr/bin is 203MB and /usr/sbin is only 19MB more. What's the compelling motivation? Thanks.

This is on ZFS, and there is one file system for "/", which sets defaults for attributes inherited by file systems within. If you want to specify different behavior for directories below "/", you need one ZFS file system per such directory, e.g. you have separate file systems for each of /bin, /etc, /lib, /sbin, /var, ... This allows to set e.g. selectively select compression algorithms (or no compression), automatic snapshot policies and many more ... You could have /bin and /sbin in / (just do not create a file system for them), but then the attributes for /bin and /sbin must be applied to "/" and they are inherited by all file systems below (unless overridden in each of them). The most rational setup is to have "/" with default attributes that are to be inherited by all file systems below (as default values), and then separate file systems per directory in "/", each with its particular attribute values. If there was a way to create a ZFS file system and then to mount it at both /bin and /sbin, there was no problem at all. But having them in "/" means that e.g. the default attribute of "/" cannot be "exec=no" or "setuid=no".

cemeyer · 2018-03-29T15:56:36Z

Thanks. I reckon this is an unsupported "hardening" setup.

I agree the right thing for install to do if (hard)linking fails (due to cross-filesystem) is to just copy.

cemeyer · 2018-03-29T16:02:32Z

LINKS is executed in bsd.links.mk, using INSTALL_LINK. INSTALL_LINK is defined in own.mk as install -l h.

install -l h ... exits with 71 and prints "Cross-device link" in this situation. install -l m ("mixed") is pretty close to what we want — drop a symlink in the cross-filesystem case. But I'd instead suggest adding new modes to all of this. I'll propose a patch shortly.

brooksdavis · 2018-03-29T16:56:16Z

Please don't make the install dependent on the layout at install time. The install process should be as close to deterministic as possible and IMO should resemble tar xf foo as much as possible.

cemeyer · 2018-03-29T17:02:15Z

Does tar attempt to handle links? I agree with "as much as possible," but am unclear on how much is possible. I am also ok just closing this out as unsupported. Another option would be to just stop using LINKS entirely for these programs, which reside in separate directories.

cemeyer · 2018-03-29T17:05:17Z

Here's an untested proposal: https://reviews.freebsd.org/D14897

brooksdavis · 2018-03-29T17:39:46Z

tar does support hard links. I don't know how it handles crossing filesystem boundaries. We probably ought to find out if chgrp and chown are going to be in the same package...

emaste · 2021-06-02T21:14:19Z

@brooksdavis I found that both BSD tar and GNU tar fail to create the link in this case. As far as I can tell this issue existed before hard link support was added to xinstall and used by the share/mk infrastructure, so no regression. I agree though that trying a link and falling back to copy if it fails is reasonable.

brooksdavis · 2021-06-04T17:47:47Z

IMO this most consistent option is to make one of them unconditionally a relative symlink. Then there's no target-dependent difference.

Given this is an extremely uncommon configuration, I'd be somewhat tempted to add a WITH(OUT)_ option to enable whatever behavior we choose rather than making it automatic. If we don't have a way to do it globally we end up with weird asymmetry depending how you populate the system.

bsdimp · 2021-06-23T16:55:45Z

Is there some action that can be done to move forward this pull request? @brooksdavis @emaste can we come to some conclusion about what to do? I see @cemeyer has abandoned his proposal, so that's not an option. If not, we should close this pull request.

emaste · 2021-08-04T14:40:13Z

I think @cemeyer's proposal is decent, but will need someone to push it forward.

stesser · 2021-08-04T15:12:27Z

Am 23.06.21 um 18:55 schrieb Warner Losh:

Is there some action that can be done to move forward this pull request? @brooksdavis <https://github.com/brooksdavis> @emaste <https://github.com/emaste> can we come to some conclusion about what to do? I see @cemeyer <https://github.com/cemeyer> has abandoned his proposal, so that's not an option. If not, we should close this pull request.

I had split ZFS file systems for /usr/bin and /usr/sbin, but have since reduced the number of files systems and merged those two back to /usr. One solution might be to just use "install -lmr" instead of "ln". That would create a hard link if possible, a relative symlink else, and all this is done with the logic already implemented in the install command. Regards, STefan

emaste · 2022-04-26T16:55:56Z

One solution might be to just use "install -lmr" instead of "ln".

It's already using install:

bsd.own.mk:

HRDLINK?=       -l h -o ${_LINKOWN} -g ${_LINKGRP} -m ${_LINKMODE}
MANHRDLINK?=    -l h -o ${MANOWN} -g ${MANGRP} -m ${MANMODE}
SYMLINK?=       -l s -o ${_SYMLINKOWN} -g ${_SYMLINKGRP} -m ${_SYMLINKMODE}
LSYMLINK?=      -l s -o ${LIBOWN} -g ${LIBGRP} -m ${LIBMODE}
RSYMLINK?=      -l rs -o ${_SYMLINKOWN} -g ${_SYMLINKGRP} -m ${_SYMLINKMODE}

INSTALL_LINK?=          ${INSTALL} ${HRDLINK}
INSTALL_MANLINK?=       ${INSTALL} ${MANHRDLINK}
INSTALL_SYMLINK?=       ${INSTALL} ${SYMLINK}
INSTALL_LIBSYMLINK?=    ${INSTALL} ${LSYMLINK}
INSTALL_RSYMLINK?=      ${INSTALL} ${RSYMLINK}

bsd.links.mk:

.for s t in ${LINKS}
        ${INSTALL_LINK} ${TAG_ARGS} ${DESTDIR}${s} ${DESTDIR}${t}
.endfor
.for s t in ${SYMLINKS}
        ${INSTALL_SYMLINK} ${TAG_ARGS} ${s} ${DESTDIR}${t}
.endfor

We could either use -mr unconditionally for ${INSTALL_LINK} or add a new ${MIXEDLINKS}

bsdimp · 2023-02-04T18:16:35Z

usr.sbin/chown/Makefile

-LINKS=	${BINDIR}/chown /usr/bin/chgrp
-MAN=	chgrp.1 chown.8
+PROG=	    chown
+SYMLINKS=	${BINDIR}/chown ${BINDIR}/chgrp


We'd likely need to have this line as well

SYMLINKS+= ${BINDIR}/chown /usr/bin/chgrp

to ensure maximum compatibility. With that, I'd think the change would be fine

Thinking more about it, I think the simplest thing would be to change LINKS to SYMLINKS and leave the directory where it is.

Too bad this doesn't work.

bsdimp · 2023-02-08T21:22:26Z

diff --git a/usr.sbin/chown/Makefile b/usr.sbin/chown/Makefile
index b9dff0785363..8bdcb4f00d26 100644
--- a/usr.sbin/chown/Makefile
+++ b/usr.sbin/chown/Makefile
@@ -4,7 +4,7 @@
 .include <src.opts.mk>

 PROG=  chown
-LINKS= ${BINDIR}/chown /usr/bin/chgrp
+SYMLINKS= ${BINDIR}/chown ../bin/chgrp
 MAN=   chgrp.1 chown.8

 HAS_TESTS=

seems the simplest approach. Neither of these binaries is used often, especially chgrp, so the overhead of a symlink would be in the noise.

bsdimp · 2023-02-08T21:24:39Z

https://reviews.freebsd.org/D38456 has my take on what we should do... and if there's no consensus on this round, I'm going to close the PR.

bsdimp · 2023-02-27T17:21:54Z

55f35c5 has ed's suggestion. I'm closing this now. We're done.
further development of this issue should be done in new PRs (for code that's ready) or discussed elsewhere.

emaste suggested this in #133 and it works. It will do the right thing by default, and handle the weird cases 133 was trying to fix.

emaste suggested this in freebsd/freebsd-src#133 and it works. It will do the right thing by default, and handle the weird cases 133 was trying to fix.

bsdimp reviewed Feb 4, 2023

View reviewed changes

bsdimp closed this Feb 27, 2023

freebsd-git pushed a commit that referenced this pull request Feb 27, 2023

Use 'mixed' links for 'hard' links by default.

55f35c5

emaste suggested this in #133 and it works. It will do the right thing by default, and handle the weird cases 133 was trying to fix.

emaste added the merged label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chown: link to chgrp in the same bin folder #133

chown: link to chgrp in the same bin folder #133

skhal commented Mar 21, 2018

stesser commented Mar 21, 2018

skhal commented Mar 21, 2018

skhal commented Mar 21, 2018

stesser commented Mar 21, 2018 via email

skhal commented Mar 21, 2018

cemeyer commented Mar 29, 2018

stesser commented Mar 29, 2018 via email

cemeyer commented Mar 29, 2018

cemeyer commented Mar 29, 2018

brooksdavis commented Mar 29, 2018

cemeyer commented Mar 29, 2018 •

edited

cemeyer commented Mar 29, 2018

brooksdavis commented Mar 29, 2018

emaste commented Jun 2, 2021

brooksdavis commented Jun 4, 2021

bsdimp commented Jun 23, 2021

emaste commented Aug 4, 2021

stesser commented Aug 4, 2021 via email

emaste commented Apr 26, 2022

bsdimp Feb 4, 2023

bsdimp Feb 8, 2023

bsdimp Feb 27, 2023

bsdimp commented Feb 8, 2023

bsdimp commented Feb 8, 2023

bsdimp commented Feb 27, 2023

chown: link to chgrp in the same bin folder #133

chown: link to chgrp in the same bin folder #133

Conversation

skhal commented Mar 21, 2018

stesser commented Mar 21, 2018

skhal commented Mar 21, 2018

skhal commented Mar 21, 2018

stesser commented Mar 21, 2018 via email

skhal commented Mar 21, 2018

cemeyer commented Mar 29, 2018

stesser commented Mar 29, 2018 via email

cemeyer commented Mar 29, 2018

cemeyer commented Mar 29, 2018

brooksdavis commented Mar 29, 2018

cemeyer commented Mar 29, 2018 • edited

cemeyer commented Mar 29, 2018

brooksdavis commented Mar 29, 2018

emaste commented Jun 2, 2021

brooksdavis commented Jun 4, 2021

bsdimp commented Jun 23, 2021

emaste commented Aug 4, 2021

stesser commented Aug 4, 2021 via email

emaste commented Apr 26, 2022

bsdimp Feb 4, 2023

Choose a reason for hiding this comment

bsdimp Feb 8, 2023

Choose a reason for hiding this comment

bsdimp Feb 27, 2023

Choose a reason for hiding this comment

bsdimp commented Feb 8, 2023

bsdimp commented Feb 8, 2023

bsdimp commented Feb 27, 2023

cemeyer commented Mar 29, 2018 •

edited