Re: [PATCH 1/7] vhost-user-gpu: fix memory disclosure in virgl_cmd_get_capset_info

[Li Qiang]

P J P <ppandit@redhat.com> 于2021年5月6日周四 下午1:53写道：
>
> +-- On Wed, 5 May 2021, Li Qiang wrote --+
> | P J P <ppandit@redhat.com> 于2021年5月5日周三 下午3:24写道：
> | > -   vg_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
> | > +   vg_ctrl_response(g, cmd, &resp.hdr, sizeof(resp.hdr));
> | >
> | > * While memset(3) is okay, should it also send header(hdr) size as 
> 'resp_len'?
> |
> | I don't think so. This function also set fields other than header such
> | as 'resp.capset_id', 'resp.capset_max_version' and so on.
>
> But it is passing 'resp.hdr' reference as parameter and size of 'resp' as
> length.
>
>   sizeof(struct virtio_gpu_ctrl_hdr): 24
>   sizeof(struct virtio_gpu_resp_capset_info): 40
>
> It may cause OOB access.

Where is the OOB access? I don't see this.
vg_ctrl_response is a general function so it accepts 'struct
virtio_gpu_ctrl_hdr' pointer and will
just set the 'hdr' field. The 'resp_len' is just used in
'iov_from_buf' to copy data to the vring.
The 'hdr' is the first field of 'virtio_gpu_resp_capset_info'.

struct virtio_gpu_resp_capset_info {
    struct virtio_gpu_ctrl_hdr hdr;
    uint32_t capset_id;
    uint32_t capset_max_version;
    uint32_t capset_max_size;
    uint32_t padding;
};

So here:
vg_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
&resp.hdr is the same as &resp.


Thanks,
Li Qiang


>
> Thank you.
> --
>  - P J P
> 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D