From: "Michal Suchánek" <msuchanek@suse.de>
To: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
Cc: Lucas De Marchi <lucas.de.marchi@gmail.com>,
linux-modules <linux-modules@vger.kernel.org>
Subject: Re: [PATCH v2 2/3] depmod: prevent module dependency files corruption due to parallel invocation.
Date: Tue, 18 Dec 2018 11:26:14 +0100 [thread overview]
Message-ID: <20181218112614.07242b2e@naga> (raw)
In-Reply-To: <xunyd0pzl49t.fsf@redhat.com>
On Tue, 18 Dec 2018 08:47:58 +0200
Yauheni Kaliuta <yauheni.kaliuta@redhat.com> wrote:
> Hi, Michal!
>
> >>>>> On Mon, 17 Dec 2018 23:07:43 +0100, Michal Suchánek wrote:
>
> > On Mon, 17 Dec 2018 10:10:37 -0800
> > Lucas De Marchi <lucas.de.marchi@gmail.com> wrote:
>
> >> On Mon, Dec 10, 2018 at 2:03 PM Michal Suchanek <msuchanek@suse.de> wrote:
> >> >
> >> > Depmod does not use unique filename for temporary files. There is no
> >> > guarantee the user does not attempt to run mutiple depmod processes in
> >> > parallel. If that happens a temporary file might be created by
> >> > depmod(1st), truncated by depmod(2nd), and renamed to final name by
> >> > depmod(1st) resulting in corrupted file seen by user.
> >> >
> >> > Due to missing mkstempat() this is more complex than it should be.
> >> > Adding PID and timestamp to the filename should be reasonably reliable.
> >> > Adding O_EXCL as mkstemp does fails creating the file rather than
> >> > corrupting existing file.
> >> >
> >> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> >> > ---
> >> > tools/depmod.c | 12 +++++++++---
> >> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/tools/depmod.c b/tools/depmod.c
> >> > index 18c0d61b2db3..3b6d16e76160 100644
> >> > --- a/tools/depmod.c
> >> > +++ b/tools/depmod.c
> >> > @@ -29,6 +29,7 @@
> >> > #include <string.h>
> >> > #include <unistd.h>
> >> > #include <sys/stat.h>
> >> > +#include <sys/time.h>
> >> > #include <sys/utsname.h>
> >> >
> >> > #include <shared/array.h>
> >> > @@ -2398,6 +2399,9 @@ static int depmod_output(struct depmod *depmod, FILE *out)
> >> > };
> >> > const char *dname = depmod->cfg->dirname;
> >> > int dfd, err = 0;
> >> > + struct timeval tv;
> >> > +
> >> > + gettimeofday(&tv, NULL);
> >> >
> >> > if (out != NULL)
> >> > dfd = -1;
> >> > @@ -2412,15 +2416,17 @@ static int depmod_output(struct depmod *depmod, FILE *out)
> >> >
> >> > for (itr = depfiles; itr->name != NULL; itr++) {
> >> > FILE *fp = out;
> >> > - char tmp[NAME_MAX] = "";
> >> > + char tmp[NAME_MAX + 1] = "";
> >> > int r, ferr;
> >> >
> >> > if (fp == NULL) {
> >> > - int flags = O_CREAT | O_TRUNC | O_WRONLY;
> >> > + int flags = O_CREAT | O_TRUNC | O_WRONLY | O_EXCL;
> >> > int mode = 0644;
> >> > int fd;
> >> >
> >> > - snprintf(tmp, sizeof(tmp), "%s.tmp", itr->name);
> >> > + snprintf(tmp, sizeof(tmp), "%s.%i.%li.%li", itr->name, getpid(),
> >> > + tv.tv_usec, tv.tv_sec);
> >> > + tmp[NAME_MAX] = 0;
> >>
> >> why? snprintf is guaranteed to nul-terminate the string.
> >>
> > AFAIK it is guaranteed to not write after end of buffer. It is
> > not guaranteed to terminate the string. To guarantee
> > terminated string you need large enough buffer or a nul after
> > the buffer. Or asprintf.
>
> Actually, it is. VS strncpy(). May be it is not so clear from the
> man page:
>
> ```
> The functions snprintf() and vsnprintf() write at most size
> bytes (including the terminating null byte ('\0')) to str.
> ```
>
Yes, that's it. The POSIX page is much clearer:
>> The snprintf() function shall be equivalent to sprintf(), with the
>> addition of the n argument which states the size of the buffer referred
>> to by s. If n is zero, nothing shall be written and s may be a null
>> pointer. Otherwise, output bytes beyond the n‐1st shall be
>> discarded instead of being written to the array, and a null byte is
>> written at the end of the bytes actually written into the array.
Thanks
Michal
next prev parent reply other threads:[~2018-12-18 10:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-07 15:45 [PATCH] depmod: Prevent module dependency files corruption due to parallel invocation Michal Suchanek
2018-12-10 21:29 ` [PATCH v2 0/3] Fix dependency file corruption with parallel depmod invocation Michal Suchanek
2018-12-10 21:29 ` [PATCH v2 1/3] depmod: prevent module dependency files missing during " Michal Suchanek
2018-12-10 21:29 ` [PATCH v2 2/3] depmod: prevent module dependency files corruption due to parallel invocation Michal Suchanek
2018-12-17 18:10 ` Lucas De Marchi
2018-12-17 22:07 ` Michal Suchánek
2018-12-18 6:47 ` Yauheni Kaliuta
2018-12-18 10:26 ` Michal Suchánek [this message]
2018-12-10 21:29 ` [PATCH v2 3/3] depmod: shut up gcc insufficinet buffer warning Michal Suchanek
2018-12-17 18:19 ` Lucas De Marchi
2018-12-17 22:46 ` [PATCH v3 0/3] Fix dependency file corruption with parallel depmod invocation Michal Suchanek
2018-12-17 22:46 ` [PATCH v3 1/3] depmod: prevent module dependency files missing during " Michal Suchanek
2018-12-17 22:46 ` [PATCH v3 2/3] depmod: prevent module dependency files corruption due to parallel invocation Michal Suchanek
2018-12-17 22:46 ` [PATCH v3 3/3] depmod: shut up gcc insufficinet buffer warning Michal Suchanek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181218112614.07242b2e@naga \
--to=msuchanek@suse.de \
--cc=linux-modules@vger.kernel.org \
--cc=lucas.de.marchi@gmail.com \
--cc=yauheni.kaliuta@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).