Re: [PATCH 2/7] sd: implement ->get_unique_id

[Keith Busch <kbusch@kernel.org>]

On Tue, Oct 12, 2021 at 02:04:40PM +0200, Christoph Hellwig wrote:
> +static int sd_get_unique_id(struct gendisk *disk, u8 id[16], u8 type)
> +{
> +	struct scsi_device *sdev = scsi_disk(disk)->device;
> +	const struct scsi_vpd *vpd;
> +	const unsigned char *d;
> +	int len = -ENXIO;
> +
> +	rcu_read_lock();
> +	vpd = rcu_dereference(sdev->vpd_pg83);
> +	if (!vpd)
> +		goto out_unlock;
> +
> +	len = -EINVAL;
> +	for (d = vpd->data + 4; d < vpd->data + vpd->len; d += d[3] + 4) {
> +		/* we only care about designators with LU association */
> +		if (((d[1] >> 4) & 0x3) != 0x00)
> +			continue;
> +		if ((d[1] & 0xf) != type)
> +			continue;
> +
> +		/*
> +		 * Only exit early if a 16-byte descriptor was found.  Otherwise
> +		 * keep looking as one with more entropy might still show up.
> +		 */
> +		len = d[3];
> +		if (len != 8 && len != 12 && len != 16)
> +			continue;

I think you need a temporary variable instead of assigning directly to
'len' here. Otherwise, the 'len' returned will be whatever the last
iteration was, which may not be then len that was copied into the 'id'.

> +		memcpy(id, d + 4, len);
> +		if (len == 16)
> +			break;
> +	}
> +out_unlock:
> +	rcu_read_unlock();
> +	return len;
> +}