On Tue, Jul 28, 2020 at 12:35:17PM +0000, David Laight wrote:
From: Peilin Ye
Sent: 28 July 2020 12:52 Currently `struct drm_buf_desc` is defined as follows:
struct drm_buf_desc { int count; int size; int low_mark; int high_mark; enum { _DRM_PAGE_ALIGN = 0x01, _DRM_AGP_BUFFER = 0x02, _DRM_SG_BUFFER = 0x04, _DRM_FB_BUFFER = 0x08, _DRM_PCI_BUFFER_RO = 0x10 } flags; unsigned long agp_start; };
copy_one_buf() is potentially copying uninitialized kernel stack memory to userspace, since the compiler may leave such "holes" (around `.flags` and `.agp_start` fields) in this statically allocated structure. Prevent it by initializing `v` with memset().
Cc: stable@vger.kernel.org Fixes: 5c7640ab6258 ("switch compat_drm_infobufs() to drm_ioctl_kernel()") Suggested-by: Dan Carpenter dan.carpenter@oracle.com Acked-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Peilin Ye yepeilin.cs@gmail.com
Change in v2: - Improve commit description. (Suggested by Arnd Bergmann arnd@arndb.de)
drivers/gpu/drm/drm_bufs.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index a0735fbc144b..f99cd4a3f951 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -1349,10 +1349,14 @@ static int copy_one_buf(void *data, int count, struct drm_buf_entry *from) { struct drm_buf_info *request = data; struct drm_buf_desc __user *to = &request->list[count];
- struct drm_buf_desc v = {.count = from->buf_count,
.size = from->buf_size,.low_mark = from->low_mark,.high_mark = from->high_mark};
struct drm_buf_desc v;
memset(&v, 0, sizeof(v));
v.count = from->buf_count;
v.size = from->buf_size;
v.low_mark = from->low_mark;
v.high_mark = from->high_mark;
if (copy_to_user(to, &v, offsetof(struct drm_buf_desc, flags))) return -EFAULT;
The memset() isn't needed. The copy_to_user() stops after the 4 'int' values so no 'random' kernel stack can get copied.
You are right, I overlooked that. Thank you for pointing this out!
Peilin Ye
Quite why it is 'right' to leave the remaining part of each userspace structure unchanged is another matter.
David.
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)