On 11/15/22 07:03, Dan Carpenter wrote:
Hello Alex Elder,
This is a semi-automatic email about new static checker warnings.
The patch 5cb76899fb47: "net: ipa: reduce arguments to
ipa_table_init_add()" from Nov 2, 2022, leads to the following Smatch
complaint:
drivers/net/ipa/ipa_table.c:423 ipa_table_init_add()
error: we previously assumed 'hash_mem' could be null (see line 414)
This is a nice catch. It turns out that the issue pointed out
is not a problem in practice. The 8 possible memory regions
that could be used are (currently) always defined, so both mem
and hash_mem will be non-null. Nevertheless Smatch identifies
a real problem with the code in this function, and I will fix
that.
IN ADDITION... This report forced me to examine this patch once
more, and I now see a different bug from what was reported,
which I'll point out below.
I intend to fix both problems with one patch.
drivers/net/ipa/ipa_table.c
413 count = mem->size / sizeof(__le64);
414 hash_count = hash_mem && hash_mem->size / sizeof(__le64);
Line 414 is wrong. It should be:
hash_count = hash_mem ? hash_mem->size / sizeof(__le64) : 0;
As it is now, the count will be 1 or 0, and it should be the
number of entries that fit in the region (or 0 if hash_mem is
a null pointer).
I did test this during development and verified the counts were
correct, HOWEVER I must have tested this on a system that did
not support hashed tables (IPA v4.2). I'm very happy for this
Smatch report.
-Alex
^^^^^^^^
The patch adds checks for NULL.
415 }
416 size = count * sizeof(__le64);
417 hash_size = hash_count * sizeof(__le64);
418
419 addr = ipa_table_addr(ipa, filter, count);
420 hash_addr = ipa_table_addr(ipa, filter, hash_count);
421
422 ipa_cmd_table_init_add(trans, opcode, size, mem->offset, addr,
423 hash_size, hash_mem->offset, hash_addr);
^^^^^^^^^^^^^^^^
Unchecked dereference.
424 if (!filter)
425 return;
regards,
dan carpenter
