* [PATCH] script: close file descriptors on exec
@ 2016-04-14 20:22 Sami Kerola
2016-04-15 7:22 ` Ruediger Meier
0 siblings, 1 reply; 5+ messages in thread
From: Sami Kerola @ 2016-04-14 20:22 UTC (permalink / raw)
To: util-linux; +Cc: Sami Kerola
The commands spawned from script(1) will never need access various file
descriptors the script(1) is using.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
term-utils/script.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/term-utils/script.c b/term-utils/script.c
index 1199742..88b522a 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -419,15 +419,15 @@ static void do_io(struct script_control *ctl)
};
- if ((ctl->typescriptfp = fopen(ctl->fname, ctl->append ? "a" : "w")) == NULL) {
+ if ((ctl->typescriptfp = fopen(ctl->fname, ctl->append ? "ae" : "we")) == NULL) {
warn(_("cannot open %s"), ctl->fname);
fail(ctl);
}
if (ctl->timing) {
if (!ctl->tname) {
- if (!(ctl->timingfp = fopen("/dev/stderr", "w")))
+ if (!(ctl->timingfp = fopen("/dev/stderr", "we")))
err(EXIT_FAILURE, _("cannot open %s"), "/dev/stderr");
- } else if (!(ctl->timingfp = fopen(ctl->tname, "w")))
+ } else if (!(ctl->timingfp = fopen(ctl->tname, "we")))
err(EXIT_FAILURE, _("cannot open %s"), ctl->tname);
}
@@ -515,7 +515,7 @@ static void getslave(struct script_control *ctl)
{
#ifndef HAVE_LIBUTIL
ctl->line[strlen("/dev/")] = 't';
- ctl->slave = open(ctl->line, O_RDWR);
+ ctl->slave = open(ctl->line, O_RDWR | O_CLOEXEC);
if (ctl->slave < 0) {
warn(_("cannot open %s"), ctl->line);
fail(ctl);
@@ -625,7 +625,7 @@ static void getmaster(struct script_control *ctl)
break;
for (cp = "0123456789abcdef"; *cp; cp++) {
*pty = *cp;
- ctl->master = open(ctl->line, O_RDWR);
+ ctl->master = open(ctl->line, O_RDWR | O_CLOEXEC);
if (ctl->master >= 0) {
char *tp = &ctl->line[strlen("/dev/")];
int ok;
@@ -765,7 +765,7 @@ int main(int argc, char **argv)
* handled according to their default dispositions */
sigprocmask(SIG_BLOCK, &ctl.sigset, &ctl.sigorg);
- if ((ctl.sigfd = signalfd(-1, &ctl.sigset, 0)) < 0)
+ if ((ctl.sigfd = signalfd(-1, &ctl.sigset, SFD_CLOEXEC)) < 0)
err(EXIT_FAILURE, _("cannot set signal handler"));
DBG(SIGNAL, ul_debug("signal fd=%d", ctl.sigfd));
--
2.8.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] script: close file descriptors on exec
2016-04-14 20:22 [PATCH] script: close file descriptors on exec Sami Kerola
@ 2016-04-15 7:22 ` Ruediger Meier
2016-04-15 8:30 ` Karel Zak
0 siblings, 1 reply; 5+ messages in thread
From: Ruediger Meier @ 2016-04-15 7:22 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Thursday 14 April 2016, Sami Kerola wrote:
> The commands spawned from script(1) will never need access various
> file descriptors the script(1) is using.
>
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
> term-utils/script.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/term-utils/script.c b/term-utils/script.c
> index 1199742..88b522a 100644
> --- a/term-utils/script.c
> +++ b/term-utils/script.c
> @@ -419,15 +419,15 @@ static void do_io(struct script_control *ctl)
> };
>
>
> - if ((ctl->typescriptfp = fopen(ctl->fname, ctl->append ? "a" :
> "w")) == NULL) { + if ((ctl->typescriptfp = fopen(ctl->fname,
> ctl->append ? "ae" : "we")) == NULL) { warn(_("cannot open %s"),
> ctl->fname);
> fail(ctl);
> }
> if (ctl->timing) {
> if (!ctl->tname) {
> - if (!(ctl->timingfp = fopen("/dev/stderr", "w")))
> + if (!(ctl->timingfp = fopen("/dev/stderr", "we")))
> err(EXIT_FAILURE, _("cannot open %s"), "/dev/stderr");
> - } else if (!(ctl->timingfp = fopen(ctl->tname, "w")))
> + } else if (!(ctl->timingfp = fopen(ctl->tname, "we")))
"e" works for glibc only.
> err(EXIT_FAILURE, _("cannot open %s"), ctl->tname);
> }
>
> @@ -515,7 +515,7 @@ static void getslave(struct script_control *ctl)
> {
> #ifndef HAVE_LIBUTIL
> ctl->line[strlen("/dev/")] = 't';
> - ctl->slave = open(ctl->line, O_RDWR);
> + ctl->slave = open(ctl->line, O_RDWR | O_CLOEXEC);
> if (ctl->slave < 0) {
> warn(_("cannot open %s"), ctl->line);
> fail(ctl);
> @@ -625,7 +625,7 @@ static void getmaster(struct script_control *ctl)
> break;
> for (cp = "0123456789abcdef"; *cp; cp++) {
> *pty = *cp;
> - ctl->master = open(ctl->line, O_RDWR);
> + ctl->master = open(ctl->line, O_RDWR | O_CLOEXEC);
> if (ctl->master >= 0) {
> char *tp = &ctl->line[strlen("/dev/")];
> int ok;
> @@ -765,7 +765,7 @@ int main(int argc, char **argv)
> * handled according to their default dispositions */
> sigprocmask(SIG_BLOCK, &ctl.sigset, &ctl.sigorg);
>
> - if ((ctl.sigfd = signalfd(-1, &ctl.sigset, 0)) < 0)
> + if ((ctl.sigfd = signalfd(-1, &ctl.sigset, SFD_CLOEXEC)) < 0)
> err(EXIT_FAILURE, _("cannot set signal handler"));
>
> DBG(SIGNAL, ul_debug("signal fd=%d", ctl.sigfd));
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] script: close file descriptors on exec
2016-04-15 7:22 ` Ruediger Meier
@ 2016-04-15 8:30 ` Karel Zak
2016-04-15 8:42 ` Ruediger Meier
0 siblings, 1 reply; 5+ messages in thread
From: Karel Zak @ 2016-04-15 8:30 UTC (permalink / raw)
To: Ruediger Meier; +Cc: Sami Kerola, util-linux
On Fri, Apr 15, 2016 at 09:22:14AM +0200, Ruediger Meier wrote:
> On Thursday 14 April 2016, Sami Kerola wrote:
> > The commands spawned from script(1) will never need access various
> > file descriptors the script(1) is using.
> >
> > Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> > ---
> > term-utils/script.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/term-utils/script.c b/term-utils/script.c
> > index 1199742..88b522a 100644
> > --- a/term-utils/script.c
> > +++ b/term-utils/script.c
> > @@ -419,15 +419,15 @@ static void do_io(struct script_control *ctl)
> > };
> >
> >
> > - if ((ctl->typescriptfp = fopen(ctl->fname, ctl->append ? "a" :
> > "w")) == NULL) { + if ((ctl->typescriptfp = fopen(ctl->fname,
> > ctl->append ? "ae" : "we")) == NULL) { warn(_("cannot open %s"),
> > ctl->fname);
> > fail(ctl);
> > }
> > if (ctl->timing) {
> > if (!ctl->tname) {
> > - if (!(ctl->timingfp = fopen("/dev/stderr", "w")))
> > + if (!(ctl->timingfp = fopen("/dev/stderr", "we")))
> > err(EXIT_FAILURE, _("cannot open %s"), "/dev/stderr");
> > - } else if (!(ctl->timingfp = fopen(ctl->tname, "w")))
> > + } else if (!(ctl->timingfp = fopen(ctl->tname, "we")))
>
> "e" works for glibc only.
Yep, see c.h, should be
else if (!(ctl->timingfp = fopen(ctl->tname, "w" UL_CLOEXECSTR)))
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] script: close file descriptors on exec
2016-04-15 8:30 ` Karel Zak
@ 2016-04-15 8:42 ` Ruediger Meier
2016-04-16 15:26 ` Sami Kerola
0 siblings, 1 reply; 5+ messages in thread
From: Ruediger Meier @ 2016-04-15 8:42 UTC (permalink / raw)
To: Karel Zak; +Cc: Sami Kerola, util-linux
On Friday 15 April 2016, Karel Zak wrote:
> On Fri, Apr 15, 2016 at 09:22:14AM +0200, Ruediger Meier wrote:
> > On Thursday 14 April 2016, Sami Kerola wrote:
> > > The commands spawned from script(1) will never need access
> > > various file descriptors the script(1) is using.
> > >
> > > Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> > > ---
> > > term-utils/script.c | 12 ++++++------
> > > 1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/term-utils/script.c b/term-utils/script.c
> > > index 1199742..88b522a 100644
> > > --- a/term-utils/script.c
> > > +++ b/term-utils/script.c
> > > @@ -419,15 +419,15 @@ static void do_io(struct script_control
> > > *ctl) };
> > >
> > >
> > > - if ((ctl->typescriptfp = fopen(ctl->fname, ctl->append ? "a" :
> > > "w")) == NULL) { + if ((ctl->typescriptfp = fopen(ctl->fname,
> > > ctl->append ? "ae" : "we")) == NULL) { warn(_("cannot open %s"),
> > > ctl->fname);
> > > fail(ctl);
> > > }
> > > if (ctl->timing) {
> > > if (!ctl->tname) {
> > > - if (!(ctl->timingfp = fopen("/dev/stderr", "w")))
> > > + if (!(ctl->timingfp = fopen("/dev/stderr", "we")))
> > > err(EXIT_FAILURE, _("cannot open %s"), "/dev/stderr");
> > > - } else if (!(ctl->timingfp = fopen(ctl->tname, "w")))
> > > + } else if (!(ctl->timingfp = fopen(ctl->tname, "we")))
> >
> > "e" works for glibc only.
>
> Yep, see c.h, should be
>
> else if (!(ctl->timingfp = fopen(ctl->tname, "w" UL_CLOEXECSTR)))
Could be also fixed here:
login-utils/sulogin-consoles.c:163: if (!(fp = fopen(file, "re")))
login-utils/sulogin-consoles.c:364: fc = fopen("/proc/consoles", "re");
> Karel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] script: close file descriptors on exec
2016-04-15 8:42 ` Ruediger Meier
@ 2016-04-16 15:26 ` Sami Kerola
0 siblings, 0 replies; 5+ messages in thread
From: Sami Kerola @ 2016-04-16 15:26 UTC (permalink / raw)
To: Ruediger Meier; +Cc: Karel Zak, util-linux
On Fri, 15 Apr 2016, Ruediger Meier wrote:
> On Friday 15 April 2016, Karel Zak wrote:
> > On Fri, Apr 15, 2016 at 09:22:14AM +0200, Ruediger Meier wrote:
> > > On Thursday 14 April 2016, Sami Kerola wrote:
> > > > The commands spawned from script(1) will never need access
> > > > various file descriptors the script(1) is using.
> > > >
> > > > Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> > > > ---
> > > > term-utils/script.c | 12 ++++++------
> > > > 1 file changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/term-utils/script.c b/term-utils/script.c
> > > > index 1199742..88b522a 100644
> > > > --- a/term-utils/script.c
> > > > +++ b/term-utils/script.c
> > > > @@ -419,15 +419,15 @@ static void do_io(struct script_control
> > > > *ctl) };
> > > >
> > > >
> > > > - if ((ctl->typescriptfp = fopen(ctl->fname, ctl->append ? "a" :
> > > > "w")) == NULL) { + if ((ctl->typescriptfp = fopen(ctl->fname,
> > > > ctl->append ? "ae" : "we")) == NULL) { warn(_("cannot open %s"),
> > > > ctl->fname);
> > > > fail(ctl);
> > > > }
> > > > if (ctl->timing) {
> > > > if (!ctl->tname) {
> > > > - if (!(ctl->timingfp = fopen("/dev/stderr", "w")))
> > > > + if (!(ctl->timingfp = fopen("/dev/stderr", "we")))
> > > > err(EXIT_FAILURE, _("cannot open %s"), "/dev/stderr");
> > > > - } else if (!(ctl->timingfp = fopen(ctl->tname, "w")))
> > > > + } else if (!(ctl->timingfp = fopen(ctl->tname, "we")))
> > >
> > > "e" works for glibc only.
> >
> > Yep, see c.h, should be
> >
> > else if (!(ctl->timingfp = fopen(ctl->tname, "w" UL_CLOEXECSTR)))
>
> Could be also fixed here:
> login-utils/sulogin-consoles.c:163: if (!(fp = fopen(file, "re")))
> login-utils/sulogin-consoles.c:364: fc = fopen("/proc/consoles", "re");
Thank you for reviews Rudi and Karel. Fixes you asked are available in the
git repository at:
git://github.com/kerolasa/lelux-utiliteetit.git 2016wk15
https://github.com/kerolasa/lelux-utiliteetit/commit/3cd326de56993e90a646219dd810c55ab7809453
https://github.com/kerolasa/lelux-utiliteetit/commit/6c73b302c68ea422b7809fceb3afdb358e315c73
Since the later commit is new here it is as email in-line patch as well to
provide appropriate visibility for review.
--->8----
From: Sami Kerola <kerolasa@iki.fi>
Date: Sat, 16 Apr 2016 15:43:17 +0100
Subject: [PATCH] sulogin: make fopen O_CLOEXEC specifier usage portable
The close at exit specifier "e" is glibc extension, so use it only if when
the extension is available.
Proposed-by: Ruediger Meier <sweet_f_a@gmx.de>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
login-utils/sulogin-consoles.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/login-utils/sulogin-consoles.c b/login-utils/sulogin-consoles.c
index fe8eab1..4dcc77c 100644
--- a/login-utils/sulogin-consoles.c
+++ b/login-utils/sulogin-consoles.c
@@ -160,7 +160,7 @@ char *oneline(const char *file)
DBG(dbgprint("reading %s", file));
- if (!(fp = fopen(file, "re")))
+ if (!(fp = fopen(file, "r" UL_CLOEXECSTR)))
return NULL;
len = getline(&ret, &dummy, fp);
if (len >= 0) {
@@ -361,7 +361,7 @@ static int detect_consoles_from_proc(struct list_head *consoles)
DBG(dbgprint("trying /proc"));
- fc = fopen("/proc/consoles", "re");
+ fc = fopen("/proc/consoles", "r" UL_CLOEXECSTR);
if (!fc) {
rc = 2;
goto done;
--
2.8.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-04-16 15:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-14 20:22 [PATCH] script: close file descriptors on exec Sami Kerola
2016-04-15 7:22 ` Ruediger Meier
2016-04-15 8:30 ` Karel Zak
2016-04-15 8:42 ` Ruediger Meier
2016-04-16 15:26 ` Sami Kerola
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox