Re: [PATCH] Avoid concurrency problem in ldconfig (bug 23973)

[Florian Weimer <fweimer at redhat dot com>]

* Andreas Schwab:

> On Apr 18 2019, Florian Weimer <fweimer@redhat.com> wrote:
>
>> * Andreas Schwab:
>>
>>> On Apr 18 2019, Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>>> * Andreas Schwab:
>>>>
>>>>> Use a unique name for the temporary file when updating the ld.so cache, so
>>>>> that two concurrent runs of ldconfig don't write to the same file.
>>>>>
>>>>> 	* elf/cache.c (save_cache): Use unique temporary name.
>>>>> 	(save_aux_cache): Likewise.
>>>>
>>>> The downside of this change is that if ldconfig is interrupted, the
>>>> temporary file never goes away.
>>>>
>>>> Ideally, we would use O_TMPFILE if supported by the (file) system, but
>>>> that can get quite involved.
>>>
>>> That shortens the window, but won't close it, since we need a name to
>>> pass to rename in any case.
>>
>> The difference is that with a fixed name, the next ldconfig run will use
>> the same name and rename, cleaning up.  With a random, unique name, that
>> automatic cleanup is gone.
>
> O_TMPFILE won't change that.

Hmm.  I thought it would, but it does not.

It's possible to avoid writing to the fixed name though, like this:

#include <err.h>
#include <errno.h>
#include <fcntl.h>
#include <libgen.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unistd.h>

int
main (int argc, char **argv)
{
  if (argc != 2)
    {
      fprintf (stderr, "usage: %s PATH\n", argv[0]);
      exit (1);
    }

  const char *path = argv[1];
  char *dir_storage = strdup (path);
  if (dir_storage == NULL)
    err (1, "strdup");
  char *dir = dirname (dir_storage);

  int fd = open64 (dir, O_TMPFILE | O_RDWR, 0700);
  if (fd < 0)
    err (1, "open (\"%s\"., O_TMPFILE | O_RDWR)", dir);

  time_t t = time (NULL);
  const char *data = asctime (gmtime (&t));
  if (write (fd, data, strlen (data)) != strlen (data))
    errx (1, "write failure");

  if (fsync (fd) != 0)
    err (1, "fsync");

  char *proc_fd_path;
  if (asprintf (&proc_fd_path, "/proc/self/fd/%d", fd) < 0)
    err (1, "asprintf");

  if (linkat (AT_FDCWD, proc_fd_path, AT_FDCWD, path, AT_SYMLINK_FOLLOW) == 0)
    {
      /* Nothing to do.  */
    }
  else
    {
      char *tmp_path;
      if (asprintf (&tmp_path, "%s~", path) < 0)
        err (1, "asprintf");

      while (true)
        if (linkat (AT_FDCWD, proc_fd_path, AT_FDCWD,
                    tmp_path, AT_SYMLINK_FOLLOW) == 0)
          break;
        else
          {
            if (errno == EEXIST)
              {
                if (unlink (tmp_path) != 0)
                  if (errno != ENOENT)
                    err (1, "unlink (\"%s\")", tmp_path);
                continue;
              }
            else
              err (1, "linkat");
          }

      if (renameat2 (AT_FDCWD, tmp_path, AT_FDCWD, path, RENAME_EXCHANGE) != 0)
        err (1, "renameat2");
      if (unlink (tmp_path) != 0)
        if (errno != ENOENT)
          err (1, "unlink (\"%s\")", tmp_path);

      free (tmp_path);
    }

  free (proc_fd_path);

  if (close (fd) != 0)
    err (1, "close");

  free (dir);

  return 0;
}

This is unfortunately way more complicated than it should be.

Thanks,
Florian