On Tue, Mar 03, 2026 at 06:17:23AM +0200, Mohamed Ayman wrote:
> Add a detailed documentation comment for vhost_user_read() describing
> its message header validation, payload size checks, and error handling.
>
> This clarifies how complete vhost-user messages (header + payload) are
> read from the character device and how protocol violations are handled.
>
> No functional changes.
>
> Signed-off-by: Mohamed Ayman <[email protected]>
This all seems unnecessarily verbose.
> ---
> hw/virtio/vhost-user.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index bb8f8eab77..157a869b35 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -300,6 +300,20 @@ static int vhost_user_read_header(struct vhost_dev *dev,
> VhostUserMsg *msg)
> return 0;
> }
>
> +/**
> + * vhost_user_read:
> + * @dev: The vhost device state
> + * @msg: Pointer to the vhost-user message structure to populate
> + *
> + * Reads a complete vhost-user message (header and optional payload) from
> + * the character device.
use imperative voice: Read ...
what "the character device"?
> It first reads and validates the header, checks
> + * the advertised payload size against maximum bounds, and then reads any
> + * attached payload directly into the message structure.
repeating implementation here does not make sense.
> + *
> + * Returns: 0 on success,
> + * -EIO or -errno on read failure or short read,
> + * -EPROTO if the protocol flags or payload size are invalid.
we usually do not document specific errors.
> + */
> static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg)
> {
> struct vhost_user *u = dev->opaque;
> @@ -307,12 +321,13 @@ static int vhost_user_read(struct vhost_dev *dev,
> VhostUserMsg *msg)
> uint8_t *p = (uint8_t *) msg;
> int r, size;
>
> + /* 1. Read and validate the message header */
obvious from function name
`
> r = vhost_user_read_header(dev, msg);
> if (r < 0) {
> return r;
> }
>
> - /* validate message size is sane */
> + /* 2. Validate that the payload size is sane to prevent buffer overflows
> */
how is this better?
> if (msg->hdr.size > VHOST_USER_PAYLOAD_SIZE) {
> error_report("Failed to read msg header."
> " Size %d exceeds the maximum %zu.", msg->hdr.size,
> @@ -320,9 +335,13 @@ static int vhost_user_read(struct vhost_dev *dev,
> VhostUserMsg *msg)
> return -EPROTO;
> }
>
> + /* 3. If the header indicates a payload exists, read it */
this one is ok.
> if (msg->hdr.size) {
> + /* Advance the pointer to the start of the payload section */
too detailed, it's clear as is.
> p += VHOST_USER_HDR_SIZE;
> size = msg->hdr.size;
> +
> + /* Block and read the exact payload size */
exact?
> r = qemu_chr_fe_read_all(chr, p, size);
> if (r != size) {
> int saved_errno = errno;
> --
> 2.34.1
