Re: Possible race condition in pg_mkdir_p()? - Mailing list pgsql-hackers

From Paul Guo
Subject Re: Possible race condition in pg_mkdir_p()?
Date
Msg-id CAEET0ZHVofSLgh_KvQDbRTYr=Ck3mWA11kOGsmHFA-w76qomiw@mail.gmail.com
Whole thread Raw
In response to Re: Possible race condition in pg_mkdir_p()?  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers


On Thu, Jul 18, 2019 at 4:57 PM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Jul 18, 2019 at 04:17:22PM +0800, Ning Yu wrote:
> This is still wrong with current code logic, because when the statusdir is
> a file the errno is also EEXIST, but it can pass the check here.  Even if
> we fix pg_mkdir_p() by following the `mkdir -p` way the errno check here is
> still wrong.

Would you like to send a patch?

Michael, we'll send out the patch later. Checked code, it seems that there is another related mkdir() issue.

MakePGDirectory() is actually a syscall mkdir(), and manpage says the errno meaning of EEXIST,

       EEXIST pathname already exists (not necessarily as a directory).  This includes the case where pathname is a symbolic link, dangling or not.

However it looks like some callers do not use that correctly, e.g.

      if (MakePGDirectory(directory) < 0)
      {
          if (errno == EEXIST)
              return;

OR

      if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)

i.e. we should better use stat(path) && S_ISDIR(buf) && errno == EEXIST to replace errno == EEXIST.

One possible fix is to add an argument like ignore_created (in case some callers want to fail if the path has been created) in MakePGDirectory() and then add that code logic into it.


pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: SegFault on 9.6.14
Next
From: Etsuro Fujita
Date:
Subject: Re: partition routing layering in nodeModifyTable.c