[PATCH 0/3] sha256/gcrypt fixes

[PATCH 0/3] sha256/gcrypt fixes

[Eric Wong]

I noticed problems requiring patches 2 and 3 while eyeballing
the code, but had to come up with the first one to fix
SANITIZE=leak, first.

Eric Wong (3):
  sha256/gcrypt: fix build with SANITIZE=leak
  sha256/gcrypt: fix memory leak with SHA-256 repos
  sha256/gcrypt: die on gcry_md_open failures

 sha256/gcrypt.h | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

[PATCH 1/3] sha256/gcrypt: fix build with SANITIZE=leak

[Eric Wong]

Non-static functions cause `undefined reference' errors when
building with `SANITIZE=leak' due to the lack of prototypes.
Mark all these functions as `static inline' as we do in
sha256/nettle.h to avoid the need to maintain prototypes.

Signed-off-by: Eric Wong <e@80x24.org>
---
 sha256/gcrypt.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h
index 501da5ed91..68cf6b6a54 100644
--- a/sha256/gcrypt.h
+++ b/sha256/gcrypt.h
@@ -7,22 +7,22 @@
 
 typedef gcry_md_hd_t gcrypt_SHA256_CTX;
 
-inline void gcrypt_SHA256_Init(gcrypt_SHA256_CTX *ctx)
+static inline void gcrypt_SHA256_Init(gcrypt_SHA256_CTX *ctx)
 {
 	gcry_md_open(ctx, GCRY_MD_SHA256, 0);
 }
 
-inline void gcrypt_SHA256_Update(gcrypt_SHA256_CTX *ctx, const void *data, size_t len)
+static inline void gcrypt_SHA256_Update(gcrypt_SHA256_CTX *ctx, const void *data, size_t len)
 {
 	gcry_md_write(*ctx, data, len);
 }
 
-inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX *ctx)
+static inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX *ctx)
 {
 	memcpy(digest, gcry_md_read(*ctx, GCRY_MD_SHA256), SHA256_DIGEST_SIZE);
 }
 
-inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA256_CTX *src)
+static inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA256_CTX *src)
 {
 	gcry_md_copy(dst, *src);
 }

[PATCH 2/3] sha256/gcrypt: fix memory leak with SHA-256 repos

[Eric Wong]

`gcry_md_open' needs to be paired with `gcry_md_close' to ensure
resources are released.  Since our internal APIs don't have
separate close/release callbacks, sticking it into the finalization
callback seems appropriate.

Building with SANITIZE=leak and running `git fsck' on a SHA-256
repository no longer reports leaks.

Signed-off-by: Eric Wong <e@80x24.org>
---
 sha256/gcrypt.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h
index 68cf6b6a54..1d06a778af 100644
--- a/sha256/gcrypt.h
+++ b/sha256/gcrypt.h
@@ -20,6 +20,7 @@ static inline void gcrypt_SHA256_Update(gcrypt_SHA256_CTX *ctx, const void *data
 static inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX *ctx)
 {
 	memcpy(digest, gcry_md_read(*ctx, GCRY_MD_SHA256), SHA256_DIGEST_SIZE);
+	gcry_md_close(*ctx);
 }
 
 static inline void gcrypt_SHA256_Clone(gcrypt_SHA256_CTX *dst, const gcrypt_SHA256_CTX *src)

[PATCH 3/3] sha256/gcrypt: die on gcry_md_open failures

[Eric Wong]

`gcry_md_open' allocates memory and must (like all allocation
functions) be checked for failure.

Signed-off-by: Eric Wong <e@80x24.org>
---
 sha256/gcrypt.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h
index 1d06a778af..17a90f1052 100644
--- a/sha256/gcrypt.h
+++ b/sha256/gcrypt.h
@@ -9,7 +9,9 @@ typedef gcry_md_hd_t gcrypt_SHA256_CTX;
 
 static inline void gcrypt_SHA256_Init(gcrypt_SHA256_CTX *ctx)
 {
-	gcry_md_open(ctx, GCRY_MD_SHA256, 0);
+	gcry_error_t err = gcry_md_open(ctx, GCRY_MD_SHA256, 0);
+	if (err)
+		die("gcry_md_open: %s", gcry_strerror(err));
 }
 
 static inline void gcrypt_SHA256_Update(gcrypt_SHA256_CTX *ctx, const void *data, size_t len)

Re: [PATCH 0/3] sha256/gcrypt fixes

[Junio C Hamano]

Eric Wong <e@80x24.org> writes:

> I noticed problems requiring patches 2 and 3 while eyeballing
> the code, but had to come up with the first one to fix
> SANITIZE=leak, first.

Thanks.

>
> Eric Wong (3):
>   sha256/gcrypt: fix build with SANITIZE=leak
>   sha256/gcrypt: fix memory leak with SHA-256 repos
>   sha256/gcrypt: die on gcry_md_open failures
>
>  sha256/gcrypt.h | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)