[2/2] block/file-posix: fix wps checking in raw_co_prw

Commit Message

Sam Li June 4, 2023, 6:16 a.m. UTC

If the write operation fails and the wps is NULL, then accessing it will
lead to data corruption.

Solving the issue by adding a nullptr checking in get_zones_wp() where
the wps is used.

This issue is found by Peter Maydell using the Coverity Tool (CID
1512459).

Signed-off-by: Sam Li <[e-mail]>
---
 block/file-posix.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Stefan Hajnoczi June 7, 2023, 4:08 p.m. UTC | #1

On Sun, Jun 04, 2023 at 02:16:58PM +0800, Sam Li wrote:
> If the write operation fails and the wps is NULL, then accessing it will
> lead to data corruption.
> 
> Solving the issue by adding a nullptr checking in get_zones_wp() where
> the wps is used.
> 
> This issue is found by Peter Maydell using the Coverity Tool (CID
> 1512459).
> 
> Signed-off-by: Sam Li <[e-mail]>
> ---
>  block/file-posix.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 0d9d179a35..620942bf40 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1340,6 +1340,10 @@ static int get_zones_wp(BlockDriverState *bs, int fd, int64_t offset,
>      rep_size = sizeof(struct blk_zone_report) + nrz * sizeof(struct blk_zone);
>      g_autofree struct blk_zone_report *rep = NULL;
>  
> +    if (!wps) {
> +        return -1;
> +    }

An error will be printed every time this happens on a non-zoned device:

  static void update_zones_wp(BlockDriverState *bs, int fd, int64_t offset,
                              unsigned int nrz)
  {
      if (get_zones_wp(bs, fd, offset, nrz, 0) < 0) {
          error_report("update zone wp failed");

Please change the following code to avoid the call to update_zones_wp():

  #if defined(CONFIG_BLKZONED)
  {
      BlockZoneWps *wps = bs->wps;
      if (ret == 0) {
          if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND))
              && wps && bs->bl.zone_size) {
              uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
              if (!BDRV_ZT_IS_CONV(*wp)) {
                  if (type & QEMU_AIO_ZONE_APPEND) {
                      *s->offset = *wp;
                      trace_zbd_zone_append_complete(bs, *s->offset
                          >> BDRV_SECTOR_BITS);
                  }
                  /* Advance the wp if needed */
                  if (offset + bytes > *wp) {
                      *wp = offset + bytes;
                  }
              }
          }
      } else {
-         if (type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) {
+         if (wps && (type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND))) {
              update_zones_wp(bs, s->fd, 0, 1);
          }
      }

Stefan

Patch

diff --git a/block/file-posix.c b/block/file-posix.c
index 0d9d179a35..620942bf40 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1340,6 +1340,10 @@  static int get_zones_wp(BlockDriverState *bs, int fd, int64_t offset,
     rep_size = sizeof(struct blk_zone_report) + nrz * sizeof(struct blk_zone);
     g_autofree struct blk_zone_report *rep = NULL;
 
+    if (!wps) {
+        return -1;
+    }
+
     rep = g_malloc(rep_size);
     blkz = (struct blk_zone *)(rep + 1);
     while (n < nrz) {