On Sun, Mar 02, 2025 at 05:55:40PM +0100, Markus Elfring wrote:
> From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> 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 <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/net/ethernet/qlogic/qed/qed_ll2.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
> index 717a0b3f89bd..941c02fccaaf 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
> @@ -346,7 +346,7 @@ static void qed_ll2_txq_flush(struct qed_hwfn *p_hwfn, u8 connection_handle)
> static int qed_ll2_txq_completion(struct qed_hwfn *p_hwfn, void *p_cookie)
> {
> struct qed_ll2_info *p_ll2_conn = p_cookie;
> - struct qed_ll2_tx_queue *p_tx = &p_ll2_conn->tx_queue;
> + struct qed_ll2_tx_queue *p_tx;
> u16 new_idx = 0, num_bds = 0, num_bds_in_packet = 0;
> struct qed_ll2_tx_packet *p_pkt;
> bool b_last_frag = false;
> @@ -356,6 +356,7 @@ static int qed_ll2_txq_completion(struct qed_hwfn *p_hwfn, void *p_cookie)
> if (!p_ll2_conn)
> return rc;
>
> + p_tx = &p_ll2_conn->tx_queue;
> spin_lock_irqsave(&p_tx->lock, flags);
> if (p_tx->b_completing_packet) {
> rc = -EBUSY;
> @@ -523,7 +524,7 @@ qed_ll2_rxq_handle_completion(struct qed_hwfn *p_hwfn,
> static int qed_ll2_rxq_completion(struct qed_hwfn *p_hwfn, void *cookie)
> {
> struct qed_ll2_info *p_ll2_conn = (struct qed_ll2_info *)cookie;
> - struct qed_ll2_rx_queue *p_rx = &p_ll2_conn->rx_queue;
> + struct qed_ll2_rx_queue *p_rx;
> union core_rx_cqe_union *cqe = NULL;
> u16 cq_new_idx = 0, cq_old_idx = 0;
> unsigned long flags = 0;
> @@ -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.
Anyway, with heaving this check for p_ll2_conn it is good to move
assigment after this check.
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@xxxxxxxxxxxxxxx>
> --
> 2.40.0
