[PATCH] scsi: qla2xxx: Rework BUILD_BUG_ON() assertion

[PATCH] scsi: qla2xxx: Rework BUILD_BUG_ON() assertion

[Finn Thain]

The LKP bot reported a build failure with CONFIG_COLDFIRE=y together with
CONFIG_SCSI_QLA_FC=y, that is attributable to the BUILD_BUG_ON() in
qlt_queue_unknown_atio().

That function uses kzalloc() to obtain memory for the following struct,
plus some extra bytes at the end.

struct qla_tgt_sess_op {
        struct scsi_qla_host *vha;
        uint32_t chip_reset;
        struct work_struct work;
        struct list_head cmd_list;
        bool aborted;
        struct rsp_que *rsp;

        struct atio_from_isp atio;
        /* DO NOT ADD ANYTHING ELSE HERE - atio must be last member */
};

The location of the 'atio' member is subsequently used as the destination
for a memcpy() that's expected to fill in the extra bytes beyond the end
of the struct.

That explains the loud warning in the comment above, which ought to be
sufficient to prevent some newly-added member from accidentally getting
clobbered. But, in case that warning was missed somehow, we also have the
failing assertion,

BUILD_BUG_ON(offsetof(struct qla_tgt_sess_op, atio) + sizeof(u->atio) !=
             sizeof(*u));

Unfortunately, this size assertion doesn't guarantee that 'atio' is the
last member. Indeed, adding a zero-length array member at the end does
not increase the struct size.

Moreover, the assertion can fail even when 'atio' really is the last
member, and that's what happened with commit e428b013d9df ("atomic:
specify alignment for atomic_t and atomic64_t"), which added 2 bytes of
harmless padding to the end of the struct.

To resolve those issues, place a flex array at the end of struct
qla_tgt_sess_op (as any member after the flex array would result in a
compiler error) and then use the BUILD_BUG_ON to ensure that the 'atio'
member ends at the offset of the flex array (as compilers aren't expected
to place any padding between the two members that would mess up this
calculation).

