Re: [PATCH v2 2/3] depmod: prevent module dependency files corruption due to parallel invocation.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Lucas De Marchi

>                         fd = openat(dfd, tmp, flags, mode);
>                         if (fd < 0) {
>                                 ERR("openat(%s, %s, %o, %o): %m\n",
> --
> 2.19.2
>


-- 
Lucas De Marchi



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux