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