Re: [PATCH] scsi: qla2xxx: Remove problematic BUILD_BUG_ON() assertion

[Tony Battersby]

On 3/5/26 18:01, Finn Thain wrote:
> The LKP bot reported a build failure with CONFIG_COLDFIRE=y together with
> CONFIG_SCSI_QLA_FC=y, that's 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, this 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.
...
> I don't know of a good way to encode an invariant like "the last member of
> struct qla_tgt_sess_op is named atio" such that it might be statically
> checked. But perhaps there is a good way to do that (?)

It might work better to add a flex array:

struct qla_tgt_sess_op {
	...

	struct atio_from_isp atio;
	/*
	atio.u.isp24.fcp_cmnd.add_cdb may extend past end of atio;
	DO NOT DELETE; DO NOT ADD ANYTHING ELSE HERE.
	*/
	uint8_t atio_isp24_fcp_cmnd_add_cdb[];
};

/* atio_isp24_fcp_cmnd_add_cdb must come 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_isp24_fcp_cmnd_add_cdb));

Tony Battersby