On 19/03/2026 02:23, Collin Funk wrote:
Collin Funk <[email protected]> writes:
Pádraig Brady <[email protected]> writes:
On 18/03/2026 06:09, Collin Funk wrote:
I noticed that 'yes' did not use pipe2_safer to check that the file
descriptors aren't any of the standard file descriptors. This fixes
that and some similar cases in other programs.
Is it worth a NEWS mention? I assume that it is difficult to reach a
point where pipe or pipe2 would open a standard file descriptor in
these areas, give that this hasn't been reported as a bug.
Oh very good spot.
I did check yes(1) with closed stdout,
with and without the initial probing write().
But it's best avoid these variances.
I see we have sc_require_{stdio,stdlib}_safer in cfg.mk.
We probably should have a unistd variant also.
This patch did add that. :)
I pushed it. Thanks for the review.
Shouldn't the same go for fcntl--.h for open, openat, and creat?
Here is the proposed rule:
diff --git a/cfg.mk b/cfg.mk
index aa2c86e23..22d0d1f2d 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -579,6 +579,19 @@ sc_prohibit_short_facl_mode_spec:
halt='setfacl mode string length < 3; extend with hyphen(s)' \
$(_sc_search_regexp)
+# Ensure that "fcntl--.h" is used where appropriate.
+sc_require_fcntl_safer:
+ @if $(VC_LIST_EXCEPT) | grep -l '\.[ch]$$' > /dev/null; then \
+ files=$$(grep -El '$(begword)(open(at)?|creat) ?\(' \
+ $$($(VC_LIST_EXCEPT) \
+ | grep '\.[ch]$$')); \
+ test -n "$$files" && grep -LE 'include "fcntl--.h"' $$files \
+ | grep . && \
+ { echo '$(ME): the above files should use "fcntl--.h"' \
+ 1>&2; exit 1; } || :; \
+ else :; \
+ fi
+
# Ensure that "stdio--.h" is used where appropriate.
sc_require_stdio_safer:
@if $(VC_LIST_EXCEPT) | grep -l '\.[ch]$$' > /dev/null; then \
And the list of files it catches:
require_fcntl_safer
gl/lib/fd-reopen.c
By its nature the above wouldn't need this.
gl/lib/targetdir.c
src/cat.c
src/chown-core.c
src/df.c
src/head.c
src/install.c
src/nohup.c
src/pinky.c
src/pr.c
src/selinux.c
src/sort.c
src/sync.c
src/system.h
src/tac.c
src/truncate.c
src/wc.c
src/who.c
maint.mk: the above files should use "fcntl--.h"
make: *** [cfg.mk:584: sc_require_fcntl_safer] Error 1
I'm not sure if it's best to always use the safer variants.
If so then perhaps we should disallow use of stdio.h etc.
and always direct to use stdio--.h?
A very quick scan shows a possible problematic usage in pinky
where open("/dev/") is not followed by a close(), but then
we may do a write(STDOUT_FILENO), possibly resulting in confusing errors.
I notice that ln.c explicitly uses openat_safer()
rather than relying on the fcntl--.h mapping
I also notice that our current syntax checks don't check all functions.
E.g. we only check for freopen() use with stdio.h, but not fopen(),
(or popen() or tmpfile() which we don't use).
Paul would know better than I about this.
cheers,
Padraig