Cc: Tony Battersby <tonyb@cybernetics.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: linux-m68k@lists.linux-m68k.org
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202603030747.VX0v4otS-lkp@intel.com/
Fixes: 091719c21d5a ("scsi: qla2xxx: target: Fix invalid memory access with big CDBs")
Fixes: e428b013d9df ("atomic: specify alignment for atomic_t and atomic64_t").
Suggested-by: Tony Battersby <tonyb@cybernetics.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
This patch is submitted as a possible alternative to "[PATCH] scsi: qla2xxx:
Remove problematic BUILD_BUG_ON() assertion", dated 2026-03-06.
Either one would do the job. Compile-tested only.
---
 drivers/scsi/qla2xxx/qla_target.c | 5 +++--
 drivers/scsi/qla2xxx/qla_target.h | 9 +++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index d772136984c9..eb1de988f69c 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -212,8 +212,9 @@ static void qlt_queue_unknown_atio(scsi_qla_host_t *vha,
 	unsigned long flags;
 	unsigned int add_cdb_len = 0;
 
-	/* atio must be the last member of qla_tgt_sess_op for add_cdb_len */
-	BUILD_BUG_ON(offsetof(struct qla_tgt_sess_op, atio) + sizeof(u->atio) != sizeof(*u));
+	/* atio_u_isp24_fcp_cmnd_add_cdb follows immediately after atio */
+	BUILD_BUG_ON(offsetof(struct qla_tgt_sess_op, atio) + sizeof(struct atio_from_isp) !=
+		     offsetof(struct qla_tgt_sess_op, atio_u_isp24_fcp_cmnd_add_cdb));
 
 	if (tgt->tgt_stop) {
 		ql_dbg(ql_dbg_async, vha, 0x502c,
diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index 61072fb41b29..11a406ee2187 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -309,7 +309,8 @@ struct atio7_fcp_cmnd {
 	/*
 	 * add_cdb is optional and can absent from struct atio7_fcp_cmnd. Size 4
 	 * only to make sizeof(struct atio7_fcp_cmnd) be as expected by
-	 * BUILD_BUG_ON in qlt_init().
+	 * BUILD_BUG_ON in tcm_qla2xxx_init(). See also, BUILD_BUG_ON in
+	 * qlt_queue_unknown_atio().
 	 */
 	uint8_t  add_cdb[4];
 	/* __le32	data_length; */
@@ -845,7 +846,11 @@ struct qla_tgt_sess_op {
 	struct rsp_que *rsp;
 
 	struct atio_from_isp atio;
-	/* DO NOT ADD ANYTHING ELSE HERE - atio must be last member */
+	/*
+	 * DO NOT ADD ANYTHING ELSE HERE.
+	 * atio.u.isp24.fcp_cmnd.add_cdb may extend past end of atio.
+	 */
+	uint8_t atio_u_isp24_fcp_cmnd_add_cdb[];
 };
 
 enum trace_flags {
-- 
2.49.1

[PATCH] scsi: qla2xxx: Rework BUILD_BUG_ON() assertion

[Finn Thain]

The LKP bot reported a build failure with CONFIG_COLDFIRE=y together with
CONFIG_SCSI_QLA_FC=y. The failure is caused by BUILD_BUG_ON() but there's
no valid reason for it -- the assertion in qlt_queue_unknown_atio() is
simply checking the wrong thing.

That function uses kzalloc() to obtain memory for the following struct,
plus some extra bytes at the end.

struct qla_tgt_sess_op {
        struct scsi_qla_host *vha;
        uint32_t chip_reset;
        struct work_struct work;
        struct list_head cmd_list;
        bool aborted;
        struct rsp_que *rsp;

        struct atio_from_isp atio;
        /* DO NOT ADD ANYTHING ELSE HERE - atio must be last member */
};

The location of the last member is subsequently used as the destination
for a memcpy() that's expected to fill in the extra bytes beyond the end
of the struct, which explains the loud warning in the comment above.

That ought to be sufficient to prevent some newly-added member from
accidentally getting clobbered. But, in case that comment was missed
somehow, we have this assertion:

BUILD_BUG_ON(offsetof(struct qla_tgt_sess_op, atio) + sizeof(u->atio) !=
             sizeof(*u));

Unfortunately, this doesn't guarantee that 'atio' is the last member.
Indeed, adding a zero-length array member at the end does not increase
the struct size. Moreover, that BUILD_BUG_ON can fail even when 'atio'
really is the last member, and that's what happened after commit
e428b013d9df ("atomic: specify alignment for atomic_t and atomic64_t")
which added two bytes of harmless padding to the end of the struct.

To resolve those issues, place a flex array at the end of struct
qla_tgt_sess_op (any new member added after a flex array will produce a
compiler error). Then have the BUILD_BUG_ON correctly assert that the
'atio' member ends at the offset of the flex array (compilers won't insert
any padding there).

Cc: Gustavo Silva <gustavoars@kernel.org>
Cc: Tony Battersby <tonyb@cybernetics.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: linux-m68k@lists.linux-m68k.org
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202603030747.VX0v4otS-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202609301230.CauIUXCF-lkp@intel.com/
Fixes: 091719c21d5a ("scsi: qla2xxx: target: Fix invalid memory access with big CDBs")
Fixes: e428b013d9df ("atomic: specify alignment for atomic_t and atomic64_t").
Suggested-by: Tony Battersby <tonyb@cybernetics.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
The build bot prodded me again me again about this last week, so I'm
re-sending the same fix I sent on March 10th, with a few clarifications
made to the patch description.

The use of a flex array here is not unlike the work that Gustavo Silva
did last year on this driver, so I've added Gustavo to the Cc list.

I don't have the necessary hardware to regression test this patch.
However, the only change to object code comes from __LINE__ values.
If the new line break is omitted there's no change at all. So this fix
looks quite safe to me.
---
 drivers/scsi/qla2xxx/qla_target.c | 5 +++--
 drivers/scsi/qla2xxx/qla_target.h | 9 +++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 53a505df8da0..44720f5c0346 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -212,8 +212,9 @@ static void qlt_queue_unknown_atio(scsi_qla_host_t *vha,
 	unsigned long flags;
 	unsigned int add_cdb_len = 0;
 
-	/* atio must be the last member of qla_tgt_sess_op for add_cdb_len */
-	BUILD_BUG_ON(offsetof(struct qla_tgt_sess_op, atio) + sizeof(u->atio) != sizeof(*u));
+	/* atio_u_isp24_fcp_cmnd_add_cdb follows immediately after atio */
+	BUILD_BUG_ON(offsetof(struct qla_tgt_sess_op, atio) + sizeof(struct atio_from_isp) !=
+		     offsetof(struct qla_tgt_sess_op, atio_u_isp24_fcp_cmnd_add_cdb));
 
 	if (tgt->tgt_stop) {
 		ql_dbg(ql_dbg_async, vha, 0x502c,
diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index 61072fb41b29..11a406ee2187 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -309,7 +309,8 @@ struct atio7_fcp_cmnd {
 	/*
 	 * add_cdb is optional and can absent from struct atio7_fcp_cmnd. Size 4
 	 * only to make sizeof(struct atio7_fcp_cmnd) be as expected by
-	 * BUILD_BUG_ON in qlt_init().
+	 * BUILD_BUG_ON in tcm_qla2xxx_init(). See also, BUILD_BUG_ON in
+	 * qlt_queue_unknown_atio().
 	 */
 	uint8_t  add_cdb[4];
 	/* __le32	data_length; */
@@ -845,7 +846,11 @@ struct qla_tgt_sess_op {
 	struct rsp_que *rsp;
 
 	struct atio_from_isp atio;
-	/* DO NOT ADD ANYTHING ELSE HERE - atio must be last member */
+	/*
+	 * DO NOT ADD ANYTHING ELSE HERE.
+	 * atio.u.isp24.fcp_cmnd.add_cdb may extend past end of atio.
+	 */
+	uint8_t atio_u_isp24_fcp_cmnd_add_cdb[];
 };
 
 enum trace_flags {

Re: [PATCH] scsi: qla2xxx: Rework BUILD_BUG_ON() assertion

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: qla2xxx: FORTIFY_SOURCE field-spanning write warning in qlt_queue_unknown_atio
- [High] scsi: qla2xxx: UBSAN/FORTIFY overflow in add_cdb array accesses
--

commit 4d96d5f0f41af34ce19bfbd80ce5e059ab28cd98
Author: Finn Thain <fthain@linux-m68k.org>

scsi: qla2xxx: Rework BUILD_BUG_ON() assertion

The LKP bot reported a build failure with CONFIG_COLDFIRE=y together with
CONFIG_SCSI_QLA_FC=y. The failure is caused by an incorrect BUILD_BUG_ON()
assertion in qlt_queue_unknown_atio(). To resolve this, a flex array is
placed at the end of struct qla_tgt_sess_op.

Closes: https://lore.kernel.org/oe-kbuild-all/202603030747.VX0v4otS-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202609301230.CauIUXCF-lkp@intel.com/

> diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
> index 71bc1affb1bc9..b27bd3aecbf8e 100644
> --- a/drivers/scsi/qla2xxx/qla_target.h
> +++ b/drivers/scsi/qla2xxx/qla_target.h
> @@ -309,7 +309,8 @@ struct atio7_fcp_cmnd {
>  	/*
>  	 * add_cdb is optional and can absent from struct atio7_fcp_cmnd. Size 4
>  	 * only to make sizeof(struct atio7_fcp_cmnd) be as expected by
> -	 * BUILD_BUG_ON in qlt_init().
> +	 * BUILD_BUG_ON in tcm_qla2xxx_init(). See also, BUILD_BUG_ON in
> +	 * qlt_queue_unknown_atio().
>  	 */
>  	uint8_t  add_cdb[4];

[Severity: High]
Since add_cdb is explicitly declared as a fixed-size 4-byte array here, does
the current code cause UBSAN array bounds violations and FORTIFY_SOURCE read
overflow panics when accessing elements beyond these bounds?

For instance, in memcpy_atio() when len > 0, it accesses
&src->u.isp24.fcp_cmnd.add_cdb[len * 4] which is out-of-bounds, and reading
4 bytes triggers the overflow panic:

drivers/scsi/qla2xxx/qla_target.c:memcpy_atio() {
...
    memcpy(&dst->u.isp24.fcp_cmnd.add_cdb[0],
           &src->u.isp24.fcp_cmnd.add_cdb[len * 4],
           4);
...
}

Similarly, get_datalen_for_atio() accesses the array out-of-bounds when
len > 0:

drivers/scsi/qla2xxx/qla_target.h:get_datalen_for_atio() {
...
    return get_unaligned_be32(&atio->u.isp24.fcp_cmnd.add_cdb[len * 4]);
...
}

[ ... ]

> @@ -845,7 +846,11 @@ struct qla_tgt_sess_op {
>  	struct rsp_que *rsp;
>  
>  	struct atio_from_isp atio;
> -	/* DO NOT ADD ANYTHING ELSE HERE - atio must be last member */
> +	/*
> +	 * DO NOT ADD ANYTHING ELSE HERE.
> +	 * atio.u.isp24.fcp_cmnd.add_cdb may extend past end of atio.
> +	 */
> +	uint8_t atio_u_isp24_fcp_cmnd_add_cdb[];
>  };

[Severity: High]
Since this patch introduces the flexible array atio_u_isp24_fcp_cmnd_add_cdb[]
to legally hold the trailing data, shouldn't the memcpy() in
qlt_queue_unknown_atio() be updated to target it?

drivers/scsi/qla2xxx/qla_target.c:qlt_queue_unknown_atio() {
...
    u = kzalloc(sizeof(*u) + add_cdb_len, GFP_ATOMIC);
    ...
    memcpy(&u->atio, atio, sizeof(*atio) + add_cdb_len);
...
}

Because the destination is typed strictly as &u->atio, copying more than
64 bytes (sizeof(*atio)) when add_cdb_len > 0 causes a FORTIFY_SOURCE
intra-object write overflow panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/4808a5dc1a71528ffa3360868561009744277450.1791502886.git.fthain@linux-m68k.org?part=1

Re: [PATCH] scsi: qla2xxx: Rework BUILD_BUG_ON() assertion

[Finn Thain]

On Thu, 8 Oct 2026, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] scsi: qla2xxx: FORTIFY_SOURCE field-spanning write warning in qlt_queue_unknown_atio
> - [High] scsi: qla2xxx: UBSAN/FORTIFY overflow in add_cdb array accesses
> --
> 
> commit 4d96d5f0f41af34ce19bfbd80ce5e059ab28cd98
> Author: Finn Thain <fthain@linux-m68k.org>
> 
> scsi: qla2xxx: Rework BUILD_BUG_ON() assertion
> 
> The LKP bot reported a build failure with CONFIG_COLDFIRE=y together with
> CONFIG_SCSI_QLA_FC=y. The failure is caused by an incorrect BUILD_BUG_ON()
> assertion in qlt_queue_unknown_atio(). To resolve this, a flex array is
> placed at the end of struct qla_tgt_sess_op.
> 
> Closes: https://lore.kernel.org/oe-kbuild-all/202603030747.VX0v4otS-lkp@intel.com/
> Closes: https://lore.kernel.org/oe-kbuild-all/202609301230.CauIUXCF-lkp@intel.com/
> 
> > diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
> > index 71bc1affb1bc9..b27bd3aecbf8e 100644
> > --- a/drivers/scsi/qla2xxx/qla_target.h
> > +++ b/drivers/scsi/qla2xxx/qla_target.h
> > @@ -309,7 +309,8 @@ struct atio7_fcp_cmnd {
> >  	/*
> >  	 * add_cdb is optional and can absent from struct atio7_fcp_cmnd. Size 4
> >  	 * only to make sizeof(struct atio7_fcp_cmnd) be as expected by
> > -	 * BUILD_BUG_ON in qlt_init().
> > +	 * BUILD_BUG_ON in tcm_qla2xxx_init(). See also, BUILD_BUG_ON in
> > +	 * qlt_queue_unknown_atio().
> >  	 */
> >  	uint8_t  add_cdb[4];
> 
> [Severity: High]
> Since add_cdb is explicitly declared as a fixed-size 4-byte array here, does
> the current code cause UBSAN array bounds violations and FORTIFY_SOURCE read
> overflow panics when accessing elements beyond these bounds?
> 
> For instance, in memcpy_atio() when len > 0, it accesses
> &src->u.isp24.fcp_cmnd.add_cdb[len * 4] which is out-of-bounds, and reading
> 4 bytes triggers the overflow panic:
> 
> drivers/scsi/qla2xxx/qla_target.c:memcpy_atio() {
> ...
>     memcpy(&dst->u.isp24.fcp_cmnd.add_cdb[0],
>            &src->u.isp24.fcp_cmnd.add_cdb[len * 4],
>            4);
> ...
> }
> 
> Similarly, get_datalen_for_atio() accesses the array out-of-bounds when
> len > 0:
> 
> drivers/scsi/qla2xxx/qla_target.h:get_datalen_for_atio() {
> ...
>     return get_unaligned_be32(&atio->u.isp24.fcp_cmnd.add_cdb[len * 4]);
> ...
> }
> 
> [ ... ]
> 

Yes, add_cdb[len * 4] is OOB, but it's deliberately OOB so not a bug. This 
patch is not intended to fix either of those pre-existing theoretical 
run-time checker failures. That's out-of-scope and would seem to require 
access to the relevant QLA2xxx hardware as well as appropriate toolchain, 
.config, workload etc.

> > @@ -845,7 +846,11 @@ struct qla_tgt_sess_op {
> >  	struct rsp_que *rsp;
> >  
> >  	struct atio_from_isp atio;
> > -	/* DO NOT ADD ANYTHING ELSE HERE - atio must be last member */
> > +	/*
> > +	 * DO NOT ADD ANYTHING ELSE HERE.
> > +	 * atio.u.isp24.fcp_cmnd.add_cdb may extend past end of atio.
> > +	 */
> > +	uint8_t atio_u_isp24_fcp_cmnd_add_cdb[];
> >  };
> 
> [Severity: High]
> Since this patch introduces the flexible array atio_u_isp24_fcp_cmnd_add_cdb[]
> to legally hold the trailing data, shouldn't the memcpy() in
> qlt_queue_unknown_atio() be updated to target it?
> 

No, because atio_u_isp24_fcp_cmnd_add_cdb[] is only added as a sentinel. 
It's an attempt to coax the compiler into helping discourage the same kind 
of mistake that FORTIFY_SOURCE and UBSAN are supposed to detect.

> drivers/scsi/qla2xxx/qla_target.c:qlt_queue_unknown_atio() {
> ...
>     u = kzalloc(sizeof(*u) + add_cdb_len, GFP_ATOMIC);
>     ...
>     memcpy(&u->atio, atio, sizeof(*atio) + add_cdb_len);
> ...
> }
> 
> Because the destination is typed strictly as &u->atio, copying more than
> 64 bytes (sizeof(*atio)) when add_cdb_len > 0 causes a FORTIFY_SOURCE
> intra-object write overflow panic.
> 

That's also a pre-existing theoretical failure (and not a real bug, since 
it's deliberately OOB) and is also out-of-scope for this patch. This patch 
just addresses the build failure caused by the struct padding change.

Thanks for your review.

P.S. my preferred solution (and the first patch* I sent) to fix this build 
failure simply removed the BUILD_BUG_ON(). Sashiko-bot's criticism seems 
to support that preference, IMHO.

*
https://lore.kernel.org/linux-m68k/550e7d7bb8c2620ca4f6c9e809a4f853bdfa4c67.1772751689.git.fthain@linux-m68k.org/