From: Guenter Roeck <linux@roeck-us.net>
To: Nilesh Javali <njavali@marvell.com>
Cc: martin.petersen@oracle.com, linux-scsi@vger.kernel.org,
GR-QLogic-Storage-Upstream@marvell.com, agurumurthy@marvell.com,
sdeodhar@marvell.com, emilne@redhat.com, jmeneghi@redhat.com
Subject: Re: [PATCH v3 11/12] qla2xxx: fix bsg_done causing double free
Date: Mon, 2 Mar 2026 03:17:48 -0800 [thread overview]
Message-ID: <2a2859ea-4815-48bf-bb91-07fd06b73f4a@roeck-us.net> (raw)
In-Reply-To: <20251210101604.431868-12-njavali@marvell.com>
Hi,
On Wed, Dec 10, 2025 at 03:46:03PM +0530, Nilesh Javali wrote:
> From: Anil Gurumurthy <agurumurthy@marvell.com>
>
> Kernel panic observed on system,
>
> [5353358.825191] BUG: unable to handle page fault for address: ff5f5e897b024000
> [5353358.825194] #PF: supervisor write access in kernel mode
> [5353358.825195] #PF: error_code(0x0002) - not-present page
> [5353358.825196] PGD 100006067 P4D 0
> [5353358.825198] Oops: 0002 [#1] PREEMPT SMP NOPTI
> [5353358.825200] CPU: 5 PID: 2132085 Comm: qlafwupdate.sub Kdump: loaded Tainted: G W L ------- --- 5.14.0-503.34.1.el9_5.x86_64 #1
> [5353358.825203] Hardware name: HPE ProLiant DL360 Gen11/ProLiant DL360 Gen11, BIOS 2.44 01/17/2025
> [5353358.825204] RIP: 0010:memcpy_erms+0x6/0x10
> [5353358.825211] RSP: 0018:ff591da8f4f6b710 EFLAGS: 00010246
> [5353358.825212] RAX: ff5f5e897b024000 RBX: 0000000000007090 RCX: 0000000000001000
> [5353358.825213] RDX: 0000000000001000 RSI: ff591da8f4fed090 RDI: ff5f5e897b024000
> [5353358.825214] RBP: 0000000000010000 R08: ff5f5e897b024000 R09: 0000000000000000
> [5353358.825215] R10: ff46cf8c40517000 R11: 0000000000000001 R12: 0000000000008090
> [5353358.825216] R13: ff591da8f4f6b720 R14: 0000000000001000 R15: 0000000000000000
> [5353358.825218] FS: 00007f1e88d47740(0000) GS:ff46cf935f940000(0000) knlGS:0000000000000000
> [5353358.825219] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [5353358.825220] CR2: ff5f5e897b024000 CR3: 0000000231532004 CR4: 0000000000771ef0
> [5353358.825221] PKRU: 55555554
> [5353358.825222] Call Trace:
> [5353358.825223] <TASK>
> [5353358.825224] ? show_trace_log_lvl+0x1c4/0x2df
> [5353358.825229] ? show_trace_log_lvl+0x1c4/0x2df
> [5353358.825232] ? sg_copy_buffer+0xc8/0x110
> [5353358.825236] ? __die_body.cold+0x8/0xd
> [5353358.825238] ? page_fault_oops+0x134/0x170
> [5353358.825242] ? kernelmode_fixup_or_oops+0x84/0x110
> [5353358.825244] ? exc_page_fault+0xa8/0x150
> [5353358.825247] ? asm_exc_page_fault+0x22/0x30
> [5353358.825252] ? memcpy_erms+0x6/0x10
> [5353358.825253] sg_copy_buffer+0xc8/0x110
> [5353358.825259] qla2x00_process_vendor_specific+0x652/0x1320 [qla2xxx]
> [5353358.825317] qla24xx_bsg_request+0x1b2/0x2d0 [qla2xxx]
>
> Most routines in qla_bsg.c call bsg_done only for
> success cases.
> However a few invoke it for failure case as well
> leading to a double free. Validate before calling bsg_done.
qla24xx_bsg_request() is called from fc_bsg_host_dispatch(), which
indeed calls bsg_job_done() on errors. However, it does a bit more:
It sets reply_payload_rcv_len to 0 and reply_len to sizeof(uint32_t).
This means that the result from the functions below is now dropped.
Yet, several of those functions explicitly set values in the reply
if ret != 0 (which may explain why bsg_done() was called even with
ret != 0). Is this ok/acceptable ?
Thanks,
Guenter
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Anil Gurumurthy <agurumurthy@marvell.com>
> Signed-off-by: Nilesh Javali <njavali@marvell.com>
> Reviewed-by: Himanshu Madhani <hmadhani2024@gmail.com>
> ---
> drivers/scsi/qla2xxx/qla_bsg.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> index 8afa8a4b8ccb..2c44a379cb23 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> @@ -1548,8 +1548,9 @@ qla2x00_update_optrom(struct bsg_job *bsg_job)
> ha->optrom_buffer = NULL;
> ha->optrom_state = QLA_SWAITING;
> mutex_unlock(&ha->optrom_mutex);
> - bsg_job_done(bsg_job, bsg_reply->result,
> - bsg_reply->reply_payload_rcv_len);
> + if (!rval)
> + bsg_job_done(bsg_job, bsg_reply->result,
> + bsg_reply->reply_payload_rcv_len);
> return rval;
> }
>
> @@ -2638,8 +2639,9 @@ qla2x00_manage_host_stats(struct bsg_job *bsg_job)
> sizeof(struct ql_vnd_mng_host_stats_resp));
>
> bsg_reply->result = DID_OK;
> - bsg_job_done(bsg_job, bsg_reply->result,
> - bsg_reply->reply_payload_rcv_len);
> + if (!ret)
> + bsg_job_done(bsg_job, bsg_reply->result,
> + bsg_reply->reply_payload_rcv_len);
>
> return ret;
> }
> @@ -2728,8 +2730,9 @@ qla2x00_get_host_stats(struct bsg_job *bsg_job)
> bsg_job->reply_payload.sg_cnt,
> data, response_len);
> bsg_reply->result = DID_OK;
> - bsg_job_done(bsg_job, bsg_reply->result,
> - bsg_reply->reply_payload_rcv_len);
> + if (!ret)
> + bsg_job_done(bsg_job, bsg_reply->result,
> + bsg_reply->reply_payload_rcv_len);
>
> kfree(data);
> host_stat_out:
> @@ -2828,8 +2831,9 @@ qla2x00_get_tgt_stats(struct bsg_job *bsg_job)
> bsg_job->reply_payload.sg_cnt, data,
> response_len);
> bsg_reply->result = DID_OK;
> - bsg_job_done(bsg_job, bsg_reply->result,
> - bsg_reply->reply_payload_rcv_len);
> + if (!ret)
> + bsg_job_done(bsg_job, bsg_reply->result,
> + bsg_reply->reply_payload_rcv_len);
>
> tgt_stat_out:
> kfree(data);
> @@ -2890,8 +2894,9 @@ qla2x00_manage_host_port(struct bsg_job *bsg_job)
> bsg_job->reply_payload.sg_cnt, &rsp_data,
> sizeof(struct ql_vnd_mng_host_port_resp));
> bsg_reply->result = DID_OK;
> - bsg_job_done(bsg_job, bsg_reply->result,
> - bsg_reply->reply_payload_rcv_len);
> + if (!ret)
> + bsg_job_done(bsg_job, bsg_reply->result,
> + bsg_reply->reply_payload_rcv_len);
>
> return ret;
> }
> @@ -3272,7 +3277,8 @@ int qla2x00_mailbox_passthru(struct bsg_job *bsg_job)
>
> bsg_job->reply_len = sizeof(*bsg_job->reply);
> bsg_reply->result = DID_OK << 16;
> - bsg_job_done(bsg_job, bsg_reply->result, bsg_reply->reply_payload_rcv_len);
> + if (!ret)
> + bsg_job_done(bsg_job, bsg_reply->result, bsg_reply->reply_payload_rcv_len);
>
> kfree(req_data);
>
> @@ -3359,8 +3365,9 @@ static int qla28xx_validate_flash_image(struct bsg_job *bsg_job)
> bsg_reply->result = DID_OK << 16;
> bsg_reply->reply_payload_rcv_len = 0;
> bsg_job->reply_len = sizeof(struct fc_bsg_reply);
> - bsg_job_done(bsg_job, bsg_reply->result,
> - bsg_reply->reply_payload_rcv_len);
> + if (!rval)
> + bsg_job_done(bsg_job, bsg_reply->result,
> + bsg_reply->reply_payload_rcv_len);
>
> return QLA_SUCCESS;
> }
> --
> 2.23.1
>
next prev parent reply other threads:[~2026-03-02 11:17 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-10 10:15 [PATCH v3 00/12] qla2xxx: Misc feature and bug fixes Nilesh Javali
2025-12-10 10:15 ` [PATCH v3 01/12] qla2xxx: Add Speed in SFP print information Nilesh Javali
2025-12-10 10:15 ` [PATCH v3 02/12] qla2xxx: Add support for 64G SFP speed Nilesh Javali
2025-12-10 10:15 ` [PATCH v3 03/12] qla2xxx: Add load flash firmware mailbox support for 28xxx Nilesh Javali
2025-12-10 10:15 ` [PATCH v3 04/12] qla2xxx: Validate MCU signature before executing MBC 03h Nilesh Javali
2025-12-10 10:15 ` [PATCH v3 05/12] qla2xxx: Add bsg interface to support firmware img validation Nilesh Javali
2025-12-10 10:15 ` [PATCH v3 06/12] qla2xxx: Allow recovery for tape devices Nilesh Javali
2025-12-10 10:15 ` [PATCH v3 07/12] qla2xxx: Delay module unload while fabric scan in progress Nilesh Javali
2025-12-10 10:16 ` [PATCH v3 08/12] qla2xxx: free sp in error path to fix system crash Nilesh Javali
2026-02-25 18:28 ` Guenter Roeck
2025-12-10 10:16 ` [PATCH v3 09/12] qla2xxx: validate sp before freeing associated memory Nilesh Javali
2025-12-10 10:16 ` [PATCH v3 10/12] qla2xxx: Query FW again before proceeding with login Nilesh Javali
2025-12-10 10:16 ` [PATCH v3 11/12] qla2xxx: fix bsg_done causing double free Nilesh Javali
2026-03-02 11:17 ` Guenter Roeck [this message]
2025-12-10 10:16 ` [PATCH v3 12/12] qla2xxx: Update version to 10.02.10.100-k Nilesh Javali
2025-12-17 3:39 ` [PATCH v3 00/12] qla2xxx: Misc feature and bug fixes Martin K. Petersen
2026-01-04 21:43 ` Martin K. Petersen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2a2859ea-4815-48bf-bb91-07fd06b73f4a@roeck-us.net \
--to=linux@roeck-us.net \
--cc=GR-QLogic-Storage-Upstream@marvell.com \
--cc=agurumurthy@marvell.com \
--cc=emilne@redhat.com \
--cc=jmeneghi@redhat.com \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=njavali@marvell.com \
--cc=sdeodhar@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox