[systemd-devel] hash context not closed.

[Stef Bon]

Hi,

when I look at the function string_hashsum it looks like the context
is not closed when done.

After a succesfull gcry_md_open the hash context md should be closed
when leaving this function. That does not happen.

Stef

[Stef Bon]

Hi,

Oh and this in in file src/basic/gcrypt-util.c.

Stef

[v***@pengaru.com]

Post by Stef Bon
Hi,
when I look at the function string_hashsum it looks like the context
is not closed when done.
After a succesfull gcry_md_open the hash context md should be closed
when leaving this function. That does not happen.

Nice catch, that indeed looks like a leak. Would you like to prep a
PR fixing it?

Regards,
Vito Caputo

[Stef Bon]

Post by v***@pengaru.com
Nice catch, that indeed looks like a leak. Would you like to prep a
PR fixing it?

PR? I do not know what you mean. A pull request?
I can write a patch.

Stef

[v***@pengaru.com]

Post by Stef Bon

Post by v***@pengaru.com
Nice catch, that indeed looks like a leak. Would you like to prep a
PR fixing it?

PR? I do not know what you mean. A pull request?
I can write a patch.

The systemd project uses github. You can create a free github account
if you don't already have one, clone the systemd project to your
account, then push your patch to your cloned repo on github using git.

Once pushed, you create a pull request (PR) of your changes on github
for the systemd project, where we'll be able to review and merge it in
the normal workflow.

If you haven't done this before, it can seem a bit onerous at first.
It's not that bad though, and you'll then be positioned to readily
participate in the abundance of open development occurring on the
platform, in addition to future systemd contributions.

Regards,
Vito Caputo

[Stef Bon]

The testing of the new code requires me too much (meson is required..
and I don't want to replace my existing systemd) and I do not expect
to add more patches is near future.

I've got a patch:

diff --git a/src/basic/gcrypt-util.c b/src/basic/gcrypt-util.c
index 1bfb77672..c7c07e3b7 100644
--- a/src/basic/gcrypt-util.c
+++ b/src/basic/gcrypt-util.c
@@ -46,6 +46,7 @@ int string_hashsum(const char *s, size_t len, int
md_algorithm, char **out) {
size_t hash_size;
void *hash;
char *enc;
+ int res=-EIO;

initialize_libgcrypt(false);

@@ -59,14 +60,24 @@ int string_hashsum(const char *s, size_t len, int
md_algorithm, char **out) {
gcry_md_write(md, s, len);

hash = gcry_md_read(md, 0);
+
if (!hash)
- return -EIO;
+ goto closemd;

+ res = -ENOMEM;
enc = hexmem(hash, hash_size);
- if (!enc)
- return -ENOMEM;

- *out = enc;
- return 0;
+ if (enc) {
+
+ *out = enc;
+ res = 0;
+
+ }
+
+ closemd:
+
+ gcry_md_close(md);
+
+ return res;
}
#endif

Stef

[v***@pengaru.com]

Post by Stef Bon
The testing of the new code requires me too much (meson is required..
and I don't want to replace my existing systemd) and I do not expect
to add more patches is near future.
diff --git a/src/basic/gcrypt-util.c b/src/basic/gcrypt-util.c
index 1bfb77672..c7c07e3b7 100644
--- a/src/basic/gcrypt-util.c
+++ b/src/basic/gcrypt-util.c
@@ -46,6 +46,7 @@ int string_hashsum(const char *s, size_t len, int
md_algorithm, char **out) {
size_t hash_size;
void *hash;
char *enc;
+ int res=-EIO;
initialize_libgcrypt(false);
@@ -59,14 +60,24 @@ int string_hashsum(const char *s, size_t len, int
md_algorithm, char **out) {
gcry_md_write(md, s, len);
hash = gcry_md_read(md, 0);
+
if (!hash)
- return -EIO;
+ goto closemd;
+ res = -ENOMEM;
enc = hexmem(hash, hash_size);
- if (!enc)
- return -ENOMEM;
- *out = enc;
- return 0;
+ if (enc) {
+
+ *out = enc;
+ res = 0;
+
+ }
+
+
+ gcry_md_close(md);
+
+ return res;
}
#endif

I'm going to be offline for a few days, so hopefully someone else on the
list will take over here.

Cheers,
Vito Caputo

[Stef Bon]

Same error here:
(gcry_md_open without gcry_md_close)
line 901 in src/resolve/resolved-dns-sec.c
while in the same file at 1227 it's done the good way.

Stef

[Zbigniew Jędrzejewski-Szmek]

Post by Stef Bon
(gcry_md_open without gcry_md_close)
line 901 in src/resolve/resolved-dns-sec.c
while in the same file at 1227 it's done the good way.

Indeed. PR submitted: https://github.com/systemd/systemd/pull/8100.

Zbyszek

[Stef Bon]

hi,

maybe good to know that libgcrypt provides a function which offers "all in one":

void gcry_md_hash_buffer (int algo, void *digest, const void *buffer,
size_t length)

digest can be an array created earlier using the function gcry_md_get_algo_dlen.

Stef