[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@xxxxxxxxxx>
Cc: Tony Battersby <tonyb@xxxxxxxxxxxxxxx>
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Arnd Bergmann <arnd@xxxxxxxx>
Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Cc: linux-m68k@xxxxxxxxxxxxxxxxxxxx
Reported-by: kernel test robot <lkp@xxxxxxxxx>
Closes: https://lore.kernel.org/oe-kbuild-all/202603030747.VX0v4otS-lkp@xxxxxxxxx/
Closes: https://lore.kernel.org/oe-kbuild-all/202609301230.CauIUXCF-lkp@xxxxxxxxx/
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@xxxxxxxxxxxxxxx>
Signed-off-by: Finn Thain <fthain@xxxxxxxxxxxxxx>
---
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 {