git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] refs: make sure we never pass NULL to hashcpy

From
Thomas Gummerer <t.gummerer@gmail.com>
Date
Sep 4, 2017, 20:05 UTC
Message-ID
<20170904200504.15249-1-t.gummerer@gmail.com>

gcc on arch linux (version 7.1.1) warns that a NULL argument is passed as the second parameter of memcpy.

In file included from refs.c:5:0:
refs.c: In function ‘ref_transaction_verify’:
cache.h:948:2: error: argument 2 null where non-null expected [-Werror=nonnull]
  memcpy(sha_dst, sha_src, GIT_SHA1_RAWSZ);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from git-compat-util.h:165:0,
                 from cache.h:4,
                 from refs.c:5:
/usr/include/string.h:43:14: note: in a call to function ‘memcpy’ declared here
 extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
              ^~~~~~

Tracking this error down, we can track it back to ref_transaction_add_update. where the call to hashcpy is however protected by the flags that are passed in.

To make sure there's no code path where the wrong flags are passed in, and to help the compiler realize that no NULL parameter is passed as second argument to hashcpy, add asserts that this is indeed the case.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---

This is based on top of ma/ts-cleanups, as that fixes another compiler warning with gcc 7.1.1.

 refs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index ba22f4acef..d8c12a9c44 100644
--- a/refs.c
+++ b/refs.c
@@ -896,10 +896,14 @@ struct ref_update *ref_transaction_add_update(
 
 	update->flags = flags;
 
-	if (flags & REF_HAVE_NEW)
+	if (flags & REF_HAVE_NEW) {
+		assert(new_sha1);
 		hashcpy(update->new_oid.hash, new_sha1);
-	if (flags & REF_HAVE_OLD)
+	}
+	if (flags & REF_HAVE_OLD) {
+		assert(old_sha1);
 		hashcpy(update->old_oid.hash, old_sha1);
+	}
 	update->msg = xstrdup_or_null(msg);
 	return update;
 }
-- 
2.14.1.480.gb18f417b89
Next: Junio C Hamano
Message 1 of 12 in “refs: make sure we never pass NULL to hashcpy”
  1. refs: make sure we never pass NULL to hashcpyThomas Gummerer, Sep 4, 2017
  2. Junio C HamanoSep 6, 2017
  3. Thomas GummererSep 6, 2017
  4. Michael HaggertySep 7, 2017
  5. Thomas GummererSep 7, 2017
  6. Junio C HamanoSep 8, 2017
  7. Michael HaggertySep 8, 2017
  8. Junio C HamanoSep 8, 2017
  9. [RFC v2] refs: strip out not allowed flags from ref_transaction_updateThomas Gummerer, Sep 12, 2017
  10. Michael HaggertySep 21, 2017
  11. Junio C HamanoSep 22, 2017
  12. Thomas GummererSep 24, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.