From dc0642218306190a2d284f47f75d71aeeaa34d02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 5 Jun 2018 19:54:38 +0000 Subject: [PATCH 1/3] refspec: s/refspec_item_init/&_or_die/g MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename the refspec_item_init() function introduced in 6d4c057859 ("refspec: introduce struct refspec", 2018-05-16) to refspec_item_init_or_die(). This follows the convention of other *_or_die() functions, and is done in preparation for making it a wrapper for a non-fatal variant. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/clone.c | 2 +- builtin/pull.c | 2 +- refspec.c | 5 +++-- refspec.h | 3 ++- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 8c5f4d8f07..05cd230973 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1077,7 +1077,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (option_required_reference.nr || option_optional_reference.nr) setup_reference(); - refspec_item_init(&refspec, value.buf, REFSPEC_FETCH); + refspec_item_init_or_die(&refspec, value.buf, REFSPEC_FETCH); strbuf_reset(&value); diff --git a/builtin/pull.c b/builtin/pull.c index 09575fd23c..af9306ecdc 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -680,7 +680,7 @@ static const char *get_tracking_branch(const char *remote, const char *refspec) const char *spec_src; const char *merge_branch; - refspec_item_init(&spec, refspec, REFSPEC_FETCH); + refspec_item_init_or_die(&spec, refspec, REFSPEC_FETCH); spec_src = spec.src; if (!*spec_src || !strcmp(spec_src, "HEAD")) spec_src = "HEAD"; diff --git a/refspec.c b/refspec.c index ada7854f7a..54b6fe0c6d 100644 --- a/refspec.c +++ b/refspec.c @@ -122,7 +122,8 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet return 1; } -void refspec_item_init(struct refspec_item *item, const char *refspec, int fetch) +void refspec_item_init_or_die(struct refspec_item *item, const char *refspec, + int fetch) { memset(item, 0, sizeof(*item)); @@ -150,7 +151,7 @@ void refspec_append(struct refspec *rs, const char *refspec) { struct refspec_item item; - refspec_item_init(&item, refspec, rs->fetch); + refspec_item_init_or_die(&item, refspec, rs->fetch); ALLOC_GROW(rs->items, rs->nr + 1, rs->alloc); rs->items[rs->nr++] = item; diff --git a/refspec.h b/refspec.h index 3a9363887c..4caaf1f8e3 100644 --- a/refspec.h +++ b/refspec.h @@ -32,7 +32,8 @@ struct refspec { int fetch; }; -void refspec_item_init(struct refspec_item *item, const char *refspec, int fetch); +void refspec_item_init_or_die(struct refspec_item *item, const char *refspec, + int fetch); void refspec_item_clear(struct refspec_item *item); void refspec_init(struct refspec *rs, int fetch); void refspec_append(struct refspec *rs, const char *refspec); From c495fd3d1b4b6b395346a8832edbea25f0d60ee7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 5 Jun 2018 19:54:39 +0000 Subject: [PATCH 2/3] refspec: add back a refspec_item_init() function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-add the non-fatal version of refspec_item_init_or_die() renamed away in an earlier change to get a more minimal diff. This should be used by callers that have their own error handling. This new function could be marked "static" since nothing outside of refspec.c uses it, but expecting future use of it, let's make it available to other users. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- refspec.c | 10 +++++++--- refspec.h | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/refspec.c b/refspec.c index 54b6fe0c6d..597ddf8b52 100644 --- a/refspec.c +++ b/refspec.c @@ -122,12 +122,16 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet return 1; } +int refspec_item_init(struct refspec_item *item, const char *refspec, int fetch) +{ + memset(item, 0, sizeof(*item)); + return parse_refspec(item, refspec, fetch); +} + void refspec_item_init_or_die(struct refspec_item *item, const char *refspec, int fetch) { - memset(item, 0, sizeof(*item)); - - if (!parse_refspec(item, refspec, fetch)) + if (!refspec_item_init(item, refspec, fetch)) die("Invalid refspec '%s'", refspec); } diff --git a/refspec.h b/refspec.h index 4caaf1f8e3..9b6e64a824 100644 --- a/refspec.h +++ b/refspec.h @@ -32,6 +32,8 @@ struct refspec { int fetch; }; +int refspec_item_init(struct refspec_item *item, const char *refspec, + int fetch); void refspec_item_init_or_die(struct refspec_item *item, const char *refspec, int fetch); void refspec_item_clear(struct refspec_item *item); From 7865d157a5e8d86f46e626d933bda5c18eab196a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Tue, 5 Jun 2018 19:54:40 +0000 Subject: [PATCH 3/3] refspec: initalize `refspec_item` in `valid_fetch_refspec()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We allocate a `struct refspec_item` on the stack without initializing it. In particular, its `dst` and `src` members will contain some random data from the stack. When we later call `refspec_item_clear()`, it will call `free()` on those pointers. So if the call to `parse_refspec()` did not assign to them, we will be freeing some random "pointers". This is undefined behavior. To the best of my understanding, this cannot currently be triggered by user-provided data. And for what it's worth, the test-suite does not trigger this with SANITIZE=address. It can be provoked by calling `valid_fetch_refspec(":*")`. Zero the struct, as is done in other users of `struct refspec_item` by using the refspec_item_init() initialization function. Signed-off-by: Martin Ågren Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- refspec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refspec.c b/refspec.c index 597ddf8b52..e4c9e86bb6 100644 --- a/refspec.c +++ b/refspec.c @@ -194,7 +194,7 @@ void refspec_clear(struct refspec *rs) int valid_fetch_refspec(const char *fetch_refspec_str) { struct refspec_item refspec; - int ret = parse_refspec(&refspec, fetch_refspec_str, REFSPEC_FETCH); + int ret = refspec_item_init(&refspec, fetch_refspec_str, REFSPEC_FETCH); refspec_item_clear(&refspec); return ret; }