On 9/5/26 01:18, Niklas Cassel wrote:
> raw_co_prw() replaces the offset of a zone append with the write pointer
> of the addressed zone, which assumes that the stored value names a
> position inside that zone. It does not in two cases.
>
> A full zone has its write pointer recorded at the end of the zone, since
> get_zones_wp() stores start + len for BLK_ZONE_COND_FULL. That is the
> first sector of the following zone, so the append is submitted there. The
> kernel accepts it whenever that zone is empty, because it is a legal
> write at its write pointer, and the completion path advances the wrong
> zone because it recomputes the zone index from the replaced offset. The
> data is written to a zone that was never addressed and success is
> returned:
>
> zone 2 finished, then a 4 KiB append to zone 2:
> After zap done, the append sector is 0x180000 <- zone 3
> zone 2: wptr 0x180000, zcond:14 (full)
> zone 3: wptr 0x180008 <- advanced
>
> A conventional zone has no write pointer at all, and its array entry
> carries only the type marker in the top bit, so the offset becomes
> negative and the write fails with EINVAL. That is harmless but it reports
> nothing about the actual mistake.
>
> Reject both while the write pointer lock is held, since the state has to
> be read and acted on atomically. check_zoned_request() in virtio-blk
> refuses an append to a conventional zone, so that case needs a caller
> that goes to the driver directly, but nothing there examines whether a
> zone is full, so a guest can reach the misdirected write.
>
> Fixes: 4751d09adcc3 ("block: introduce zone append write for zoned devices")
> Signed-off-by: Niklas Cassel <[email protected]>
Looks good, modulo the comment below that needs to be addressed.
With that fixed, feel free to add:
Reviewed-by: Damien Le Moal <[email protected]>
> diff --git a/block/io.c b/block/io.c
> index b59c7fdf3e..452254bc63 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -3378,6 +3378,15 @@ uint32_t bdrv_zone_index(BlockDriverState *bs,
> uint64_t offset)
> return offset >> ctz64(bs->bl.zone_size);
> }
>
> +bool bdrv_zone_is_full(BlockDriverState *bs, uint32_t index)
> +{
> + uint64_t zone_end = MIN((uint64_t)(index + 1) * bs->bl.zone_size,
I think this should look at zone capacity, not size. For cases where we have
zone cap < zone size, zone full condition is reached when the write pointer is
at the zone capacity.
> + (uint64_t)bs->total_sectors << BDRV_SECTOR_BITS);
> + IO_CODE();
> +
> + return bs->wps->wp[index] >= zone_end;
> +}
--
Damien Le Moal
Western Digital Research
