@@ -496,6 +496,7 @@ static unsigned int __init print_ht(struct rhltable *rhlt)
struct rhashtable *ht;
const struct bucket_table *tbl;
char buff[512] = "";
+ size_t len = 0;
unsigned int i, cnt = 0;
ht = &rhlt->ht;
@@ -508,19 +509,19 @@ static unsigned int __init print_ht(struct rhltable *rhlt)
next = !rht_is_a_nulls(pos) ? rht_dereference(pos->next, ht) : NULL;
if (!rht_is_a_nulls(pos)) {
- sprintf(buff, "%s\nbucket[%d] -> ", buff, i);
+ len += scnprintf(buff + len, sizeof(buff) - len, "\nbucket[%d] -> ", i);
}
while (!rht_is_a_nulls(pos)) {
struct rhlist_head *list = container_of(pos, struct rhlist_head, rhead);
- sprintf(buff, "%s[[", buff);
+ len += scnprintf(buff + len, sizeof(buff) - len, "[[");
do {
pos = &list->rhead;
list = rht_dereference(list->next, ht);
p = rht_obj(ht, pos);
- sprintf(buff, "%s val %d (tid=%d)%s", buff, p->value.id, p->value.tid,
- list? ", " : " ");
+ len += scnprintf(buff + len, sizeof(buff) - len, " val %d (tid=%d)%s",
+ p->value.id, p->value.tid, list? ", " : " ");
cnt++;
} while (list);
@@ -528,7 +529,8 @@ static unsigned int __init print_ht(struct rhltable *rhlt)
next = !rht_is_a_nulls(pos) ?
rht_dereference(pos->next, ht) : NULL;
- sprintf(buff, "%s]]%s", buff, !rht_is_a_nulls(pos) ? " -> " : "");
+ len += scnprintf(buff + len, sizeof(buff) - len, "]]%s",
+ !rht_is_a_nulls(pos) ? " -> " : "");
}
}
printk(KERN_ERR "\n---- ht: ----%s\n-------------\n", buff);
gcc-8 warns about a code pattern that is used in the newly added test_rhashtable code: lib/test_rhashtable.c: In function 'print_ht': lib/test_rhashtable.c:511:21: error: ' bucket[' directive writing 8 bytes into a region of size between 1 and 512 [-Werror=format-overflow=] sprintf(buff, "%s\nbucket[%d] -> ", buff, i); ^~~~~~~~~ lib/test_rhashtable.c:511:4: note: 'sprintf' output between 15 and 536 bytes into a destination of size 512 sprintf(buff, "%s\nbucket[%d] -> ", buff, i); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The problem here is using the same fixed-length buffer as input and output of snprintf(), which for an unbounded loop has an actual potential to overflow the buffer. The '512' byte length was apparently chosen to be "long enough" to prevent that in practice, but without any specific guarantees of being the smallest safe size. I can see three possible ways to avoid this warning: - rewrite the code to use pointer arithmetic to forward the buffer, rather than copying the buffer itself. This is a more conventional use of sprintf(), and it avoids the warning, but is not any more safe than the original code. - Rewrite the function in a safe way that avoids both the potential overflow and the warning. - Ask the gcc developers to not warn for this pattern if we consider the warning to be inappropriate. This patch implements the second of the above, using scnprintf() as Rasmus suggested. Fixes: 499ac3b60f65 ("test_rhashtable: add test case for rhltable with duplicate objects") Cc: Martin Sebor <msebor@gcc.gnu.org> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- My patch is still untested, please try it out before applying. --- lib/test_rhashtable.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) -- 2.9.0