netdev - Re: [PATCH iproute2] testsuite: don't clobber /tmp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGnkfhyS64WA+947iQFwA9+=yS6Zk856SWBR9Zy7w90xhBmC=Q@mail.gmail.com>
Date:   Thu, 13 Jun 2019 19:15:01 +0200
From:   Matteo Croce <mcroce@...hat.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev <netdev@...r.kernel.org>, David Ahern <dsahern@...nel.org>
Subject: Re: [PATCH iproute2] testsuite: don't clobber /tmp

On Wed, Jun 12, 2019 at 8:20 PM Stephen Hemminger
<stephen@...workplumber.org> wrote:
>
> On Wed, 12 Jun 2019 19:32:29 +0200
> Matteo Croce <mcroce@...hat.com> wrote:
>
> > On Wed, Jun 12, 2019 at 6:04 PM Matteo Croce <mcroce@...hat.com> wrote:
> > >
> > > On Wed, Jun 12, 2019 at 5:55 PM Stephen Hemminger
> > > <stephen@...workplumber.org> wrote:
> > > >
> > > > On Tue, 11 Jun 2019 20:03:26 +0200
> > > > Matteo Croce <mcroce@...hat.com> wrote:
> > > >
> > > > > Even if not running the testsuite, every build will leave
> > > > > a stale tc_testkenv.* file in the system temp directory.
> > > > > Conditionally create the temp file only if we're running the testsuite.
> > > > >
> > > > > Signed-off-by: Matteo Croce <mcroce@...hat.com>
> > > > > ---
> > > > >  testsuite/Makefile | 5 ++++-
> > > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/testsuite/Makefile b/testsuite/Makefile
> > > > > index 7f247bbc..5353244b 100644
> > > > > --- a/testsuite/Makefile
> > > > > +++ b/testsuite/Makefile
> > > > > @@ -14,7 +14,9 @@ TESTS_DIR := $(dir $(TESTS))
> > > > >
> > > > >  IPVERS := $(filter-out iproute2/Makefile,$(wildcard iproute2/*))
> > > > >
> > > > > -KENVFN := $(shell mktemp /tmp/tc_testkenv.XXXXXX)
> > > > > +ifeq ($(MAKECMDGOALS),alltests)
> > > > > +     KENVFN := $(shell mktemp /tmp/tc_testkenv.XXXXXX)
> > > > > +endif
> > > > >  ifneq (,$(wildcard /proc/config.gz))
> > > > >       KCPATH := /proc/config.gz
> > > > >  else
> > > > > @@ -94,3 +96,4 @@ endif
> > > > >               rm "$$TMP_ERR" "$$TMP_OUT"; \
> > > > >               sudo dmesg > $(RESULTS_DIR)/$@...o.dmesg; \
> > > > >       done
> > > > > +     @$(RM) $(KENVFN)
> > > >
> > > > My concern is that there are several targets in this one Makefile.
> > > >
> > > > Why not use -u which gives name but does not create the file?
> > >
> > > As the manpage says, this is unsafe, as a file with the same name can
> > > be created in the meantime.
> > > Another option is to run the mktemp in the target shell, but this will
> > > require to escape every single end of line to make it a single shell
> > > command, e.g.:
> > >
> > >         KENVFN=$$(mktemp /tmp/tc_testkenv.XXXXXX); \
> > >         if [ "$(KCPATH)" = "/proc/config.gz" ]; then \
> > >                 gunzip -c $(KCPATH) >$$KENVFN; \
> > >         ...
> > >         done ; \
> > >         $(RM) $$KENVFN
> > >
> > > --
> > > Matteo Croce
> > > per aspera ad upstream
> >
> > Anyway, looking for "tc" instead of "alltests" is probably better, as
> > it only runs mktemp when at least the tc test is selected, both
> > manually or via make check from topdir, eg.g
> >
> > ifeq ($(MAKECMDGOALS),tc)
> >
> > Do you agree?
>
> Why use /tmp at all for this config file?

To me any path could work, both /tmp or in the current dir, I have no
preference.
The important thing is to remove them wherever they are, as clobbering
the build dir is bad as messing /tmp.

Anyway, I double checked, and the only target which uses that
temporary file is 'alltests' so, if the path is ok, I think that the
condition "ifeq ($(MAKECMDGOALS),alltests)" is the only one which
fixes the issue and keeps the behaviour unaltered.
I did some quick tests and it works for me.

Bye,
-- 
Matteo Croce
per aspera ad upstream