# [PATCH 0/3] sha256/gcrypt fixes 5 messages from 2023-07-31 to 2023-07-31. Participants: Eric Wong, Junio C Hamano. Thread: https://gitlist.dev/t/60044 ## Eric Wong, 2023-07-31 12:08 Subject: [PATCH 0/3] sha256/gcrypt fixes Message-ID: <20230731120808.1230210-1-e@80x24.org> URL: https://gitlist.dev/e/20230731120808.1230210-1-e%4080x24.org ``` 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(-) ``` ## Eric Wong, 2023-07-31 12:08 Subject: [PATCH 1/3] sha256/gcrypt: fix build with SANITIZE=leak Message-ID: <20230731120808.1230210-2-e@80x24.org> URL: https://gitlist.dev/e/20230731120808.1230210-2-e%4080x24.org In-Reply-To: <20230731120808.1230210-1-e@80x24.org> ``` 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 --- 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); } ``` ## Eric Wong, 2023-07-31 12:08 Subject: [PATCH 2/3] sha256/gcrypt: fix memory leak with SHA-256 repos Message-ID: <20230731120808.1230210-3-e@80x24.org> URL: https://gitlist.dev/e/20230731120808.1230210-3-e%4080x24.org In-Reply-To: <20230731120808.1230210-1-e@80x24.org> ``` `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 --- 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) ``` ## Eric Wong, 2023-07-31 12:08 Subject: [PATCH 3/3] sha256/gcrypt: die on gcry_md_open failures Message-ID: <20230731120808.1230210-4-e@80x24.org> URL: https://gitlist.dev/e/20230731120808.1230210-4-e%4080x24.org In-Reply-To: <20230731120808.1230210-1-e@80x24.org> ``` `gcry_md_open' allocates memory and must (like all allocation functions) be checked for failure. Signed-off-by: Eric Wong --- 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 Hamano, 2023-07-31 15:58 Subject: Re: [PATCH 0/3] sha256/gcrypt fixes Message-ID: URL: https://gitlist.dev/e/xmqqy1iwyswr.fsf%40gitster.g In-Reply-To: <20230731120808.1230210-1-e@80x24.org> ``` Eric Wong 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(-) ```