Re: [PATCH v3 19/57] scsi: bnx2fc: Enable lock context analysis

[email protected] Thu, 30 Jul 2026 23:01:00 +0000
Newsgroups org.kernel.vger.linux-scsi
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional evaluation of nested pointer chain in `__assume_ctx_lock()` causes a potential fatal NULL pointer dereference for standard SCSI commands.
--

--- Patch [19]: [PATCH v3 19/57] scsi: bnx2fc: Enable lock context analysis ---
commit 914539bc4256a83053d649d7450d8f075fb16ba8
Author: Bart Van Assche <[email protected]>

scsi: bnx2fc: Enable lock context analysis
    
This patch enables context lock analysis by documenting locking requirements
and informing the compiler about lock aliases. It updates functions with
__must_hold() and __assume_ctx_lock() to track context around target locks.
It also standardizes an argument name in bnx2fc_process_seq_cleanup_compl().

> diff --git a/drivers/scsi/bnx2fc/bnx2fc_hwi.c b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> index a5ecb87d5b2d7..a3670c48900ba 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_hwi.c
> @@ -880,6 +880,9 @@ void bnx2fc_process_cq_compl(struct bnx2fc_rport *tgt, u16 wqe,
>  		return;
>  	}
>  
> +	/* Tell the compiler that there is an alias for tgt->tgt_lock. */
> +	__assume_ctx_lock(&io_req->cb_arg->aborted_io_req->tgt->tgt_lock);

[Severity: High]
Does this unconditionally evaluate io_req->cb_arg?

For standard SCSI commands, io_req->cb_arg is NULL.

Since the kernel is compiled with -fno-delete-null-pointer-checks, computing
the address of tgt_lock requires evaluating the intermediate pointers.

This means the compiler is highly likely to emit a memory load rather than
optimizing away the dead read, since it treats address 0 as valid.

Could this lead to a fatal NULL pointer dereference on every standard IO
completion processed by bnx2fc_process_cq_compl()?

It appears this annotation might belong solely inside the BNX2FC_SEQ_CLEANUP
case where cb_arg is known to be valid.

> +
>  	/* Timestamp IO completion time */
>  	cmd_type = io_req->cmd_type;
>

-- 
Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=19