Re: [PATCH] vfio-user: validate VERSION replies

["Cédric Le Goater" <clg@redhat.com>]

On 6/3/26 08:21, zhaoguohan@kylinos.cn wrote:
> From: GuoHan Zhao <zhaoguohan@kylinos.cn>
> 
> The vfio-user protocol makes the VERSION payload optional, so a
> reply may legally stop after the major and minor fields.
> 
> vfio_user_validate_version() currently assumes a capabilities string is
> always present and NUL-terminated. When the server replies without
> version data, QEMU ends up reusing the request-side capabilities buffer
> and the terminating-NUL check underflows. Replies shorter than the fixed
> VERSION header are also accessed before they are validated.
> 
> Reject replies shorter than the fixed VERSION header and only parse
> capabilities when the reply actually carries version data.
> 
> Fixes: 36227628d824 (vfio-user: implement message send infrastructure)
> Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
> ---
>   hw/vfio-user/proxy.c | 21 ++++++++++++++-------
>   1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c
> index 0f7d8425d614..197aee07bf7a 100644
> --- a/hw/vfio-user/proxy.c
> +++ b/hw/vfio-user/proxy.c
> @@ -1292,7 +1292,7 @@ bool vfio_user_validate_version(VFIOUserProxy *proxy, Error **errp)
>   {
>       g_autofree VFIOUserVersion *msgp = NULL;
>       GString *caps;
> -    char *reply;
> +    const char *reply = "";
>       int size, caplen;
>   
>       caps = caps_json();
> @@ -1322,17 +1322,24 @@ bool vfio_user_validate_version(VFIOUserProxy *proxy, Error **errp)
>           return false;
>       }
>   
> -    reply = msgp->capabilities;
> -    if (reply[msgp->hdr.size - sizeof(*msgp) - 1] != '\0') {
> -        error_setg(errp, "corrupt version reply");
> +    if (msgp->hdr.size < sizeof(*msgp)) {
> +        error_setg(errp, "short version reply");
>           return false;
>       }
>   
> -    if (!caps_check(proxy, msgp->minor, reply, errp)) {
> -        return false;
> +    if (msgp->hdr.size > sizeof(*msgp)) {
> +        reply = msgp->capabilities;
> +        if (reply[msgp->hdr.size - sizeof(*msgp) - 1] != '\0') {
> +            error_setg(errp, "corrupt version reply");
> +            return false;
> +        }
> +
> +        if (!caps_check(proxy, msgp->minor, reply, errp)) {
> +            return false;
> +        }
>       }
>   
> -    trace_vfio_user_version(msgp->major, msgp->minor, msgp->capabilities);
> +    trace_vfio_user_version(msgp->major, msgp->minor, reply);
>       return true;
>   }
>   


Applied to

     https://github.com/legoater/qemu vfio-next

Thanks,

C.