Re: [PATCH v2 2/2] scsi: ufs: Serve the device init reads from one aggregated read

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ufs: unaligned memory access in ufshcd_agg_group() crashes on bad device input
--

commit 8bc3a3d567bfb8009dd9c20e3fdb96ed162e1ae5
Author: Hyeoncheol Jeong <hyenc.jeong@samsung.com>

scsi: ufs: Serve the device init reads from one aggregated read

This commit introduces an aggregated read mechanism for UFS 5.0 devices to
fetch attributes, flags, and ID-strings in a single query during device
initialization.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index c18b4784a3595..028f0179ee8be 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c

[ ... ]

> +static const u8 *ufshcd_agg_group(const struct ufs_hba *hba, u8 type,
> +				  u16 *group_len)
> +{
> +	const u8 *packet = hba->agg_packet;
> +	u16 packet_len = hba->agg_packet_len;
> +	u16 group_off = 0;
> +
> +	while (packet && group_off + QUERY_AGG_GROUP_HDR_SIZE <= packet_len) {
> +		const struct utp_agg_group_header *hdr =
> +			(const void *)(packet + group_off);
> +		u16 next_group_off = be16_to_cpu(hdr->next_group_offset);

[Severity: High]
Does this code safely handle unaligned memory accesses?

If a malformed or untrusted UFS device returns an odd next_group_offset, the
loop adds this odd offset to the base pointer, and the next iteration will
attempt to read the 16-bit hdr->next_group_offset from an unaligned address.

This will cause a kernel panic on architectures without unaligned access
support. Should get_unaligned_be16() be used here, or should the
utp_agg_group_header struct be marked as __packed?

> +		u16 payload_off = group_off + QUERY_AGG_GROUP_HDR_SIZE;
> +
> +		if (next_group_off && (next_group_off < payload_off ||
> +				       next_group_off + QUERY_AGG_GROUP_HDR_SIZE > packet_len))
> +			return NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/1860250540.01790752381858.JavaMail.epsvc@epcpadp2new?part=2