Re: [patch] Avoid race in zf_mkdir

[Matthew Martin <phy1729@xxxxxxxxx>]

On Fri, Oct 09, 2020 at 11:22:00PM +0200, Roman Perepelitsa wrote:
> Perhaps something like this? This should provide the following
> guarantees for zf_mkdir -p:
> 
> - If it succeeds, the directory must have existed at some point during
> the execution of the function (either created by zf_mkdir itself or by
> some other concurrent process).
> - If it fails, there must have been a point in time during the
> execution of the function where the target directory or one of its
> parents didn't exist and it was impossible to create it.
> 
> `zf_mkdir -p foo` It should work as expected in the face of concurrent
> `mkdir foo && rmdir foo` or `touch foo && rm foo`.
> 
> I confess that I haven't tested it.
> 
> Roman.

> diff --git a/Src/Modules/files.c b/Src/Modules/files.c
> index 6d20e38a8..a9ccccb8b 100644
> --- a/Src/Modules/files.c
> +++ b/Src/Modules/files.c
> @@ -122,19 +122,28 @@ domkdir(char *nam, char *path, mode_t mode, int p)
>  {
>      int err;
>      mode_t oumask;
> +    struct stat st;
>      char const *rpath = unmeta(path);
>  
> -    if(p) {
> -	struct stat st;
> -
> -	if(!stat(rpath, &st) && S_ISDIR(st.st_mode))
> +    while(1) {
> +	oumask = umask(0);
> +	err = mkdir(rpath, mode) ? errno : 0;
> +	umask(oumask);
> +	if (!err)
> +	    return 0;
> +	if(!p || err != EEXIST)
> +	    break;
> +	if(!stat(rpath, &st)) {
> +	    if(errno == ENOENT)
> +		continue;

For a sufficiently well timed attacker, the target could be created and
deleted so that this loop never exits.  Even if pathological, I don't
think it should be possible for mkdir to loop forever.

> +	    err = errno;
> +	    break;
> +	}
> +	if(S_ISDIR(st.st_mode))
>  	    return 0;
> +	break;
>      }
> -    oumask = umask(0);
> -    err = mkdir(rpath, mode) ? errno : 0;
> -    umask(oumask);
> -    if(!err)
> -	return 0;
> +
>      zwarnnam(nam, "cannot make directory `%s': %e", path, err);
>      return 1;
>  }