From efe0cac14901ef3248e15a0084591a8fe9b1b450 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 14 Nov 2024 00:53:14 +0100 Subject: [PATCH 1/8] man: remove duplicate option in depmod.8.scd Fixes: c36ddb6 ("depmod: Add option to override MODULE_DIRECTORY") Signed-off-by: Martin Wilck --- man/depmod.8.scd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/depmod.8.scd b/man/depmod.8.scd index a64ee97e..1cb6cfc2 100644 --- a/man/depmod.8.scd +++ b/man/depmod.8.scd @@ -6,7 +6,7 @@ depmod - Generate modules.dep and map files. # SYNOPSIS -*depmod* [*-b* _basedir_] [*-m* _moduledir_] [*-o* _outdir_][*-o* _outdir_] [*-e*] [*-E* _Module.symvers_] +*depmod* [*-b* _basedir_] [*-m* _moduledir_] [*-o* _outdir_] [*-e*] [*-E* _Module.symvers_] \ \ \ \ \ \ \ \[*-F* _System.map_] [*-n*] [*-v*] [*-A*] [*-P* _prefix_] [*-w*] [_version_] *depmod* [*-e*] [*-E* _Module.symvers_] [*-F* _System.map_] [*-n*] [*-v*] [*-P* _prefix_] From 3e58c873cad12fe56d0b37052c428264c0a96ce2 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 21 Nov 2024 22:52:34 +0100 Subject: [PATCH 2/8] testsuite: test-hash: add a test for deleting a single element ...using n_buckets = 32, which will cause a step size of 1. This test fails, and will be fixed by the next commit. Signed-off-by: Martin Wilck --- testsuite/test-hash.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/testsuite/test-hash.c b/testsuite/test-hash.c index cfe5a8e4..1ec820d1 100644 --- a/testsuite/test-hash.c +++ b/testsuite/test-hash.c @@ -171,6 +171,22 @@ static int test_hash_iter_after_del(const struct test *t) DEFINE_TEST(test_hash_iter_after_del, .description = "test hash_iter, after deleting element"); +static int test_hash_del(const struct test *t) +{ + struct hash *h = hash_new(32, NULL); + const char *k1 = "k1"; + const char *v1 = "v1"; + + hash_add(h, k1, v1); + hash_del(h, k1); + + hash_free(h); + + return 0; +} +DEFINE_TEST(test_hash_del, .description = "test add / delete a single element", + .expected_fail = true); + static int test_hash_free(const struct test *t) { struct hash *h = hash_new(8, countfreecalls); From 026f6190648eb15173e7b4cb4cb84dc0adb9b15f Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 15 Nov 2024 23:24:30 +0100 Subject: [PATCH 3/8] libkmod: hash_del: fix out-of-bounds memory access Assume bucket->used is 1, and element 0 is deleted. We shouldn't access any memory above (entry + 1) in this case. Likewise, if bucked->used is 2, only one element should be shifted, etc. Fixes: 7db0865 ("Add simple hash implementation") Signed-off-by: Martin Wilck --- shared/hash.c | 2 +- testsuite/test-hash.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/shared/hash.c b/shared/hash.c index 38b7bb30..158d85d9 100644 --- a/shared/hash.c +++ b/shared/hash.c @@ -258,7 +258,7 @@ int hash_del(struct hash *hash, const char *key) hash->free_value((void *)entry->value); entry_end = bucket->entries + bucket->used; - memmove(entry, entry + 1, (entry_end - entry) * sizeof(struct hash_entry)); + memmove(entry, entry + 1, (entry_end - entry - 1) * sizeof(struct hash_entry)); bucket->used--; hash->count--; diff --git a/testsuite/test-hash.c b/testsuite/test-hash.c index 1ec820d1..b115a623 100644 --- a/testsuite/test-hash.c +++ b/testsuite/test-hash.c @@ -184,8 +184,7 @@ static int test_hash_del(const struct test *t) return 0; } -DEFINE_TEST(test_hash_del, .description = "test add / delete a single element", - .expected_fail = true); +DEFINE_TEST(test_hash_del, .description = "test add / delete a single element"); static int test_hash_free(const struct test *t) { From ae2c7cb45d797341d8d00c134b2e9bc16bb70012 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 21 Nov 2024 23:45:02 +0100 Subject: [PATCH 4/8] testsuite: test-hash: add a test for deleting a non-existing element This test fails and will be fixed by the next commit. Signed-off-by: Martin Wilck --- testsuite/test-hash.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/testsuite/test-hash.c b/testsuite/test-hash.c index b115a623..e9041117 100644 --- a/testsuite/test-hash.c +++ b/testsuite/test-hash.c @@ -186,6 +186,23 @@ static int test_hash_del(const struct test *t) } DEFINE_TEST(test_hash_del, .description = "test add / delete a single element"); +static int test_hash_del_nonexistent(const struct test *t) +{ + struct hash *h = hash_new(32, NULL); + const char *k1 = "k1"; + int rc; + + rc = hash_del(h, k1); + assert_return(rc == -ENOENT, EXIT_FAILURE); + + hash_free(h); + + return 0; +} +DEFINE_TEST(test_hash_del_nonexistent, + .description = "test deleting an element that doesn't exist", + .expected_fail = true); + static int test_hash_free(const struct test *t) { struct hash *h = hash_new(8, countfreecalls); From 57e75769e4d6f24f6f60c5a52ac1fdae581fc8d2 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 15 Nov 2024 23:25:15 +0100 Subject: [PATCH 5/8] libkmod: hash_del: handle deleting a non-existing element gracefully It can happen that bucket->entries is NULL, but bsearch is annotated such that it's second argument must be non-NULL. Fixes: 7db0865 ("Add simple hash implementation") Signed-off-by: Martin Wilck --- shared/hash.c | 3 +++ testsuite/test-hash.c | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/shared/hash.c b/shared/hash.c index 158d85d9..f9a0c00c 100644 --- a/shared/hash.c +++ b/shared/hash.c @@ -249,6 +249,9 @@ int hash_del(struct hash *hash, const char *key) .value = NULL, }; + if (bucket->entries == NULL) + return -ENOENT; + entry = bsearch(&se, bucket->entries, bucket->used, sizeof(struct hash_entry), hash_entry_cmp); if (entry == NULL) diff --git a/testsuite/test-hash.c b/testsuite/test-hash.c index e9041117..35cf06ed 100644 --- a/testsuite/test-hash.c +++ b/testsuite/test-hash.c @@ -200,8 +200,7 @@ static int test_hash_del_nonexistent(const struct test *t) return 0; } DEFINE_TEST(test_hash_del_nonexistent, - .description = "test deleting an element that doesn't exist", - .expected_fail = true); + .description = "test deleting an element that doesn't exist"); static int test_hash_free(const struct test *t) { From 5cfcc0febbcfec0932d6efd2ad5d18da2abeb43f Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 21 Nov 2024 23:56:10 +0100 Subject: [PATCH 6/8] testsuite: test_strbuf_pushmem: test pushing 0 bytes to an empty strbuf This test fails, and will be fixed by the next commit. Signed-off-by: Martin Wilck --- testsuite/test-strbuf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/testsuite/test-strbuf.c b/testsuite/test-strbuf.c index e77fbdb9..9f2dcf8f 100644 --- a/testsuite/test-strbuf.c +++ b/testsuite/test-strbuf.c @@ -191,6 +191,7 @@ static int test_strbuf_pushmem(const struct test *t) size_t size; strbuf_init(&buf); + strbuf_pushmem(&buf, "", 0); strbuf_reserve_extra(&buf, strlen(TEXT) + 1); size = buf.size; strbuf_pushmem(&buf, TEXT, strlen(TEXT) + 1); @@ -201,7 +202,8 @@ static int test_strbuf_pushmem(const struct test *t) return 0; } -DEFINE_TEST(test_strbuf_pushmem, .description = "test strbuf_reserve"); +DEFINE_TEST(test_strbuf_pushmem, .description = "test strbuf_reserve", + .expected_fail = true); static int test_strbuf_used(const struct test *t) { From de91a6409a4a13e41a07d66aec3b2bb0539cf277 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 15 Nov 2024 23:26:26 +0100 Subject: [PATCH 7/8] libkmod: strbuf_pushchars: handle pushing empty string gracefully If buf->bytes, buf->used, and len are all 0, buf_grow() will do nothing, and memcpy() willbe called with a NULL first argument. This will cause an error because the function is annotated with __nonnull(1, 2). Fixes: e62d8c7 ("strbuf: make strbuf_pushchars() a little less dumb") Signed-off-by: Martin Wilck --- shared/strbuf.c | 3 +++ testsuite/test-strbuf.c | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/shared/strbuf.c b/shared/strbuf.c index dad5da52..0dd7a5b0 100644 --- a/shared/strbuf.c +++ b/shared/strbuf.c @@ -100,6 +100,9 @@ size_t strbuf_pushmem(struct strbuf *buf, const char *src, size_t sz) assert(src != NULL); assert(buf != NULL); + if (sz == 0) + return 0; + if (!strbuf_reserve_extra(buf, sz)) return 0; diff --git a/testsuite/test-strbuf.c b/testsuite/test-strbuf.c index 9f2dcf8f..50c75d92 100644 --- a/testsuite/test-strbuf.c +++ b/testsuite/test-strbuf.c @@ -202,8 +202,7 @@ static int test_strbuf_pushmem(const struct test *t) return 0; } -DEFINE_TEST(test_strbuf_pushmem, .description = "test strbuf_reserve", - .expected_fail = true); +DEFINE_TEST(test_strbuf_pushmem, .description = "test strbuf_reserve"); static int test_strbuf_used(const struct test *t) { From eb15490b00e5dcf365d5cdf1d2cf12b902b5aca7 Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Fri, 22 Nov 2024 00:02:26 +0100 Subject: [PATCH 8/8] libkmod: hash_new(): always use a step size of at least 4 The current algorithm would choose a step size of 4 for n_buckets == 31, but 1 for n_buckets == 32, which seems wrong. Fix it. Signed-off-by: Martin Wilck --- shared/hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/hash.c b/shared/hash.c index f9a0c00c..d0e7f86d 100644 --- a/shared/hash.c +++ b/shared/hash.c @@ -41,7 +41,7 @@ struct hash *hash_new(unsigned int n_buckets, void (*free_value)(void *value)) hash->n_buckets = n_buckets; hash->free_value = free_value; hash->step = n_buckets / 32; - if (hash->step == 0) + if (hash->step < 4) hash->step = 4; else if (hash->step > 64) hash->step = 64;