While pushing to a remote repo, Git transiently adds a .keep file for the
pack being pushed, to protect it from a concurrent "git gc". However, the
permissions on this .keep file are such that if a different user attempts
a local cross-filesystem clone ("git clone --no-hardlinks") on the server
while the .keep file is present (either because of a concurrent push, or
because of a prior failed push that left a stale .keep file), the clone
will fail because the second user cannot access the .keep file created by
the first user.
There's no reason why the permission mode of a .keep file should be any
different from the permission mode of the corresponding .pack/.idx files.
Therefore, adjust the permission of .keep files from 0600 to 0444 modulo
the shared_repository setting.
In the above scenario, the .keep file is now accessible to the second user,
and will not prevent the clone.
Signed-off-by: Johan Herland <johan@xxxxxxxxxxx>
---
On Thursday 31 March 2011, Jeff King wrote:
> On Thu, Mar 31, 2011 at 12:46:25PM +0200, Johan Herland wrote:
> > 1. Why does the .keep file have 0600 permissions (preventing a local
> > clone by any other user)
>
> The relevant code is in 6e180cd (Make sure objects/pack exists before
> creating a new pack, 2009-02-24). I don't see anything particular about
> the mode, so I suspect it was simply habit to make tempfiles restricted.
>
> There is nothing secret in the contents, so I don't see any reason to
> loosen it to the same permissions as the packfiles themselves.
This patch attempts to fix the permissions on .keep files.
...Johan
builtin/index-pack.c | 9 ++++++---
environment.c | 4 ++--
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 5a67c81..586c9ac 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -792,10 +792,11 @@ static void final(const char *final_pack_name, const char
*curr_pack_name,
if (keep_msg) {
int keep_fd, keep_msg_len = strlen(keep_msg);
- if (!keep_name)
+ if (!keep_name) {
keep_fd = odb_pack_keep(name, sizeof(name), sha1);
- else
- keep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);
+ keep_name = name;
+ } else
+ keep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0444);
if (keep_fd < 0) {
if (errno != EEXIST)
@@ -811,6 +812,8 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
keep_name);
report = "keep";
}
+ if (adjust_shared_perm(keep_name))
+ error("unable to set permission to '%s'", keep_name);
}
if (final_pack_name != curr_pack_name) {
diff --git a/environment.c b/environment.c
index f4549d3..86bf8f4 100644
--- a/environment.c
+++ b/environment.c
@@ -191,13 +191,13 @@ int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1)
snprintf(name, namesz, "%s/pack/pack-%s.keep",
get_object_directory(), sha1_to_hex(sha1));
- fd = open(name, O_RDWR|O_CREAT|O_EXCL, 0600);
+ fd = open(name, O_RDWR|O_CREAT|O_EXCL, 0444);
if (0 <= fd)
return fd;
/* slow path */
safe_create_leading_directories(name);
- return open(name, O_RDWR|O_CREAT|O_EXCL, 0600);
+ return open(name, O_RDWR|O_CREAT|O_EXCL, 0444);
}
char *get_index_file(void)
--
1.7.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html