chown: link to chgrp in the same bin folder

[skhal]

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]

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).

[skhal]

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

To keep history clean:

• commit is amended
• branch re-pushed

[skhal]

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]

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]

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     [email protected]:/usr/obj/usr/src/sys/GENERIC  amd64 

[cemeyer]

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]

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]

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]

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]

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]

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]

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

[brooksdavis]

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]

@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]

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]

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]

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

[stesser]

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]

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]

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]

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]

55f35c5398ff 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.