{"thread":{"id":"5561","subject":"Enable the packed refs file format","startedAt":"2006-09-14T17:14:47Z","lastAt":"2006-09-23T04:34:48Z","messageCount":8,"participants":["Linus Torvalds","Petr Baudis","Phil Richards","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"26901","messageId":"Pine.LNX.4.64.0609141005440.4388@g5.osdl.org","threadId":"5561","inReplyTo":null,"subject":"Enable the packed refs file format","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-09-14T17:14:47Z","receivedAt":"2006-09-14T17:14:47Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThis actually \"turns on\" the packed ref file format, now that the \ninfrastructure to do so sanely exists (ie notably the change to make the \nreference reading logic take refnames rather than pathnames to the loose \nobjects that no longer necessarily even exist).\n\nIn particular, when the ref lookup hits a refname that has no loose file \nassociated with it, it falls back on the packed-ref information. Also, the \nref-locking code, while still using a loose file for the locking itself \n(and _creating_ a loose file for the new ref) no longer requires that the \nold ref be in such an unpacked state.\n\nFinally, this does a minimal hack to git-checkout.sh to rather than check \nthe ref-file directly, do a \"git-rev-parse\" on the \"heads/$refname\". \nThat's not really wonderful - we should rather really have a special \nroutine to verify the names as proper branch head names, but it is a \nworkable solution for now.\n\nWith this, I can literally do something like\n\n\tgit pack-refs\n\tfind .git/refs -type f -print0 | xargs -0 rm -f --\n\nand the end result is a largely working repository (ie I've done two \ncommits - which creates _one_ unpacked ref file - done things like run \n\"gitk\" and \"git log\" etc, and it all looks ok).\n\nThere are probably things missing, but I'm hoping that the missing things \nare now of the \"small and obvious\" kind, and that somebody else might want \nto start looking at this too. Hint hint ;)\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n---\n\nThis obviously depends on the lt/refs branch in Junio's tree, that is \ncurrently only in -pu.\n\ndiff --git a/git-checkout.sh b/git-checkout.sh\nindex 580a9e8..c60e029 100755\n--- a/git-checkout.sh\n+++ b/git-checkout.sh\n@@ -22,7 +22,7 @@ while [ \"$#\" != \"0\" ]; do\n \t\tshift\n \t\t[ -z \"$newbranch\" ] &&\n \t\t\tdie \"git checkout: -b needs a branch name\"\n-\t\t[ -e \"$GIT_DIR/refs/heads/$newbranch\" ] &&\n+\t\tgit-rev-parse --symbolic \"heads/$newbranch\" >&/dev/null &&\n \t\t\tdie \"git checkout: branch $newbranch already exists\"\n \t\tgit-check-ref-format \"heads/$newbranch\" ||\n \t\t\tdie \"git checkout: we do not like '$newbranch' as a branch name.\"\n@@ -51,7 +51,7 @@ while [ \"$#\" != \"0\" ]; do\n \t\t\tfi\n \t\t\tnew=\"$rev\"\n \t\t\tnew_name=\"$arg^0\"\n-\t\t\tif [ -f \"$GIT_DIR/refs/heads/$arg\" ]; then\n+\t\t\tif git-rev-parse \"heads/$arg^0\" >&/dev/null; then\n \t\t\t\tbranch=\"$arg\"\n \t\t\tfi\n \t\telif rev=$(git-rev-parse --verify \"$arg^{tree}\" 2>/dev/null)\ndiff --git a/refs.c b/refs.c\nindex 50c25d3..134c0fc 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -28,6 +28,8 @@ static const char *parse_ref_line(char *\n \tif (!isspace(line[40]))\n \t\treturn NULL;\n \tline += 41;\n+\tif (isspace(*line))\n+\t\treturn NULL;\n \tif (line[len] != '\\n')\n \t\treturn NULL;\n \tline[len] = 0;\n@@ -168,6 +170,14 @@ const char *resolve_ref(const char *ref,\n \t\t * reading.\n \t\t */\n \t\tif (lstat(path, &st) < 0) {\n+\t\t\tstruct ref_list *list = get_packed_refs();\n+\t\t\twhile (list) {\n+\t\t\t\tif (!strcmp(ref, list->name)) {\n+\t\t\t\t\thashcpy(sha1, list->sha1);\n+\t\t\t\t\treturn ref;\n+\t\t\t\t}\n+\t\t\t\tlist = list->next;\n+\t\t\t}\n \t\t\tif (reading || errno != ENOENT)\n \t\t\t\treturn NULL;\n \t\t\thashclr(sha1);\n@@ -400,22 +410,13 @@ int check_ref_format(const char *ref)\n static struct ref_lock *verify_lock(struct ref_lock *lock,\n \tconst unsigned char *old_sha1, int mustexist)\n {\n-\tchar buf[40];\n-\tint nr, fd = open(lock->ref_file, O_RDONLY);\n-\tif (fd < 0 && (mustexist || errno != ENOENT)) {\n-\t\terror(\"Can't verify ref %s\", lock->ref_file);\n-\t\tunlock_ref(lock);\n-\t\treturn NULL;\n-\t}\n-\tnr = read(fd, buf, 40);\n-\tclose(fd);\n-\tif (nr != 40 || get_sha1_hex(buf, lock->old_sha1) < 0) {\n-\t\terror(\"Can't verify ref %s\", lock->ref_file);\n+\tif (!resolve_ref(lock->ref_name, lock->old_sha1, mustexist)) {\n+\t\terror(\"Can't verify ref %s\", lock->ref_name);\n \t\tunlock_ref(lock);\n \t\treturn NULL;\n \t}\n \tif (hashcmp(lock->old_sha1, old_sha1)) {\n-\t\terror(\"Ref %s is at %s but expected %s\", lock->ref_file,\n+\t\terror(\"Ref %s is at %s but expected %s\", lock->ref_name,\n \t\t\tsha1_to_hex(lock->old_sha1), sha1_to_hex(old_sha1));\n \t\tunlock_ref(lock);\n \t\treturn NULL;\n@@ -427,6 +428,7 @@ static struct ref_lock *lock_ref_sha1_ba\n \tint plen,\n \tconst unsigned char *old_sha1, int mustexist)\n {\n+\tchar *ref_file;\n \tconst char *orig_ref = ref;\n \tstruct ref_lock *lock;\n \tstruct stat st;\n@@ -445,13 +447,14 @@ static struct ref_lock *lock_ref_sha1_ba\n \t}\n \tlock->lk = xcalloc(1, sizeof(struct lock_file));\n \n-\tlock->ref_file = xstrdup(git_path(\"%s\", ref));\n+\tlock->ref_name = xstrdup(ref);\n \tlock->log_file = xstrdup(git_path(\"logs/%s\", ref));\n-\tlock->force_write = lstat(lock->ref_file, &st) && errno == ENOENT;\n+\tref_file = git_path(ref);\n+\tlock->force_write = lstat(ref_file, &st) && errno == ENOENT;\n \n-\tif (safe_create_leading_directories(lock->ref_file))\n-\t\tdie(\"unable to create directory for %s\", lock->ref_file);\n-\tlock->lock_fd = hold_lock_file_for_update(lock->lk, lock->ref_file, 1);\n+\tif (safe_create_leading_directories(ref_file))\n+\t\tdie(\"unable to create directory for %s\", ref_file);\n+\tlock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, 1);\n \n \treturn old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;\n }\n@@ -479,7 +482,7 @@ void unlock_ref(struct ref_lock *lock)\n \t\tif (lock->lk)\n \t\t\trollback_lock_file(lock->lk);\n \t}\n-\tfree(lock->ref_file);\n+\tfree(lock->ref_name);\n \tfree(lock->log_file);\n \tfree(lock);\n }\n@@ -556,7 +559,7 @@ int write_ref_sha1(struct ref_lock *lock\n \t\treturn -1;\n \t}\n \tif (commit_lock_file(lock->lk)) {\n-\t\terror(\"Couldn't set %s\", lock->ref_file);\n+\t\terror(\"Couldn't set %s\", lock->ref_name);\n \t\tunlock_ref(lock);\n \t\treturn -1;\n \t}\ndiff --git a/refs.h b/refs.h\nindex 553155c..af347e6 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -2,7 +2,7 @@ #ifndef REFS_H\n #define REFS_H\n \n struct ref_lock {\n-\tchar *ref_file;\n+\tchar *ref_name;\n \tchar *log_file;\n \tstruct lock_file *lk;\n \tunsigned char old_sha1[20];\n"},{"id":"27166","messageId":"20060919205554.GA8259@pasky.or.cz","threadId":"5561","inReplyTo":"Pine.LNX.4.64.0609141005440.4388@g5.osdl.org","subject":"Re: Enable the packed refs file format","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-09-19T20:55:54Z","receivedAt":"2006-09-19T20:55:54Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Thu, Sep 14, 2006 at 07:14:47PM CEST, I got a letter\nwhere Linus Torvalds <torvalds@osdl.org> said that...\n> +\tref_file = git_path(ref);\n\nYou slip...\nYou fall...\n*BLAMMMM!!!*\n\nCloning a repository with '%s' tag over HTTP now dumps core nicely, and\nI guess this kind of bugs tends to be exploitable.\n\n-- \n\t\t\t\tPetr \"Pasky Yay for Obscure ADOM\n\t\t\t\t\tReferences\" Baudis\nStuff: http://pasky.or.cz/\nSnow falling on Perl. White noise covering line noise.\nHides all the bugs too. -- J. Putnam\n"},{"id":"27169","messageId":"Pine.LNX.4.64.0609191407340.4388@g5.osdl.org","threadId":"5561","inReplyTo":"20060919205554.GA8259@pasky.or.cz","subject":"Re: Enable the packed refs file format","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-09-19T21:09:57Z","receivedAt":"2006-09-19T21:09:57Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 19 Sep 2006, Petr Baudis wrote:\n\n> Dear diary, on Thu, Sep 14, 2006 at 07:14:47PM CEST, I got a letter\n> where Linus Torvalds <torvalds@osdl.org> said that...\n> > +\tref_file = git_path(ref);\n> \n> You slip...\n> You fall...\n> *BLAMMMM!!!*\n\nGaah. Yes. I fixed one such mistake already.\n\nToo bad that we can't get gcc to warn on these things. We do mark it as \n\"format(printf)\", but I don't know of any way to tell gcc that it _has_ to \nhave that initial constant string.\n\n\t\tLinus\n"},{"id":"27282","messageId":"20060920201930.9E48F487D@derisoft.derived-software.demon.co.uk","threadId":"5561","inReplyTo":"Pine.LNX.4.64.0609191407340.4388@g5.osdl.org","subject":"Re: Enable the packed refs file format","fromName":"Phil Richards","fromEmail":"news@derived-software.ltd.uk","sentAt":"2006-09-20T20:19:30Z","receivedAt":"2006-09-20T20:19:30Z","isPatch":false,"sender":{"key":"news@derived-software.ltd.uk","avatar":null},"body":"On 2006-09-19, Linus Torvalds <torvalds@osdl.org> wrote:\n>  Too bad that we can't get gcc to warn on these things. We do mark it as \n>  \"format(printf)\", but I don't know of any way to tell gcc that it _has_ to \n>  have that initial constant string.\n\nNot sure if it just a gcc 4.x-ism, but -Wformat-nonliteral or -Wformat-security\nmight be what you are looking for.\n\n`-Wformat-nonliteral'\n     If `-Wformat' is specified, also warn if the format string is not a\n     string literal and so cannot be checked, unless the format function\n     takes its format arguments as a `va_list'.\n\n`-Wformat-security'\n     If `-Wformat' is specified, also warn about uses of format\n     functions that represent possible security problems.  At present,\n     this warns about calls to `printf' and `scanf' functions where the\n     format string is not a string literal and there are no format\n     arguments, as in `printf (foo);'.  This may be a security hole if\n     the format string came from untrusted input and contains `%n'.\n     (This is currently a subset of what `-Wformat-nonliteral' warns\n     about, but in future warnings may be added to `-Wformat-security'\n     that are not included in `-Wformat-nonliteral'.)\n\n\nphil\n-- \nchange name before \"@\" to \"phil\" for email\n"},{"id":"27423","messageId":"20060922230845.GB8259@pasky.or.cz","threadId":"5561","inReplyTo":"20060919205554.GA8259@pasky.or.cz","subject":"[PATCH] Fix buggy ref recording","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-09-22T23:08:45Z","receivedAt":"2006-09-22T23:08:45Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Tue, Sep 19, 2006 at 10:55:54PM CEST, I got a letter\nwhere Petr Baudis <pasky@suse.cz> said that...\n> Dear diary, on Thu, Sep 14, 2006 at 07:14:47PM CEST, I got a letter\n> where Linus Torvalds <torvalds@osdl.org> said that...\n> > +\tref_file = git_path(ref);\n> \n> You slip...\n> You fall...\n> *BLAMMMM!!!*\n> \n> Cloning a repository with '%s' tag over HTTP now dumps core nicely, and\n> I guess this kind of bugs tends to be exploitable.\n\nAnd since just reporting it did not magically result in a fix... ;-)\n\n-8<-\n\nThere is a format string vulnerability introduced with the packed refs\nfile format.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n refs.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 40f16af..5fdf9c4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -472,7 +472,7 @@ static struct ref_lock *lock_ref_sha1_ba\n\n \tlock->ref_name = xstrdup(ref);\n \tlock->log_file = xstrdup(git_path(\"logs/%s\", ref));\n-\tref_file = git_path(ref);\n+\tref_file = git_path(\"%s\", ref);\n \tlock->force_write = lstat(ref_file, &st) && errno == ENOENT;\n\n \tif (safe_create_leading_directories(ref_file))\n\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj\n$/=unpack('H*',$_);$_=`echo 16dio\\U$k\"SK$/SM$n\\EsN0p[lN*1\nlK[d2%Sa2/d0$^Ixp\"|dc`;s/\\W//g;$_=pack('H*',/((..)*)$/)\n"},{"id":"27434","messageId":"7vzmcrxvgw.fsf@assigned-by-dhcp.cox.net","threadId":"5561","inReplyTo":"20060922230845.GB8259@pasky.or.cz","subject":"Re: [PATCH] Fix buggy ref recording","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-23T00:44:31Z","receivedAt":"2006-09-23T00:44:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> And since just reporting it did not magically result in a fix... ;-)\n\nYes, please always send in a patch to be applied to get the\nattribution right.\n\nI've never seen you send out a corrupt patch over e-mail.\nWhat's different this time?\n\n> diff --git a/refs.c b/refs.c\n> index 40f16af..5fdf9c4 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -472,7 +472,7 @@ static struct ref_lock *lock_ref_sha1_ba\n>\n>  \tlock->ref_name = xstrdup(ref);\n>  \tlock->log_file = xstrdup(git_path(\"logs/%s\", ref));\n\nThe empty line at the beginning of the hunk is totally empty,\nnot even with a SP to show it is a context line.\n\nWill hand-apply, no need to resend.\n"},{"id":"27438","messageId":"20060923011634.GL13132@pasky.or.cz","threadId":"5561","inReplyTo":"7vzmcrxvgw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix buggy ref recording","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-09-23T01:16:34Z","receivedAt":"2006-09-23T01:16:34Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sat, Sep 23, 2006 at 02:44:31AM CEST, I got a letter\nwhere Junio C Hamano <junkio@cox.net> said that...\n> I've never seen you send out a corrupt patch over e-mail.\n> What's different this time?\n> \n> > diff --git a/refs.c b/refs.c\n> > index 40f16af..5fdf9c4 100644\n> > --- a/refs.c\n> > +++ b/refs.c\n> > @@ -472,7 +472,7 @@ static struct ref_lock *lock_ref_sha1_ba\n> >\n> >  \tlock->ref_name = xstrdup(ref);\n> >  \tlock->log_file = xstrdup(git_path(\"logs/%s\", ref));\n> \n> The empty line at the beginning of the hunk is totally empty,\n> not even with a SP to show it is a context line.\n\nSorry, I've cut'n'pasted from an ssh session on repo.or.cz and *thought*\nthat I fixed the whitespaces...\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj\n$/=unpack('H*',$_);$_=`echo 16dio\\U$k\"SK$/SM$n\\EsN0p[lN*1\nlK[d2%Sa2/d0$^Ixp\"|dc`;s/\\W//g;$_=pack('H*',/((..)*)$/)\n"},{"id":"27446","messageId":"7vmz8rw68n.fsf_-_@assigned-by-dhcp.cox.net","threadId":"5561","inReplyTo":"20060923011634.GL13132@pasky.or.cz","subject":"[PATCH] pack-refs: fix git_path() usage.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-23T04:34:48Z","receivedAt":"2006-09-23T04:34:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\n * A valid ref name can contain %.\n\n builtin-pack-refs.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-pack-refs.c b/builtin-pack-refs.c\nindex 246dd63..db57fee 100644\n--- a/builtin-pack-refs.c\n+++ b/builtin-pack-refs.c\n@@ -56,7 +56,7 @@ static void prune_ref(struct ref_to_prun\n \tstruct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1, 1);\n \n \tif (lock) {\n-\t\tunlink(git_path(r->name));\n+\t\tunlink(git_path(\"%s\", r->name));\n \t\tunlock_ref(lock);\n \t}\n }\n-- \n1.4.2.1.gf2bba\n"}]}