On 4/15/24 4:13 PM, Kevin Brodsky wrote:
On 15/04/2024 16:48, carsten.haitzler@foss.arm.com wrote:
[...]
int drm_mode_atomic_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) {
- struct drm_mode_atomic32 *arg32 = data; struct drm_mode_atomic *arg = data;
- uint32_t __user *objs_ptr = uaddr_to_user_ptr(arg->objs_ptr);
- uint32_t __user *count_props_ptr = uaddr_to_user_ptr(arg->count_props_ptr);
- uint32_t __user *props_ptr = uaddr_to_user_ptr(arg->props_ptr);
- uint64_t __user *prop_values_ptr = uaddr_to_user_ptr(arg->prop_values_ptr); unsigned int copied_objs, copied_props; struct drm_atomic_state *state; struct drm_modeset_acquire_ctx ctx; struct drm_out_fence_state *fence_state; int ret = 0; unsigned int i, j, num_fences;
- __u32 flags;
- __u32 count_objs;
- uint32_t __user *objs_ptr;
- uint32_t __user *count_props_ptr;
- uint32_t __user *props_ptr;
- uint64_t __user *prop_values_ptr;
- void __user *reserved;
/* disallow for drivers not supporting atomic: */ if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) @@ -1352,25 +1388,43 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, return -EINVAL; }
- if (arg->flags & ~DRM_MODE_ATOMIC_FLAGS) {
- if (in_compat64_syscall()) {
flags = arg32->flags;count_objs = arg32->count_objs;objs_ptr = compat_ptr(arg32->objs_ptr);count_props_ptr = compat_ptr(arg32->count_props_ptr);props_ptr = compat_ptr(arg32->props_ptr);prop_values_ptr = compat_ptr(arg32->prop_values_ptr);reserved = compat_ptr(arg32->reserved);- } else {
flags = arg->flags;count_objs = arg->count_objs;objs_ptr = (uint32_t __user *)arg->objs_ptr;count_props_ptr = (uint32_t __user *)arg->count_props_ptr;props_ptr = (uint32_t __user *)arg->props_ptr;prop_values_ptr = (uint64_t __user *)arg->prop_values_ptr;reserved = (void __user *)arg->reserved;Not sure it makes sense to treat reserved as a pointer. Can't we keep it as a __u64?
Happy with how things are looking otherwise.
alignment will force it to have to use up a pointer/cap of space anyway... it is as if this was considered for future use as a pointer so making it a __u64 won't change memory layout or struct size, so may as well go with the worst case - a ptr. :) the code very much wants this to NOT be set to anything. if we make it a __u64 we won't be testing the content of the padding space being zero... :)