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
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
`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
`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)
Junio C HamanoRe: [PATCH 0/3] sha256/gcrypt fixes
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(-)