| Message ID | 20210505045824.33880-4-liq3ea@163.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vhost-user-gpu: fix several security issues | expand |
+-- On Tue, 4 May 2021, Li Qiang wrote --+
| Check whether the 'res' has already been attach_backing to avoid
| memory leak.
|
| Signed-off-by: Li Qiang <liq3ea@163.com>
| ---
| contrib/vhost-user-gpu/vhost-user-gpu.c | 5 +++++
| 1 file changed, 5 insertions(+)
|
| diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c
| index b5e153d0d6..0437e52b64 100644
| --- a/contrib/vhost-user-gpu/vhost-user-gpu.c
| +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c
| @@ -489,6 +489,11 @@ vg_resource_attach_backing(VuGpu *g,
| return;
| }
|
| + if (res->iov) {
| + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
| + return;
| + }
| +
| ret = vg_create_mapping_iov(g, &ab, cmd, &res->iov);
| if (ret != 0) {
| cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
* While it'll work, should the check be done by 'virtio_gpu_find_resource()'
before returning 'res' ? ie. if 'res->iov' is being used, then it's similar
case as 'illegal resource specified %d'.
Thank you.
--
- P J P
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
P J P <ppandit@redhat.com> 于2021年5月5日周三 下午3:39写道： > > +-- On Tue, 4 May 2021, Li Qiang wrote --+ > | Check whether the 'res' has already been attach_backing to avoid > | memory leak. > | > | Signed-off-by: Li Qiang <liq3ea@163.com> > | --- > | contrib/vhost-user-gpu/vhost-user-gpu.c | 5 +++++ > | 1 file changed, 5 insertions(+) > | > | diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c > | index b5e153d0d6..0437e52b64 100644 > | --- a/contrib/vhost-user-gpu/vhost-user-gpu.c > | +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c > | @@ -489,6 +489,11 @@ vg_resource_attach_backing(VuGpu *g, > | return; > | } > | > | + if (res->iov) { > | + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; > | + return; > | + } > | + > | ret = vg_create_mapping_iov(g, &ab, cmd, &res->iov); > | if (ret != 0) { > | cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; > > > * While it'll work, should the check be done by 'virtio_gpu_find_resource()' > before returning 'res' ? ie. if 'res->iov' is being used, then it's similar > case as 'illegal resource specified %d'. No, 'virtio_gpu_find_resource' is a general function and is used in a lot place and 'res->iov' is specified to 'vg_resource_[de]attach_backing' so we should do this check in 'vg_resource_attach_backing'. Thanks, Li Qiang > > > Thank you. > -- > - P J P > 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D >
diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c index b5e153d0d6..0437e52b64 100644 --- a/contrib/vhost-user-gpu/vhost-user-gpu.c +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c @@ -489,6 +489,11 @@ vg_resource_attach_backing(VuGpu *g, return; } + if (res->iov) { + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; + return; + } + ret = vg_create_mapping_iov(g, &ab, cmd, &res->iov); if (ret != 0) { cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
Check whether the 'res' has already been attach_backing to avoid memory leak. Signed-off-by: Li Qiang <liq3ea@163.com> --- contrib/vhost-user-gpu/vhost-user-gpu.c | 5 +++++ 1 file changed, 5 insertions(+)