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@xxxxxxxxx> wrote:
>> On Mon, Dec 10, 2018 at 2:03 PM Michal Suchanek <msuchanek@xxxxxxx> 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@xxxxxxx>
>> > ---
>> > 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.
```
--
WBR,
Yauheni Kaliuta
