linux-kernel - Re: [PATCH] drm/i915/gvt: fix memory leak in intel_vgpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 8 Aug 2018 15:23:20 +0800
From:   Zhenyu Wang <zhenyuw@...ux.intel.com>
To:     Yi Wang <wang.yi59@....com.cn>
Cc:     zhi.a.wang@...el.com, jani.nikula@...ux.intel.com,
        joonas.lahtinen@...ux.intel.com, rodrigo.vivi@...el.com,
        airlied@...ux.ie, intel-gvt-dev@...ts.freedesktop.org,
        intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, jiang.biao2@....com.cn,
        zhong.weidong@....com.cn
Subject: Re: [PATCH] drm/i915/gvt: fix memory leak in intel_vgpu_ioctl()

On 2018.08.03 08:41:19 +0800, Yi Wang wrote:
> The 'sparse' variable may leak when return in function
> intel_vgpu_ioctl(), and this patch fixes this.
> 
> Signed-off-by: Yi Wang <wang.yi59@....com.cn>
> Reviewed-by: Jiang Biao <jiang.biao2@....com.cn>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index df4e4a0..6a6f199 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -1200,6 +1200,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
>  					return ret;
>  				break;
>  			default:
> +				kfree(sparse);
>  				return -EINVAL;
>  			}
>  		}
> @@ -1215,6 +1216,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
>  						  sizeof(info), caps.buf,
>  						  caps.size)) {
>  					kfree(caps.buf);
> +					kfree(sparse);
>  					return -EFAULT;
>  				}
>  				info.cap_offset = sizeof(info);
> @@ -1223,6 +1225,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
>  			kfree(caps.buf);
>  		}
>  
> +		kfree(sparse);

Unfortunately this would cause a double-free error in normal path, as we
tried to free sparse after use to add caps. So may be better to fix free
in error path and move normal free of sparse in final point, e.g

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 68df9aa88890..47b897b6ea93 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1257,11 +1257,13 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 					&sparse->header, sizeof(*sparse) +
 					(sparse->nr_areas *
 						sizeof(*sparse->areas)));
-				kfree(sparse);
-				if (ret)
+				if (ret) {
+					kfree(sparse);
 					return ret;
+				}
 				break;
 			default:
+				kfree(sparse);
 				return -EINVAL;
 			}
 		}
@@ -1277,6 +1279,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 						  sizeof(info), caps.buf,
 						  caps.size)) {
 					kfree(caps.buf);
+					kfree(sparse);
 					return -EFAULT;
 				}
 				info.cap_offset = sizeof(info);
@@ -1285,6 +1288,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 			kfree(caps.buf);
 		}
 
+		kfree(sparse);
 		return copy_to_user((void __user *)arg, &info, minsz) ?
 			-EFAULT : 0;
 	} else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {



-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)