qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH RFC v2 06/16] vfio-user: negotiate version with remote server


From: Stefan Hajnoczi
Subject: Re: [PATCH RFC v2 06/16] vfio-user: negotiate version with remote server
Date: Tue, 7 Sep 2021 14:52:50 +0100

On Mon, Aug 30, 2021 at 03:08:50AM +0000, John Johnson wrote:
> > On Aug 24, 2021, at 8:59 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Mon, Aug 16, 2021 at 09:42:39AM -0700, Elena Ufimtseva wrote:
> >> +int vfio_user_validate_version(VFIODevice *vbasedev, Error **errp)
> >> +{
> >> +    g_autofree VFIOUserVersion *msgp;
> >> +    GString *caps;
> >> +    int size, caplen;
> >> +
> >> +    caps = caps_json();
> >> +    caplen = caps->len + 1;
> >> +    size = sizeof(*msgp) + caplen;
> >> +    msgp = g_malloc0(size);
> >> +
> >> +    vfio_user_request_msg(&msgp->hdr, VFIO_USER_VERSION, size, 0);
> >> +    msgp->major = VFIO_USER_MAJOR_VER;
> >> +    msgp->minor = VFIO_USER_MINOR_VER;
> >> +    memcpy(&msgp->capabilities, caps->str, caplen);
> >> +    g_string_free(caps, true);
> >> +
> >> +    vfio_user_send_recv(vbasedev->proxy, &msgp->hdr, NULL, 0, 0);
> >> +    if (msgp->hdr.flags & VFIO_USER_ERROR) {
> >> +        error_setg_errno(errp, msgp->hdr.error_reply, "version reply");
> >> +        return -1;
> >> +    }
> >> +
> >> +    if (msgp->major != VFIO_USER_MAJOR_VER ||
> >> +        msgp->minor > VFIO_USER_MINOR_VER) {
> >> +        error_setg(errp, "incompatible server version");
> >> +        return -1;
> >> +    }
> >> +    if (caps_check(msgp->minor, (char *)msgp + sizeof(*msgp), errp) != 0) 
> >> {
> > 
> > The reply is untrusted so we cannot treat it as a NUL-terminated string
> > yet. The final byte msgp->capabilities[] needs to be checked first.
> > 
> > Please be careful about input validation, I might miss something so it's
> > best if you audit the patches too. QEMU must not trust the device
> > emulation process and vice versa.
> > 
> 
>       This message is consumed by vfio-user, so I can check for valid
> replies, but for most messages this checking will have to be done at in
> the VFIO common or bus-specific code, as vfio-user doens’t know valid
> data from invalid.

Good point. Today's VFIO code trusts the replies because they come from
the kernel. Now that they can come from an untrusted source they must be
validated and common VFIO code may need to change its trust model.

Stefan

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]