On Sat, Sep 05, 2026 at 09:38:08AM +0900, Damien Le Moal wrote:
> 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.
There is no concept of zone capacity in QEMU upstream yet.
It is added in Sam Li's QCOW2 zoned patch series.
But yes, I already have a patch that modifies bdrv_zone_is_full() to use
zone capacity rather than zone size, once it is introduced.
Kind regards,
Niklas
