2023-09-20 20:17:27

by Philipp Stanner

[permalink] [raw]
Subject: [PATCH v3 0/5] Introduce new wrappers to copy user-arrays

Hi!

David Airlie suggested that we could implement new wrappers around
(v)memdup_user() for duplicating user arrays.

This small patch series first implements the two new wrapper functions
memdup_array_user() and vmemdup_array_user(). They calculate the
array-sizes safely, i.e., they return an error in case of an overflow.

It then implements the new wrappers in two components in kernel/ and two
in the drm-subsystem.

In total, there are 18 files in the kernel that use (v)memdup_user() to
duplicate arrays. My plan is to provide patches for the other 14
successively once this series has been merged.


Changes since v2:
- Remove the unlikely() from the overflow-check, since the latter
already implements one (Dan Carpenter)
- Add the Reviewd-bys for the changes already reviewed in v2

Changes since v1:
- Insert new headers alphabetically ordered
- Remove empty lines in functions' docstrings
- Return -EOVERFLOW instead of -EINVAL from wrapper functions

P.

Philipp Stanner (5):
string.h: add array-wrappers for (v)memdup_user()
kernel: kexec: copy user-array safely
kernel: watch_queue: copy user-array safely
drm_lease.c: copy user-array safely
drm: vmgfx_surface.c: copy user-array safely

drivers/gpu/drm/drm_lease.c | 4 +--
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 4 +--
include/linux/string.h | 40 +++++++++++++++++++++++++
kernel/kexec.c | 2 +-
kernel/watch_queue.c | 2 +-
5 files changed, 46 insertions(+), 6 deletions(-)

--
2.41.0


2023-09-20 20:24:22

by Philipp Stanner

[permalink] [raw]
Subject: [PATCH v3 4/5] drm_lease.c: copy user-array safely

Currently, there is no overflow-check with memdup_user().

Use the new function memdup_array_user() instead of memdup_user() for
duplicating the user-space array safely.

Suggested-by: David Airlie <[email protected]>
Signed-off-by: Philipp Stanner <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Reviewed-by: Zack Rusin <[email protected]>
---
drivers/gpu/drm/drm_lease.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 150fe1555068..94375c6a5425 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -510,8 +510,8 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
/* Handle leased objects, if any */
idr_init(&leases);
if (object_count != 0) {
- object_ids = memdup_user(u64_to_user_ptr(cl->object_ids),
- array_size(object_count, sizeof(__u32)));
+ object_ids = memdup_array_user(u64_to_user_ptr(cl->object_ids),
+ object_count, sizeof(__u32));
if (IS_ERR(object_ids)) {
ret = PTR_ERR(object_ids);
idr_destroy(&leases);
--
2.41.0

2023-09-22 04:17:27

by Philipp Stanner

[permalink] [raw]
Subject: [PATCH v3 5/5] drm: vmgfx_surface.c: copy user-array safely

Currently, there is no overflow-check with memdup_user().

Use the new function memdup_array_user() instead of memdup_user() for
duplicating the user-space array safely.

Suggested-by: David Airlie <[email protected]>
Signed-off-by: Philipp Stanner <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Reviewed-by: Zack Rusin <[email protected]>
---
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 5db403ee8261..9be185b094cb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -777,9 +777,9 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void *data,
sizeof(metadata->mip_levels));
metadata->num_sizes = num_sizes;
metadata->sizes =
- memdup_user((struct drm_vmw_size __user *)(unsigned long)
+ memdup_array_user((struct drm_vmw_size __user *)(unsigned long)
req->size_addr,
- sizeof(*metadata->sizes) * metadata->num_sizes);
+ metadata->num_sizes, sizeof(*metadata->sizes));
if (IS_ERR(metadata->sizes)) {
ret = PTR_ERR(metadata->sizes);
goto out_no_sizes;
--
2.41.0