Thread (21 messages) 21 messages, 4 authors, 2021-02-09

Re: [PATCH v4 15/15] dmaengine: dw-edma: Add pcim_iomap_table return checker

From: Krzysztof Wilczyński <hidden>
Date: 2021-02-09 19:46:01
Also in: linux-pci, lkml

Hi Gustavo,

[...]
quoted
This "pcim_iomap_table(dev)[n]" pattern is extremely common.  There
are over 100 calls of pcim_iomap_table(), and

  $ git grep "pcim_iomap_table(.*)\[.*\]" | wc -l

says about 75 of them are of this form, where we dereference the
result before testing it.
That's true, there are a lot of drivers that don't verify that pointer. 
What do you suggest?
1) To remove the verification so that is aligned with the other drivers
2) Leave it as is. Or even to add this verification to the other drivers?

Either way, I will add the pcim_iomap_table(pdev) before this 
instruction.
[...]

A lot of the drivers consume the value from pcim_iomap_table() at
a given BAR index directly as-is, some check if the pointer they got
back is not NULL, a very few also check if the address at a given index
is not NULL.

Given that the memory allocation for the table can fail, we ought to
check for a NULL pointer.  It's a bit worrying that people decided to
consume the value it returns directly without any verification.

I only found two drivers that perform this additional verification of
checking whether the address at a given index is valid, as per:

  https://lore.kernel.org/linux-pci/YCLFTjZQ2bCfGC+J@rocinante/ (local)

Personally, I would opt for (2), and then like you suggested send
a separate series to update other drivers so that they also include the
this NULL pointer check.

But let's wait for Bjorn's take on this, though.

Krzysztof
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help