From: Markus Elfring <hidden> Date: 2025-03-02 16:56:17
From: Markus Elfring <redacted>
Date: Tue, 11 Apr 2023 19:33:53 +0200
The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the functions “qed_ll2_rxq_completion” and “qed_ll2_txq_completion”.
Thus avoid the risk for undefined behaviour by moving the assignment
for the variables “p_rx” and “p_tx” behind the null pointer check.
This issue was detected by using the Coccinelle software.
Fixes: 0a7fb11c23c0 ("qed: Add Light L2 support")
Signed-off-by: Markus Elfring <redacted>
---
drivers/net/ethernet/qlogic/qed/qed_ll2.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
From: Michal Swiatkowski <hidden> Date: 2025-03-03 06:25:20
On Sun, Mar 02, 2025 at 05:55:40PM +0100, Markus Elfring wrote:
quoted hunk
From: Markus Elfring <redacted>
Date: Tue, 11 Apr 2023 19:33:53 +0200
The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the functions “qed_ll2_rxq_completion” and “qed_ll2_txq_completion”.
Thus avoid the risk for undefined behaviour by moving the assignment
for the variables “p_rx” and “p_tx” behind the null pointer check.
This issue was detected by using the Coccinelle software.
Fixes: 0a7fb11c23c0 ("qed: Add Light L2 support")
Signed-off-by: Markus Elfring <redacted>
---
drivers/net/ethernet/qlogic/qed/qed_ll2.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
For future submission plase specify the target kernel
[PATCH net] for fixes, [PATCH net-next] for other.
Looking at the code callback is always set together with cookie (which
is pointing to p_ll2_conn. I am not sure if this is fixing real issue,
but maybe there are a cases when callback is still connected and cookie
is NULL.
Anyway, with heaving this check for p_ll2_conn it is good to move
assigment after this check.
Reviewed-by: Michal Swiatkowski <redacted>
@@ -532,6 +533,7 @@ static int qed_ll2_rxq_completion(struct qed_hwfn *p_hwfn, void *cookie) if (!p_ll2_conn) return rc;+ p_rx = &p_ll2_conn->rx_queue; spin_lock_irqsave(&p_rx->lock, flags); if (!QED_LL2_RX_REGISTERED(p_ll2_conn)) {
For future submission plase specify the target kernel
[PATCH net] for fixes, [PATCH net-next] for other.
Looking at the code callback is always set together with cookie (which
is pointing to p_ll2_conn. I am not sure if this is fixing real issue,
but maybe there are a cases when callback is still connected and cookie
is NULL.
The assignment:
p_rx = &p_ll2_conn->rx_queue;
does not dereference "p_ll2_conn". It just does pointer math. So the
original code works fine.
The question is, did the original author write it that way intentionally?
I've had this conversation with people before and they eventually are
convinced that "Okay, the code works, but only by sheer chance." In my
experiences, most of the time, the authors of this type of code are so
used to weirdnesses of C that they write code like this without even
thinking about it. It never even occurs to them how it could be
confusing.
This commit message is misleading and there should not be a Fixes tag.
regards,
dan carpenter
From: Markus Elfring <hidden> Date: 2025-03-03 10:26:12
This is not a NULL dereference. It's just pointer math.
How does your view fit to information in an article like “Fun with NULL pointers, part 1”
(by Jonathan Corbet from 2009-07-20)?
https://lwn.net/Articles/342330/
Regards,
Markus
This is not a NULL dereference. It's just pointer math.
regards,
dan carpenter
Feel free to ignore Markus, he has a long history of sending unhelpful reviews,
comments or patches and continues to ignore repeated requests to stop. He is
already on the ignore lists of several maintainers.
There is a lot of chance that he is a bot.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com