Skip to content

Commit

Permalink
Virtio-9P device attach: fix initialization of virtio_9p struct
Browse files Browse the repository at this point in the history
Since commit d037970, in multi-vCPU instances root filesystem
initialization can complete before PCI bus discovery; this means
that any filesystem mount points specified in the manifest options
can be already processed by the time a virtio-9p device is probed
and a corresponding volume is added; this in turn means that the
`volume_add()` function called by `v9p_dev_attach()` can trigger a
direct call to `v9p_fs_init()`.
The `v9p_fs_init()` function uses the `general` field of the
`virtio_9p` struct, which in the current code is initialized after
calling the `volume_add()` function; this causes `v9p_fs_init()`
to access an uninitialized pointer, leading to an unhandled page
fault. The same issue applies to the `backed` field of the struct.
This change fixes the above issue by moving the initialization of
the struct fields before the call to `volume_add()`. In addition,
the unused `dev` field is being removed, and setting the DRIVER_OK
flag in the device status field is being moved before the call to
`volume_add()` (according to the virtio specs, a driver must not
send any buffer available notifications to the device before
setting DRIVER_OK).
  • Loading branch information
francescolavra committed Jun 20, 2024
1 parent 0c267e7 commit 5649086
Showing 1 changed file with 3 additions and 5 deletions.
8 changes: 3 additions & 5 deletions src/virtio/virtio_9p.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
typedef struct virtio_9p {
heap general;
backed_heap backed;
vtdev dev;
closure_struct(fs_init_handler, fs_init);
virtqueue vq;
u16 next_tag;
Expand Down Expand Up @@ -103,6 +102,9 @@ static boolean v9p_dev_attach(heap general, backed_heap backed, vtdev dev)
goto err;
}
spin_lock_init(&v9p->lock);
v9p->general = general;
v9p->backed = backed;
vtdev_set_status(dev, VIRTIO_CONFIG_STATUS_DRIVER_OK);
u8 uuid[UUID_LEN];
char label[VOLUME_LABEL_MAX_LEN];
rsnprintf(label, sizeof(label), "virtfs%ld", attach_id);
Expand All @@ -112,10 +114,6 @@ static boolean v9p_dev_attach(heap general, backed_heap backed, vtdev dev)
msg_err("failed to add volume\n");
goto err;
}
v9p->general = general;
v9p->backed = backed;
v9p->dev = dev;
vtdev_set_status(dev, VIRTIO_CONFIG_STATUS_DRIVER_OK);
return true;
err:
deallocate(general, v9p, sizeof(*v9p));
Expand Down

0 comments on commit 5649086

Please sign in to comment.