From: Jeff King <peff@peff.net>
To: Andrey Okoshkin <a.okoshkin@samsung.com>
Cc: gitster@pobox.com, git@vger.kernel.org, pclouds@gmail.com,
l.s.r@web.de, avarab@gmail.com, krh@redhat.com,
rctay89@gmail.com, Ivan Arishchenko <i.arishchenk@samsung.com>,
Mikhail Labiuk <m.labiuk@samsung.com>
Subject: [PATCH 4/4] worktree: handle broken symrefs in find_shared_symref()
Date: Thu, 19 Oct 2017 13:49:36 -0400 [thread overview]
Message-ID: <20171019174936.izojvrh5w35s3adi@sigill.intra.peff.net> (raw)
In-Reply-To: <20171019174452.hd3c47ocducddvgr@sigill.intra.peff.net>
The refs_resolve_ref_unsafe() function may return NULL even
with a REF_ISSYMREF flag if a symref points to a broken ref.
As a result, it's possible for find_shared_symref() to
segfault when it passes NULL to strcmp().
This is hard to trigger for most code paths. We typically
pass HEAD to the function as the symref to resolve, and
programs like "git branch" will bail much earlier if HEAD
isn't valid.
I did manage to trigger it through one very obscure
sequence:
# You have multiple notes refs which conflict.
git notes add -m base
git notes --ref refs/notes/foo add -m foo
# There's left-over cruft in NOTES_MERGE_REF that
# makes it a broken symref (in this case we point
# to a syntactically invalid ref).
echo "ref: refs/heads/master.lock" >.git/NOTES_MERGE_REF
# You try to merge the notes. We read the broken value in
# order to complain that another notes-merge is
# in-progress, but we segfault in find_shared_symref().
git notes merge refs/notes/foo
This is obviously silly and almost certainly impossible to
trigger accidentally, but it does show that the bug is
triggerable from at least one code path. In addition, it
would trigger if we saw a transient filesystem error when
resolving the pointed-to ref.
We can fix this by treating NULL the same as a non-matching
symref. Arguably we'd prefer to tell know if a symref points
to "refs/heads/foo", but "refs/heads/foo" is broken. But
refs_resolve_ref_unsafe() isn't capable of giving us that
information, so this is the best we can do.
Signed-off-by: Jeff King <peff@peff.net>
---
worktree.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/worktree.c b/worktree.c
index 70015629dc..f8c40f2f5f 100644
--- a/worktree.c
+++ b/worktree.c
@@ -327,7 +327,8 @@ const struct worktree *find_shared_symref(const char *symref,
refs = get_worktree_ref_store(wt);
symref_target = refs_resolve_ref_unsafe(refs, symref, 0,
NULL, &flags);
- if ((flags & REF_ISSYMREF) && !strcmp(symref_target, target)) {
+ if ((flags & REF_ISSYMREF) &&
+ symref_target && !strcmp(symref_target, target)) {
existing = wt;
break;
}
--
2.15.0.rc1.560.g5f0609e481
next prev parent reply other threads:[~2017-10-19 17:49 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20171018170047epcas2p4310be357e11e194d6d08ac3bdc478ba3@epcas2p4.samsung.com>
2017-10-18 17:00 ` [PATCH] commit: check result of resolve_ref_unsafe Andrey Okoshkin
2017-10-18 18:34 ` Jeff King
2017-10-19 0:41 ` Junio C Hamano
2017-10-19 2:49 ` Jeff King
2017-10-19 9:33 ` Andrey Okoshkin
2017-10-19 9:36 ` [PATCH v2] " Andrey Okoshkin
2017-10-19 17:44 ` Jeff King
2017-10-19 17:46 ` [PATCH 1/4] test-ref-store: avoid passing NULL to printf Jeff King
2017-10-19 17:47 ` [PATCH 2/4] remote: handle broken symrefs Jeff King
2017-10-19 17:53 ` Jeff King
2017-10-19 17:49 ` [PATCH 3/4] log: handle broken HEAD in decoration check Jeff King
2017-10-19 17:49 ` Jeff King [this message]
2017-10-21 10:49 ` [PATCH 4/4] worktree: handle broken symrefs in find_shared_symref() Eric Sunshine
2017-10-21 19:26 ` Jeff King
2017-10-22 0:46 ` Junio C Hamano
2017-10-20 10:40 ` [PATCH v2] commit: check result of resolve_ref_unsafe Andrey Okoshkin
2017-10-20 11:03 ` [PATCH v3] " Andrey Okoshkin
2017-10-20 13:09 ` [PATCH v4] " Andrey Okoshkin
2017-10-21 6:19 ` Jeff King
2017-10-22 0:46 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171019174936.izojvrh5w35s3adi@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=a.okoshkin@samsung.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=i.arishchenk@samsung.com \
--cc=krh@redhat.com \
--cc=l.s.r@web.de \
--cc=m.labiuk@samsung.com \
--cc=pclouds@gmail.com \
--cc=rctay89@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.