Re: [PATCH v2 1/3] read-cache: use shared perms when writing shared index

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 23, 2017, 22:02 UTC
Message-ID: <xmqq60fmcqgm.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170623151640.24082-1-chriscool@tuxfamily.org>

Christian Couder <christian.couder@gmail.com> writes:

Show 20 quoted lines

> Since f6ecc62dbf (write_shared_index(): use tempfile module, 2015-08-10)
> write_shared_index() has been using mks_tempfile() to create the
> temporary file that will become the shared index.
>
> But even before that, it looks like the functions used to create this
> file didn't call adjust_shared_perm(), which means that the shared
> index file has always been created with 600 permissions regardless
> of the shared permission settings.
>
> Because of that, on repositories created with `git init --shared=all`
> and using the split index feature, one gets an error like:
>
> fatal: .git/sharedindex.a52f910b489bc462f187ab572ba0086f7b5157de: index file open failed: Permission denied
>
> when another user performs any operation that reads the shared index.
>
> We could use create_tempfile() that calls adjust_shared_perm(), but
> unfortunately create_tempfile() doesn't replace the XXXXXX at the end
> of the path it is passed. So let's just call adjust_shared_perm() by
> ourselves.

Because create_tempfile() is not even a viable alternative, the above sounds just as silly as saying "We could use X, but unfortunately that X doesn't create a temporary file and return its file descriptor" with X replaced with any one of about a dozen functions that happen to call adjust_shared_perm().

	Call adjust_shared_perm() on the temporary file created by
	mks_tempfile() ourselves to adjust the permission bits.

should be sufficient.

Thanks.

Show 25 quoted lines

>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  read-cache.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/read-cache.c b/read-cache.c
> index bc156a133e..66f85f8d58 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2425,6 +2425,14 @@ static int write_shared_index(struct index_state *istate,
>  		delete_tempfile(&temporary_sharedindex);
>  		return ret;
>  	}
> +	ret = adjust_shared_perm(temporary_sharedindex.filename.buf);
> +	if (ret) {
> +		int save_errno = errno;
> +		error("cannot fix permission bits on %s", temporary_sharedindex.filename.buf);
> +		delete_tempfile(&temporary_sharedindex);
> +		errno = save_errno;
> +		return ret;
> +	}
>  	ret = rename_tempfile(&temporary_sharedindex,
>  			      git_path("sharedindex.%s", sha1_to_hex(si->base->sha1)));
>  	if (!ret) {