On Wed, Jul 24, 2019 at 3:53 PM Rob Landley <[email protected]> wrote: > > On 7/23/19 11:50 PM, enh via Toybox wrote: > > static void do_tac(int fd, char *name) > > { > > struct arg_list *list = NULL; > > - char *c; > > + FILE *fp; > > > > - // Read in lines > > + if (fd == -1) { > > + perror_msg_raw(name); > > + return; > > + } > > The infrastructure does that if you pass WARN_ONLY in the flags. (Which > loopfiles() does by default.) > > Don't close stdin when they -, it makes the rest of your pipeline unhappy. > (do_lines() in lib/ checks for this)... > > Actually, lemme rewrite this to use do_lines(), via the loopfiles_lines() > bridge > that's already in lib. And use a dlist since that already has a function to > allocate the wrapper for us and it's trivially reversible... > > There, checked in on top of yours. Try now?
even better, thanks. i did realize though that the no-O_CLOEXEC bit is actually missing from loopfiles_lines itself. see attached patch. (one day we'll catch these automatically, but we're not in enforcing mode yet so this one was just by inspection. see https://android.googlesource.com/platform/bionic/+/master/docs/fdsan.md for details of the sanitizer that's in Android Q. this double close isn't an actual problem here, where nothing runs in between fclose() and close(), but it's a common problem in long-lived multi-threaded processes, which is basically everything these days.) > Rob
From d8f5920f2e8b0fbb5818c19d6ce4f1b74c1f680a Mon Sep 17 00:00:00 2001 From: Elliott Hughes <[email protected]> Date: Wed, 24 Jul 2019 16:00:58 -0700 Subject: [PATCH] Avoid double-close of fd in loopfiles_lines. Test: no EBADF in `strace -e close ./toybox tac /proc/version` --- lib/lib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/lib.c b/lib/lib.c index 47e5ca21..53962a59 100644 --- a/lib/lib.c +++ b/lib/lib.c @@ -694,7 +694,8 @@ static void loopfile_lines_bridge(int fd, char *name) void loopfiles_lines(char **argv, void (*function)(char **pline, long len)) { do_lines_bridge = function; - loopfiles(argv, loopfile_lines_bridge); + // No O_CLOEXEC because we need to call fclose. + loopfiles_rw(argv, O_RDONLY|WARN_ONLY, 0, loopfile_lines_bridge); } // Slow, but small. -- 2.22.0.657.g960e92d24f-goog
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
