Possible race condition in pg_mkdir_p()?
От | Ning Yu |
---|---|
Тема | Possible race condition in pg_mkdir_p()? |
Дата | |
Msg-id | CAKmaiL3fXqQHJ-b7M3RoPpfBR2CSroPHACwUb-r1TAxH0qqsHg@mail.gmail.com обсуждение исходный текст |
Ответы |
Re: Possible race condition in pg_mkdir_p()?
Re: Possible race condition in pg_mkdir_p()? |
Список | pgsql-hackers |
Hi Hackers,
We found a race condition in pg_mkdir_p(), here is a simple reproducer:
#!/bin/sh
basedir=pgdata
datadir=$basedir/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z
logdir=$basedir/logs
n=2
rm -rf $basedir
mkdir -p $logdir
# init databases concurrently, they will all try to create the parent dirs
for i in `seq $n`; do
initdb -D $datadir/$i >$logdir/$i.log 2>&1 &
done
wait
# there is a chance one of the initdb commands failed to create the datadir
grep 'could not create directory' $logdir/*
basedir=pgdata
datadir=$basedir/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z
logdir=$basedir/logs
n=2
rm -rf $basedir
mkdir -p $logdir
# init databases concurrently, they will all try to create the parent dirs
for i in `seq $n`; do
initdb -D $datadir/$i >$logdir/$i.log 2>&1 &
done
wait
# there is a chance one of the initdb commands failed to create the datadir
grep 'could not create directory' $logdir/*
The logic in pg_mkdir_p() is as below:
/* check for pre-existing directory */
if (stat(path, &sb) == 0)
{
if (!S_ISDIR(sb.st_mode))
{
if (last)
errno = EEXIST;
else
errno = ENOTDIR;
retval = -1;
break;
}
}
else if (mkdir(path, last ? omode : S_IRWXU | S_IRWXG | S_IRWXO) < 0)
{
retval = -1;
break;
}
if (stat(path, &sb) == 0)
{
if (!S_ISDIR(sb.st_mode))
{
if (last)
errno = EEXIST;
else
errno = ENOTDIR;
retval = -1;
break;
}
}
else if (mkdir(path, last ? omode : S_IRWXU | S_IRWXG | S_IRWXO) < 0)
{
retval = -1;
break;
}
This seems buggy as it first checks the existence of the dir and makes the dir if it does not exist yet, however when executing concurrently a possible race condition can be as below:
A: does a/ exists? no
B: does a/ exists? no
A: try to create a/, succeed
B: try to create a/, failed as it already exists
B: does a/ exists? no
A: try to create a/, succeed
B: try to create a/, failed as it already exists
To prevent the race condition we could mkdir() directly, if it returns -1 and errno is EEXIST then check whether it's really a dir with stat(). In fact this is what is done in the `mkdir -p` command: https://github.com/coreutils/gnulib/blob/b5a9fa677847081c9b4f26908272f122b15df8b9/lib/mkdir-p.c#L130-L164
By the way, some callers of pg_mkdir_p() checks for EEXIST explicitly, such as in pg_basebackup.c:
if (pg_mkdir_p(statusdir, pg_dir_create_mode) != 0 && errno != EEXIST)
{
pg_log_error("could not create directory \"%s\": %m", statusdir);
exit(1);
}
{
pg_log_error("could not create directory \"%s\": %m", statusdir);
exit(1);
}
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.
Best Regards
Ning
В списке pgsql-hackers по дате отправления: