[PATCH 0/5] staging: vchiq: stop using compat_alloc_user_space

[PATCH 0/5] staging: vchiq: stop using compat_alloc_user_space

[Arnd Bergmann]

This driver is one of only a few remaining files using
compat_alloc_user_space() and copy_in_user() to implement the compat_ioctl
handlers.

Change it to be more like the other drivers, calling the underlying
implementation directly, which is generally simpler and less
error-prone.

This is only build tested so far.

    Arnd

Arnd Bergmann (5):
  staging: vchiq: rework compat handling
  staging: vchiq: convert compat create_service
  staging: vchiq: convert compat dequeue_message
  staging: vchiq: convert compat bulk transfer
  staging: vchiq: convert compat await_completion

 .../interface/vchiq_arm/vchiq_arm.c           | 1194 ++++++++---------
 1 file changed, 551 insertions(+), 643 deletions(-)

-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/5] staging: vchiq: stop using compat_alloc_user_space

[Arnd Bergmann]

This driver is one of only a few remaining files using
compat_alloc_user_space() and copy_in_user() to implement the compat_ioctl
handlers.

Change it to be more like the other drivers, calling the underlying
implementation directly, which is generally simpler and less
error-prone.

This is only build tested so far.

    Arnd

Arnd Bergmann (5):
  staging: vchiq: rework compat handling
  staging: vchiq: convert compat create_service
  staging: vchiq: convert compat dequeue_message
  staging: vchiq: convert compat bulk transfer
  staging: vchiq: convert compat await_completion

 .../interface/vchiq_arm/vchiq_arm.c           | 1194 ++++++++---------
 1 file changed, 551 insertions(+), 643 deletions(-)

-- 
2.27.0

[PATCH 1/5] staging: vchiq: rework compat handling

[Arnd Bergmann]

The compat handlers for VCHIQ_IOC_QUEUE_MESSAGE32 and
VCHIQ_IOC_GET_CONFIG32 can simply call the underlying implementations
that are already separate functions rather than using copy_in_user to
simulate the native 64-bit interface for the full ioctl handler.

vchiq_ioc_queue_message gets a small update to the calling
conventions to simplify the compat version by directly
returning a normal errno value.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 109 +++++++++---------
 1 file changed, 56 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index d4d811884861..56a38bec848a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -765,12 +765,13 @@ static ssize_t vchiq_ioc_copy_element_data(void *context, void *dest,
  *   vchiq_ioc_queue_message
  *
  **************************************************************************/
-static enum vchiq_status
+static int
 vchiq_ioc_queue_message(unsigned int handle,
 			struct vchiq_element *elements,
 			unsigned long count)
 {
 	struct vchiq_io_copy_callback_context context;
+	enum vchiq_status status = VCHIQ_SUCCESS;
 	unsigned long i;
 	size_t total_size = 0;
 
@@ -785,8 +786,14 @@ vchiq_ioc_queue_message(unsigned int handle,
 		total_size += elements[i].size;
 	}
 
-	return vchiq_queue_message(handle, vchiq_ioc_copy_element_data,
-				   &context, total_size);
+	status = vchiq_queue_message(handle, vchiq_ioc_copy_element_data,
+				     &context, total_size);
+
+	if (status == VCHIQ_ERROR)
+		return -EIO;
+	else if (status == VCHIQ_RETRY)
+		return -EINTR;
+	return 0;
 }
 
 /****************************************************************************
@@ -1020,9 +1027,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 			if (copy_from_user(elements, args.elements,
 				args.count * sizeof(struct vchiq_element)) == 0)
-				status = vchiq_ioc_queue_message
-					(args.handle,
-					elements, args.count);
+				ret = vchiq_ioc_queue_message(args.handle, elements,
+							      args.count);
 			else
 				ret = -EFAULT;
 		} else {
@@ -1550,55 +1556,53 @@ struct vchiq_queue_message32 {
 static long
 vchiq_compat_ioctl_queue_message(struct file *file,
 				 unsigned int cmd,
-				 unsigned long arg)
+				 struct vchiq_queue_message32 __user *arg)
 {
-	struct vchiq_queue_message __user *args;
-	struct vchiq_element __user *elements;
+	struct vchiq_queue_message args;
 	struct vchiq_queue_message32 args32;
-	unsigned int count;
-
-	if (copy_from_user(&args32,
-			   (struct vchiq_queue_message32 __user *)arg,
-			   sizeof(args32)))
-		return -EFAULT;
-
-	args = compat_alloc_user_space(sizeof(*args) +
-				       (sizeof(*elements) * MAX_ELEMENTS));
+	struct vchiq_service *service;
+	int ret;
 
-	if (!args)
+	if (copy_from_user(&args32, arg, sizeof(args32)))
 		return -EFAULT;
 
-	if (put_user(args32.handle, &args->handle) ||
-	    put_user(args32.count, &args->count) ||
-	    put_user(compat_ptr(args32.elements), &args->elements))
-		return -EFAULT;
+	args = (struct vchiq_queue_message) {
+		.handle   = args32.handle,
+		.count    = args32.count,
+		.elements = compat_ptr(args32.elements),
+	};
 
 	if (args32.count > MAX_ELEMENTS)
 		return -EINVAL;
 
-	if (args32.elements && args32.count) {
-		struct vchiq_element32 tempelement32[MAX_ELEMENTS];
+	service = find_service_for_instance(file->private_data, args.handle);
+	if (!service)
+		return -EINVAL;
 
-		elements = (struct vchiq_element __user *)(args + 1);
+	if (args32.elements && args32.count) {
+		struct vchiq_element32 element32[MAX_ELEMENTS];
+		struct vchiq_element elements[MAX_ELEMENTS];
+		unsigned int count;
 
-		if (copy_from_user(&tempelement32,
-				   compat_ptr(args32.elements),
-				   sizeof(tempelement32)))
+		if (copy_from_user(&element32, args.elements,
+				   sizeof(element32))) {
+			unlock_service(service);
 			return -EFAULT;
+		}
 
 		for (count = 0; count < args32.count; count++) {
-			if (put_user(compat_ptr(tempelement32[count].data),
-				     &elements[count].data) ||
-			    put_user(tempelement32[count].size,
-				     &elements[count].size))
-				return -EFAULT;
+			elements[count].data =
+				compat_ptr(element32[count].data);
+			elements[count].size = element32[count].size;
 		}
-
-		if (put_user(elements, &args->elements))
-			return -EFAULT;
+		ret = vchiq_ioc_queue_message(args.handle, elements,
+					      args.count);
+	} else {
+		ret = -EINVAL;
 	}
+	unlock_service(service);
 
-	return vchiq_ioctl(file, VCHIQ_IOC_QUEUE_MESSAGE, (unsigned long)args);
+	return ret;
 }
 
 struct vchiq_queue_bulk_transfer32 {
@@ -1866,35 +1870,34 @@ struct vchiq_get_config32 {
 static long
 vchiq_compat_ioctl_get_config(struct file *file,
 			      unsigned int cmd,
-			      unsigned long arg)
+			      struct vchiq_get_config32 __user *arg)
 {
-	struct vchiq_get_config __user *args;
 	struct vchiq_get_config32 args32;
+	struct vchiq_config config;
+	void __user *ptr;
 
-	args = compat_alloc_user_space(sizeof(*args));
-	if (!args)
-		return -EFAULT;
-
-	if (copy_from_user(&args32,
-			   (struct vchiq_get_config32 __user *)arg,
-			   sizeof(args32)))
+	if (copy_from_user(&args32, arg, sizeof(args32)))
 		return -EFAULT;
+	if (args32.config_size > sizeof(config))
+		return -EINVAL;
 
-	if (put_user(args32.config_size, &args->config_size) ||
-	    put_user(compat_ptr(args32.pconfig), &args->pconfig))
+	vchiq_get_config(&config);
+	ptr = compat_ptr(args32.pconfig);
+	if (copy_to_user(ptr, &config, args32.config_size))
 		return -EFAULT;
 
-	return vchiq_ioctl(file, VCHIQ_IOC_GET_CONFIG, (unsigned long)args);
+	return 0;
 }
 
 static long
 vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
+	void __user *argp = compat_ptr(arg);
 	switch (cmd) {
 	case VCHIQ_IOC_CREATE_SERVICE32:
 		return vchiq_compat_ioctl_create_service(file, cmd, arg);
 	case VCHIQ_IOC_QUEUE_MESSAGE32:
-		return vchiq_compat_ioctl_queue_message(file, cmd, arg);
+		return vchiq_compat_ioctl_queue_message(file, cmd, argp);
 	case VCHIQ_IOC_QUEUE_BULK_TRANSMIT32:
 	case VCHIQ_IOC_QUEUE_BULK_RECEIVE32:
 		return vchiq_compat_ioctl_queue_bulk(file, cmd, arg);
@@ -1903,9 +1906,9 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case VCHIQ_IOC_DEQUEUE_MESSAGE32:
 		return vchiq_compat_ioctl_dequeue_message(file, cmd, arg);
 	case VCHIQ_IOC_GET_CONFIG32:
-		return vchiq_compat_ioctl_get_config(file, cmd, arg);
+		return vchiq_compat_ioctl_get_config(file, cmd, argp);
 	default:
-		return vchiq_ioctl(file, cmd, arg);
+		return vchiq_ioctl(file, cmd, (unsigned long)argp);
 	}
 }
 
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/5] staging: vchiq: rework compat handling

[Arnd Bergmann]

The compat handlers for VCHIQ_IOC_QUEUE_MESSAGE32 and
VCHIQ_IOC_GET_CONFIG32 can simply call the underlying implementations
that are already separate functions rather than using copy_in_user to
simulate the native 64-bit interface for the full ioctl handler.

vchiq_ioc_queue_message gets a small update to the calling
conventions to simplify the compat version by directly
returning a normal errno value.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 109 +++++++++---------
 1 file changed, 56 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index d4d811884861..56a38bec848a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -765,12 +765,13 @@ static ssize_t vchiq_ioc_copy_element_data(void *context, void *dest,
  *   vchiq_ioc_queue_message
  *
  **************************************************************************/
-static enum vchiq_status
+static int
 vchiq_ioc_queue_message(unsigned int handle,
 			struct vchiq_element *elements,
 			unsigned long count)
 {
 	struct vchiq_io_copy_callback_context context;
+	enum vchiq_status status = VCHIQ_SUCCESS;
 	unsigned long i;
 	size_t total_size = 0;
 
@@ -785,8 +786,14 @@ vchiq_ioc_queue_message(unsigned int handle,
 		total_size += elements[i].size;
 	}
 
-	return vchiq_queue_message(handle, vchiq_ioc_copy_element_data,
-				   &context, total_size);
+	status = vchiq_queue_message(handle, vchiq_ioc_copy_element_data,
+				     &context, total_size);
+
+	if (status == VCHIQ_ERROR)
+		return -EIO;
+	else if (status == VCHIQ_RETRY)
+		return -EINTR;
+	return 0;
 }
 
 /****************************************************************************
@@ -1020,9 +1027,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 			if (copy_from_user(elements, args.elements,
 				args.count * sizeof(struct vchiq_element)) == 0)
-				status = vchiq_ioc_queue_message
-					(args.handle,
-					elements, args.count);
+				ret = vchiq_ioc_queue_message(args.handle, elements,
+							      args.count);
 			else
 				ret = -EFAULT;
 		} else {
@@ -1550,55 +1556,53 @@ struct vchiq_queue_message32 {
 static long
 vchiq_compat_ioctl_queue_message(struct file *file,
 				 unsigned int cmd,
-				 unsigned long arg)
+				 struct vchiq_queue_message32 __user *arg)
 {
-	struct vchiq_queue_message __user *args;
-	struct vchiq_element __user *elements;
+	struct vchiq_queue_message args;
 	struct vchiq_queue_message32 args32;
-	unsigned int count;
-
-	if (copy_from_user(&args32,
-			   (struct vchiq_queue_message32 __user *)arg,
-			   sizeof(args32)))
-		return -EFAULT;
-
-	args = compat_alloc_user_space(sizeof(*args) +
-				       (sizeof(*elements) * MAX_ELEMENTS));
+	struct vchiq_service *service;
+	int ret;
 
-	if (!args)
+	if (copy_from_user(&args32, arg, sizeof(args32)))
 		return -EFAULT;
 
-	if (put_user(args32.handle, &args->handle) ||
-	    put_user(args32.count, &args->count) ||
-	    put_user(compat_ptr(args32.elements), &args->elements))
-		return -EFAULT;
+	args = (struct vchiq_queue_message) {
+		.handle   = args32.handle,
+		.count    = args32.count,
+		.elements = compat_ptr(args32.elements),
+	};
 
 	if (args32.count > MAX_ELEMENTS)
 		return -EINVAL;
 
-	if (args32.elements && args32.count) {
-		struct vchiq_element32 tempelement32[MAX_ELEMENTS];
+	service = find_service_for_instance(file->private_data, args.handle);
+	if (!service)
+		return -EINVAL;
 
-		elements = (struct vchiq_element __user *)(args + 1);
+	if (args32.elements && args32.count) {
+		struct vchiq_element32 element32[MAX_ELEMENTS];
+		struct vchiq_element elements[MAX_ELEMENTS];
+		unsigned int count;
 
-		if (copy_from_user(&tempelement32,
-				   compat_ptr(args32.elements),
-				   sizeof(tempelement32)))
+		if (copy_from_user(&element32, args.elements,
+				   sizeof(element32))) {
+			unlock_service(service);
 			return -EFAULT;
+		}
 
 		for (count = 0; count < args32.count; count++) {
-			if (put_user(compat_ptr(tempelement32[count].data),
-				     &elements[count].data) ||
-			    put_user(tempelement32[count].size,
-				     &elements[count].size))
-				return -EFAULT;
+			elements[count].data =
+				compat_ptr(element32[count].data);
+			elements[count].size = element32[count].size;
 		}
-
-		if (put_user(elements, &args->elements))
-			return -EFAULT;
+		ret = vchiq_ioc_queue_message(args.handle, elements,
+					      args.count);
+	} else {
+		ret = -EINVAL;
 	}
+	unlock_service(service);
 
-	return vchiq_ioctl(file, VCHIQ_IOC_QUEUE_MESSAGE, (unsigned long)args);
+	return ret;
 }
 
 struct vchiq_queue_bulk_transfer32 {
@@ -1866,35 +1870,34 @@ struct vchiq_get_config32 {
 static long
 vchiq_compat_ioctl_get_config(struct file *file,
 			      unsigned int cmd,
-			      unsigned long arg)
+			      struct vchiq_get_config32 __user *arg)
 {
-	struct vchiq_get_config __user *args;
 	struct vchiq_get_config32 args32;
+	struct vchiq_config config;
+	void __user *ptr;
 
-	args = compat_alloc_user_space(sizeof(*args));
-	if (!args)
-		return -EFAULT;
-
-	if (copy_from_user(&args32,
-			   (struct vchiq_get_config32 __user *)arg,
-			   sizeof(args32)))
+	if (copy_from_user(&args32, arg, sizeof(args32)))
 		return -EFAULT;
+	if (args32.config_size > sizeof(config))
+		return -EINVAL;
 
-	if (put_user(args32.config_size, &args->config_size) ||
-	    put_user(compat_ptr(args32.pconfig), &args->pconfig))
+	vchiq_get_config(&config);
+	ptr = compat_ptr(args32.pconfig);
+	if (copy_to_user(ptr, &config, args32.config_size))
 		return -EFAULT;
 
-	return vchiq_ioctl(file, VCHIQ_IOC_GET_CONFIG, (unsigned long)args);
+	return 0;
 }
 
 static long
 vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
+	void __user *argp = compat_ptr(arg);
 	switch (cmd) {
 	case VCHIQ_IOC_CREATE_SERVICE32:
 		return vchiq_compat_ioctl_create_service(file, cmd, arg);
 	case VCHIQ_IOC_QUEUE_MESSAGE32:
-		return vchiq_compat_ioctl_queue_message(file, cmd, arg);
+		return vchiq_compat_ioctl_queue_message(file, cmd, argp);
 	case VCHIQ_IOC_QUEUE_BULK_TRANSMIT32:
 	case VCHIQ_IOC_QUEUE_BULK_RECEIVE32:
 		return vchiq_compat_ioctl_queue_bulk(file, cmd, arg);
@@ -1903,9 +1906,9 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case VCHIQ_IOC_DEQUEUE_MESSAGE32:
 		return vchiq_compat_ioctl_dequeue_message(file, cmd, arg);
 	case VCHIQ_IOC_GET_CONFIG32:
-		return vchiq_compat_ioctl_get_config(file, cmd, arg);
+		return vchiq_compat_ioctl_get_config(file, cmd, argp);
 	default:
-		return vchiq_ioctl(file, cmd, arg);
+		return vchiq_ioctl(file, cmd, (unsigned long)argp);
 	}
 }
 
-- 
2.27.0

[PATCH 2/5] staging: vchiq: convert compat create_service

[Arnd Bergmann]

Split out the ioctl implementation for VCHIQ_IOC_CREATE_SERVICE
into a separate function so it can be shared with the compat
implementation.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 189 +++++++++---------
 1 file changed, 89 insertions(+), 100 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 56a38bec848a..1404a5a0c7b0 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -796,6 +796,68 @@ vchiq_ioc_queue_message(unsigned int handle,
 	return 0;
 }
 
+static int vchiq_ioc_create_service(struct vchiq_instance *instance,
+				    struct vchiq_create_service *args)
+{
+	struct user_service *user_service = NULL;
+	struct vchiq_service *service;
+	enum vchiq_status status = VCHIQ_SUCCESS;
+	void *userdata;
+	int srvstate;
+
+	user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
+	if (!user_service)
+		return -ENOMEM;
+
+	if (args->is_open) {
+		if (!instance->connected) {
+			kfree(user_service);
+			return -ENOTCONN;
+		}
+		srvstate = VCHIQ_SRVSTATE_OPENING;
+	} else {
+		srvstate = instance->connected ?
+			 VCHIQ_SRVSTATE_LISTENING : VCHIQ_SRVSTATE_HIDDEN;
+	}
+
+	userdata = args->params.userdata;
+	args->params.callback = service_callback;
+	args->params.userdata = user_service;
+	service = vchiq_add_service_internal(instance->state, &args->params,
+					     srvstate, instance,
+					     user_service_free);
+
+	if (!service) {
+		kfree(user_service);
+		return -EEXIST;
+	}
+
+	user_service->service = service;
+	user_service->userdata = userdata;
+	user_service->instance = instance;
+	user_service->is_vchi = (args->is_vchi != 0);
+	user_service->dequeue_pending = 0;
+	user_service->close_pending = 0;
+	user_service->message_available_pos = instance->completion_remove - 1;
+	user_service->msg_insert = 0;
+	user_service->msg_remove = 0;
+	init_completion(&user_service->insert_event);
+	init_completion(&user_service->remove_event);
+	init_completion(&user_service->close_event);
+
+	if (args->is_open) {
+		status = vchiq_open_service_internal(service, instance->pid);
+		if (status != VCHIQ_SUCCESS) {
+			vchiq_remove_service(service->handle);
+			return (status == VCHIQ_RETRY) ?
+				-EINTR : -EIO;
+		}
+	}
+	args->handle = service->handle;
+
+	return 0;
+}
+
 /****************************************************************************
 *
 *   vchiq_ioctl
@@ -868,85 +930,22 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		break;
 
 	case VCHIQ_IOC_CREATE_SERVICE: {
+		struct vchiq_create_service __user *argp;
 		struct vchiq_create_service args;
-		struct user_service *user_service = NULL;
-		void *userdata;
-		int srvstate;
 
-		if (copy_from_user(&args, (const void __user *)arg,
-				   sizeof(args))) {
+		argp = (void __user *)arg;
+		if (copy_from_user(&args, argp, sizeof(args))) {
 			ret = -EFAULT;
 			break;
 		}
 
-		user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
-		if (!user_service) {
-			ret = -ENOMEM;
+		ret = vchiq_ioc_create_service(instance, &args);
+		if (ret < 0)
 			break;
-		}
-
-		if (args.is_open) {
-			if (!instance->connected) {
-				ret = -ENOTCONN;
-				kfree(user_service);
-				break;
-			}
-			srvstate = VCHIQ_SRVSTATE_OPENING;
-		} else {
-			srvstate =
-				 instance->connected ?
-				 VCHIQ_SRVSTATE_LISTENING :
-				 VCHIQ_SRVSTATE_HIDDEN;
-		}
 
-		userdata = args.params.userdata;
-		args.params.callback = service_callback;
-		args.params.userdata = user_service;
-		service = vchiq_add_service_internal(
-				instance->state,
-				&args.params, srvstate,
-				instance, user_service_free);
-
-		if (service) {
-			user_service->service = service;
-			user_service->userdata = userdata;
-			user_service->instance = instance;
-			user_service->is_vchi = (args.is_vchi != 0);
-			user_service->dequeue_pending = 0;
-			user_service->close_pending = 0;
-			user_service->message_available_pos =
-				instance->completion_remove - 1;
-			user_service->msg_insert = 0;
-			user_service->msg_remove = 0;
-			init_completion(&user_service->insert_event);
-			init_completion(&user_service->remove_event);
-			init_completion(&user_service->close_event);
-
-			if (args.is_open) {
-				status = vchiq_open_service_internal
-					(service, instance->pid);
-				if (status != VCHIQ_SUCCESS) {
-					vchiq_remove_service(service->handle);
-					service = NULL;
-					ret = (status == VCHIQ_RETRY) ?
-						-EINTR : -EIO;
-					break;
-				}
-			}
-
-			if (copy_to_user((void __user *)
-				&(((struct vchiq_create_service __user *)
-					arg)->handle),
-				(const void *)&service->handle,
-				sizeof(service->handle))) {
-				ret = -EFAULT;
-				vchiq_remove_service(service->handle);
-			}
-
-			service = NULL;
-		} else {
-			ret = -EEXIST;
-			kfree(user_service);
+		if (put_user(args.handle, &argp->handle)) {
+			vchiq_remove_service(args.handle);
+			ret = -EFAULT;
 		}
 	} break;
 
@@ -1495,46 +1494,36 @@ static long
 vchiq_compat_ioctl_create_service(
 	struct file *file,
 	unsigned int cmd,
-	unsigned long arg)
+	struct vchiq_create_service32 __user *ptrargs32)
 {
-	struct vchiq_create_service __user *args;
-	struct vchiq_create_service32 __user *ptrargs32 =
-		(struct vchiq_create_service32 __user *)arg;
+	struct vchiq_create_service args;
 	struct vchiq_create_service32 args32;
 	long ret;
 
-	args = compat_alloc_user_space(sizeof(*args));
-	if (!args)
-		return -EFAULT;
-
 	if (copy_from_user(&args32, ptrargs32, sizeof(args32)))
 		return -EFAULT;
 
-	if (put_user(args32.params.fourcc, &args->params.fourcc) ||
-	    put_user(compat_ptr(args32.params.callback),
-		     &args->params.callback) ||
-	    put_user(compat_ptr(args32.params.userdata),
-		     &args->params.userdata) ||
-	    put_user(args32.params.version, &args->params.version) ||
-	    put_user(args32.params.version_min,
-		     &args->params.version_min) ||
-	    put_user(args32.is_open, &args->is_open) ||
-	    put_user(args32.is_vchi, &args->is_vchi) ||
-	    put_user(args32.handle, &args->handle))
-		return -EFAULT;
-
-	ret = vchiq_ioctl(file, VCHIQ_IOC_CREATE_SERVICE, (unsigned long)args);
+	args = (struct vchiq_create_service) {
+		.params = {
+			.fourcc	     = args32.params.fourcc,
+			.callback    = compat_ptr(args32.params.callback),
+			.userdata    = compat_ptr(args32.params.userdata),
+			.version     = args32.params.version,
+			.version_min = args32.params.version_min,
+		},
+		.is_open = args32.is_open,
+		.is_vchi = args32.is_vchi,
+		.handle  = args32.handle,
+	};
 
+	ret = vchiq_ioc_create_service(file->private_data, &args);
 	if (ret < 0)
 		return ret;
 
-	if (get_user(args32.handle, &args->handle))
-		return -EFAULT;
-
-	if (copy_to_user(&ptrargs32->handle,
-			 &args32.handle,
-			 sizeof(args32.handle)))
+	if (put_user(args.handle, &ptrargs32->handle)) {
+		vchiq_remove_service(args.handle);
 		return -EFAULT;
+	}
 
 	return 0;
 }
@@ -1895,7 +1884,7 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	void __user *argp = compat_ptr(arg);
 	switch (cmd) {
 	case VCHIQ_IOC_CREATE_SERVICE32:
-		return vchiq_compat_ioctl_create_service(file, cmd, arg);
+		return vchiq_compat_ioctl_create_service(file, cmd, argp);
 	case VCHIQ_IOC_QUEUE_MESSAGE32:
 		return vchiq_compat_ioctl_queue_message(file, cmd, argp);
 	case VCHIQ_IOC_QUEUE_BULK_TRANSMIT32:
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/5] staging: vchiq: convert compat create_service

[Arnd Bergmann]

Split out the ioctl implementation for VCHIQ_IOC_CREATE_SERVICE
into a separate function so it can be shared with the compat
implementation.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 189 +++++++++---------
 1 file changed, 89 insertions(+), 100 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 56a38bec848a..1404a5a0c7b0 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -796,6 +796,68 @@ vchiq_ioc_queue_message(unsigned int handle,
 	return 0;
 }
 
+static int vchiq_ioc_create_service(struct vchiq_instance *instance,
+				    struct vchiq_create_service *args)
+{
+	struct user_service *user_service = NULL;
+	struct vchiq_service *service;
+	enum vchiq_status status = VCHIQ_SUCCESS;
+	void *userdata;
+	int srvstate;
+
+	user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
+	if (!user_service)
+		return -ENOMEM;
+
+	if (args->is_open) {
+		if (!instance->connected) {
+			kfree(user_service);
+			return -ENOTCONN;
+		}
+		srvstate = VCHIQ_SRVSTATE_OPENING;
+	} else {
+		srvstate = instance->connected ?
+			 VCHIQ_SRVSTATE_LISTENING : VCHIQ_SRVSTATE_HIDDEN;
+	}
+
+	userdata = args->params.userdata;
+	args->params.callback = service_callback;
+	args->params.userdata = user_service;
+	service = vchiq_add_service_internal(instance->state, &args->params,
+					     srvstate, instance,
+					     user_service_free);
+
+	if (!service) {
+		kfree(user_service);
+		return -EEXIST;
+	}
+
+	user_service->service = service;
+	user_service->userdata = userdata;
+	user_service->instance = instance;
+	user_service->is_vchi = (args->is_vchi != 0);
+	user_service->dequeue_pending = 0;
+	user_service->close_pending = 0;
+	user_service->message_available_pos = instance->completion_remove - 1;
+	user_service->msg_insert = 0;
+	user_service->msg_remove = 0;
+	init_completion(&user_service->insert_event);
+	init_completion(&user_service->remove_event);
+	init_completion(&user_service->close_event);
+
+	if (args->is_open) {
+		status = vchiq_open_service_internal(service, instance->pid);
+		if (status != VCHIQ_SUCCESS) {
+			vchiq_remove_service(service->handle);
+			return (status == VCHIQ_RETRY) ?
+				-EINTR : -EIO;
+		}
+	}
+	args->handle = service->handle;
+
+	return 0;
+}
+
 /****************************************************************************
 *
 *   vchiq_ioctl
@@ -868,85 +930,22 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		break;
 
 	case VCHIQ_IOC_CREATE_SERVICE: {
+		struct vchiq_create_service __user *argp;
 		struct vchiq_create_service args;
-		struct user_service *user_service = NULL;
-		void *userdata;
-		int srvstate;
 
-		if (copy_from_user(&args, (const void __user *)arg,
-				   sizeof(args))) {
+		argp = (void __user *)arg;
+		if (copy_from_user(&args, argp, sizeof(args))) {
 			ret = -EFAULT;
 			break;
 		}
 
-		user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
-		if (!user_service) {
-			ret = -ENOMEM;
+		ret = vchiq_ioc_create_service(instance, &args);
+		if (ret < 0)
 			break;
-		}
-
-		if (args.is_open) {
-			if (!instance->connected) {
-				ret = -ENOTCONN;
-				kfree(user_service);
-				break;
-			}
-			srvstate = VCHIQ_SRVSTATE_OPENING;
-		} else {
-			srvstate =
-				 instance->connected ?
-				 VCHIQ_SRVSTATE_LISTENING :
-				 VCHIQ_SRVSTATE_HIDDEN;
-		}
 
-		userdata = args.params.userdata;
-		args.params.callback = service_callback;
-		args.params.userdata = user_service;
-		service = vchiq_add_service_internal(
-				instance->state,
-				&args.params, srvstate,
-				instance, user_service_free);
-
-		if (service) {
-			user_service->service = service;
-			user_service->userdata = userdata;
-			user_service->instance = instance;
-			user_service->is_vchi = (args.is_vchi != 0);
-			user_service->dequeue_pending = 0;
-			user_service->close_pending = 0;
-			user_service->message_available_pos =
-				instance->completion_remove - 1;
-			user_service->msg_insert = 0;
-			user_service->msg_remove = 0;
-			init_completion(&user_service->insert_event);
-			init_completion(&user_service->remove_event);
-			init_completion(&user_service->close_event);
-
-			if (args.is_open) {
-				status = vchiq_open_service_internal
-					(service, instance->pid);
-				if (status != VCHIQ_SUCCESS) {
-					vchiq_remove_service(service->handle);
-					service = NULL;
-					ret = (status == VCHIQ_RETRY) ?
-						-EINTR : -EIO;
-					break;
-				}
-			}
-
-			if (copy_to_user((void __user *)
-				&(((struct vchiq_create_service __user *)
-					arg)->handle),
-				(const void *)&service->handle,
-				sizeof(service->handle))) {
-				ret = -EFAULT;
-				vchiq_remove_service(service->handle);
-			}
-
-			service = NULL;
-		} else {
-			ret = -EEXIST;
-			kfree(user_service);
+		if (put_user(args.handle, &argp->handle)) {
+			vchiq_remove_service(args.handle);
+			ret = -EFAULT;
 		}
 	} break;
 
@@ -1495,46 +1494,36 @@ static long
 vchiq_compat_ioctl_create_service(
 	struct file *file,
 	unsigned int cmd,
-	unsigned long arg)
+	struct vchiq_create_service32 __user *ptrargs32)
 {
-	struct vchiq_create_service __user *args;
-	struct vchiq_create_service32 __user *ptrargs32 =
-		(struct vchiq_create_service32 __user *)arg;
+	struct vchiq_create_service args;
 	struct vchiq_create_service32 args32;
 	long ret;
 
-	args = compat_alloc_user_space(sizeof(*args));
-	if (!args)
-		return -EFAULT;
-
 	if (copy_from_user(&args32, ptrargs32, sizeof(args32)))
 		return -EFAULT;
 
-	if (put_user(args32.params.fourcc, &args->params.fourcc) ||
-	    put_user(compat_ptr(args32.params.callback),
-		     &args->params.callback) ||
-	    put_user(compat_ptr(args32.params.userdata),
-		     &args->params.userdata) ||
-	    put_user(args32.params.version, &args->params.version) ||
-	    put_user(args32.params.version_min,
-		     &args->params.version_min) ||
-	    put_user(args32.is_open, &args->is_open) ||
-	    put_user(args32.is_vchi, &args->is_vchi) ||
-	    put_user(args32.handle, &args->handle))
-		return -EFAULT;
-
-	ret = vchiq_ioctl(file, VCHIQ_IOC_CREATE_SERVICE, (unsigned long)args);
+	args = (struct vchiq_create_service) {
+		.params = {
+			.fourcc	     = args32.params.fourcc,
+			.callback    = compat_ptr(args32.params.callback),
+			.userdata    = compat_ptr(args32.params.userdata),
+			.version     = args32.params.version,
+			.version_min = args32.params.version_min,
+		},
+		.is_open = args32.is_open,
+		.is_vchi = args32.is_vchi,
+		.handle  = args32.handle,
+	};
 
+	ret = vchiq_ioc_create_service(file->private_data, &args);
 	if (ret < 0)
 		return ret;
 
-	if (get_user(args32.handle, &args->handle))
-		return -EFAULT;
-
-	if (copy_to_user(&ptrargs32->handle,
-			 &args32.handle,
-			 sizeof(args32.handle)))
+	if (put_user(args.handle, &ptrargs32->handle)) {
+		vchiq_remove_service(args.handle);
 		return -EFAULT;
+	}
 
 	return 0;
 }
@@ -1895,7 +1884,7 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	void __user *argp = compat_ptr(arg);
 	switch (cmd) {
 	case VCHIQ_IOC_CREATE_SERVICE32:
-		return vchiq_compat_ioctl_create_service(file, cmd, arg);
+		return vchiq_compat_ioctl_create_service(file, cmd, argp);
 	case VCHIQ_IOC_QUEUE_MESSAGE32:
 		return vchiq_compat_ioctl_queue_message(file, cmd, argp);
 	case VCHIQ_IOC_QUEUE_BULK_TRANSMIT32:
-- 
2.27.0

Re: [PATCH 2/5] staging: vchiq: convert compat create_service

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 8153 bytes --]

Hi Arnd,

I love your patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/staging-vchiq-rework-compat-handling/20200918-185438
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 8436f932d84b1d53d2f4a2fa88c7aacdb0313265
config: arm64-randconfig-s032-20200921 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-201-g24bdaac6-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1509:50: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected int enum vchiq_status ( *callback )( ... ) @@     got void [noderef] __user * @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1509:50: sparse:     expected int enum vchiq_status ( *callback )( ... )
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1509:50: sparse:     got void [noderef] __user *
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1510:50: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *userdata @@     got void [noderef] __user * @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1510:50: sparse:     expected void *userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1510:50: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1629:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1629:13: sparse:     expected void *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1629:13: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1631:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1631:13: sparse:     expected void *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1631:13: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1706:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct vchiq_completion_data *__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1706:13: sparse:     expected struct vchiq_completion_data *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1706:13: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1709:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void **__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1709:13: sparse:     expected void **__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1709:13: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1756:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct vchiq_completion_data *__pu_val @@     got struct vchiq_completion_data [noderef] __user *[assigned] completion @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1756:13: sparse:     expected struct vchiq_completion_data *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1756:13: sparse:     got struct vchiq_completion_data [noderef] __user *[assigned] completion
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1786:59: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got struct vchiq_header *[addressable] header @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1786:59: sparse:     expected void [noderef] __user *uptr
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1786:59: sparse:     got struct vchiq_header *[addressable] header
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1788:45: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got void *[addressable] service_userdata @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1788:45: sparse:     expected void [noderef] __user *uptr
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1788:45: sparse:     got void *[addressable] service_userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1790:45: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got void *[addressable] bulk_userdata @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1790:45: sparse:     expected void [noderef] __user *uptr
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1790:45: sparse:     got void *[addressable] bulk_userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1844:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1844:13: sparse:     expected void *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1844:13: sparse:     got void [noderef] __user *

# https://github.com/0day-ci/linux/commit/69c0b51efb112af6fa886dca152e2bd3824c126c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Arnd-Bergmann/staging-vchiq-rework-compat-handling/20200918-185438
git checkout 69c0b51efb112af6fa886dca152e2bd3824c126c
vim +1509 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c

  1489	
  1490	#define VCHIQ_IOC_CREATE_SERVICE32 \
  1491		_IOWR(VCHIQ_IOC_MAGIC, 2, struct vchiq_create_service32)
  1492	
  1493	static long
  1494	vchiq_compat_ioctl_create_service(
  1495		struct file *file,
  1496		unsigned int cmd,
  1497		struct vchiq_create_service32 __user *ptrargs32)
  1498	{
  1499		struct vchiq_create_service args;
  1500		struct vchiq_create_service32 args32;
  1501		long ret;
  1502	
  1503		if (copy_from_user(&args32, ptrargs32, sizeof(args32)))
  1504			return -EFAULT;
  1505	
  1506		args = (struct vchiq_create_service) {
  1507			.params = {
  1508				.fourcc	     = args32.params.fourcc,
> 1509				.callback    = compat_ptr(args32.params.callback),
> 1510				.userdata    = compat_ptr(args32.params.userdata),
  1511				.version     = args32.params.version,
  1512				.version_min = args32.params.version_min,
  1513			},
  1514			.is_open = args32.is_open,
  1515			.is_vchi = args32.is_vchi,
  1516			.handle  = args32.handle,
  1517		};
  1518	
  1519		ret = vchiq_ioc_create_service(file->private_data, &args);
  1520		if (ret < 0)
  1521			return ret;
  1522	
  1523		if (put_user(args.handle, &ptrargs32->handle)) {
  1524			vchiq_remove_service(args.handle);
  1525			return -EFAULT;
  1526		}
  1527	
  1528		return 0;
  1529	}
  1530	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33311 bytes --]

[PATCH 3/5] staging: vchiq: convert compat dequeue_message

[Arnd Bergmann]

Split out the ioctl implementation for VCHIQ_IOC_DEQUEUE_MESSAGE
into a separate function so it can be shared with the compat
implementation.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 180 +++++++++---------
 1 file changed, 92 insertions(+), 88 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 1404a5a0c7b0..cbe9583a0114 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -858,6 +858,86 @@ static int vchiq_ioc_create_service(struct vchiq_instance *instance,
 	return 0;
 }
 
+static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
+				     struct vchiq_dequeue_message *args)
+{
+	struct user_service *user_service;
+	struct vchiq_service *service;
+	struct vchiq_header *header;
+	int ret;
+
+	DEBUG_INITIALISE(g_state.local)
+	DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+	service = find_service_for_instance(instance, args->handle);
+	if (!service)
+		return -EINVAL;
+
+	user_service = (struct user_service *)service->base.userdata;
+	if (user_service->is_vchi == 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	spin_lock(&msg_queue_spinlock);
+	if (user_service->msg_remove == user_service->msg_insert) {
+		if (!args->blocking) {
+			spin_unlock(&msg_queue_spinlock);
+			DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+			ret = -EWOULDBLOCK;
+			goto out;
+		}
+		user_service->dequeue_pending = 1;
+		ret = 0;
+		do {
+			spin_unlock(&msg_queue_spinlock);
+			DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+			if (wait_for_completion_interruptible(
+				&user_service->insert_event)) {
+				vchiq_log_info(vchiq_arm_log_level,
+					"DEQUEUE_MESSAGE interrupted");
+				ret = -EINTR;
+				break;
+			}
+			spin_lock(&msg_queue_spinlock);
+		} while (user_service->msg_remove ==
+			user_service->msg_insert);
+
+		if (ret)
+			goto out;
+	}
+
+	BUG_ON((int)(user_service->msg_insert -
+		user_service->msg_remove) < 0);
+
+	header = user_service->msg_queue[user_service->msg_remove &
+		(MSG_QUEUE_SIZE - 1)];
+	user_service->msg_remove++;
+	spin_unlock(&msg_queue_spinlock);
+
+	complete(&user_service->remove_event);
+	if (!header) {
+		ret = -ENOTCONN;
+	} else if (header->size <= args->bufsize) {
+		/* Copy to user space if msgbuf is not NULL */
+		if (!args->buf || (copy_to_user((void __user *)args->buf,
+					header->data, header->size) == 0)) {
+			ret = header->size;
+			vchiq_release_message(service->handle, header);
+		} else
+			ret = -EFAULT;
+	} else {
+		vchiq_log_error(vchiq_arm_log_level,
+			"header %pK: bufsize %x < size %x",
+			header, args->bufsize, header->size);
+		WARN(1, "invalid size\n");
+		ret = -EMSGSIZE;
+	}
+	DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+out:
+	unlock_service(service);
+	return ret;
+}
+
 /****************************************************************************
 *
 *   vchiq_ioctl
@@ -1287,84 +1367,14 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 	case VCHIQ_IOC_DEQUEUE_MESSAGE: {
 		struct vchiq_dequeue_message args;
-		struct user_service *user_service;
-		struct vchiq_header *header;
 
-		DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
 		if (copy_from_user(&args, (const void __user *)arg,
 				   sizeof(args))) {
 			ret = -EFAULT;
 			break;
 		}
-		service = find_service_for_instance(instance, args.handle);
-		if (!service) {
-			ret = -EINVAL;
-			break;
-		}
-		user_service = (struct user_service *)service->base.userdata;
-		if (user_service->is_vchi == 0) {
-			ret = -EINVAL;
-			break;
-		}
-
-		spin_lock(&msg_queue_spinlock);
-		if (user_service->msg_remove == user_service->msg_insert) {
-			if (!args.blocking) {
-				spin_unlock(&msg_queue_spinlock);
-				DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
-				ret = -EWOULDBLOCK;
-				break;
-			}
-			user_service->dequeue_pending = 1;
-			do {
-				spin_unlock(&msg_queue_spinlock);
-				DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
-				if (wait_for_completion_interruptible(
-					&user_service->insert_event)) {
-					vchiq_log_info(vchiq_arm_log_level,
-						"DEQUEUE_MESSAGE interrupted");
-					ret = -EINTR;
-					break;
-				}
-				spin_lock(&msg_queue_spinlock);
-			} while (user_service->msg_remove ==
-				user_service->msg_insert);
-
-			if (ret)
-				break;
-		}
-
-		BUG_ON((int)(user_service->msg_insert -
-			user_service->msg_remove) < 0);
 
-		header = user_service->msg_queue[user_service->msg_remove &
-			(MSG_QUEUE_SIZE - 1)];
-		user_service->msg_remove++;
-		spin_unlock(&msg_queue_spinlock);
-
-		complete(&user_service->remove_event);
-		if (!header)
-			ret = -ENOTCONN;
-		else if (header->size <= args.bufsize) {
-			/* Copy to user space if msgbuf is not NULL */
-			if (!args.buf ||
-				(copy_to_user((void __user *)args.buf,
-				header->data,
-				header->size) == 0)) {
-				ret = header->size;
-				vchiq_release_message(
-					service->handle,
-					header);
-			} else
-				ret = -EFAULT;
-		} else {
-			vchiq_log_error(vchiq_arm_log_level,
-				"header %pK: bufsize %x < size %x",
-				header, args.bufsize, header->size);
-			WARN(1, "invalid size\n");
-			ret = -EMSGSIZE;
-		}
-		DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+		ret = vchiq_ioc_dequeue_message(instance, &args);
 	} break;
 
 	case VCHIQ_IOC_GET_CLIENT_ID: {
@@ -1824,28 +1834,22 @@ struct vchiq_dequeue_message32 {
 static long
 vchiq_compat_ioctl_dequeue_message(struct file *file,
 				   unsigned int cmd,
-				   unsigned long arg)
+				   struct vchiq_dequeue_message32 __user *arg)
 {
-	struct vchiq_dequeue_message __user *args;
 	struct vchiq_dequeue_message32 args32;
+	struct vchiq_dequeue_message args;
 
-	args = compat_alloc_user_space(sizeof(*args));
-	if (!args)
-		return -EFAULT;
-
-	if (copy_from_user(&args32,
-			   (struct vchiq_dequeue_message32 __user *)arg,
-			   sizeof(args32)))
+	if (copy_from_user(&args32, arg, sizeof(args32)))
 		return -EFAULT;
 
-	if (put_user(args32.handle, &args->handle) ||
-	    put_user(args32.blocking, &args->blocking) ||
-	    put_user(args32.bufsize, &args->bufsize) ||
-	    put_user(compat_ptr(args32.buf), &args->buf))
-		return -EFAULT;
+	args = (struct vchiq_dequeue_message) {
+		.handle		= args32.handle,
+		.blocking	= args32.blocking,
+		.bufsize	= args32.bufsize,
+		.buf		= compat_ptr(args32.buf),
+	};
 
-	return vchiq_ioctl(file, VCHIQ_IOC_DEQUEUE_MESSAGE,
-			   (unsigned long)args);
+	return vchiq_ioc_dequeue_message(file->private_data, &args);
 }
 
 struct vchiq_get_config32 {
@@ -1893,7 +1897,7 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case VCHIQ_IOC_AWAIT_COMPLETION32:
 		return vchiq_compat_ioctl_await_completion(file, cmd, arg);
 	case VCHIQ_IOC_DEQUEUE_MESSAGE32:
-		return vchiq_compat_ioctl_dequeue_message(file, cmd, arg);
+		return vchiq_compat_ioctl_dequeue_message(file, cmd, argp);
 	case VCHIQ_IOC_GET_CONFIG32:
 		return vchiq_compat_ioctl_get_config(file, cmd, argp);
 	default:
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/5] staging: vchiq: convert compat dequeue_message

[Arnd Bergmann]

Split out the ioctl implementation for VCHIQ_IOC_DEQUEUE_MESSAGE
into a separate function so it can be shared with the compat
implementation.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 180 +++++++++---------
 1 file changed, 92 insertions(+), 88 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 1404a5a0c7b0..cbe9583a0114 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -858,6 +858,86 @@ static int vchiq_ioc_create_service(struct vchiq_instance *instance,
 	return 0;
 }
 
+static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
+				     struct vchiq_dequeue_message *args)
+{
+	struct user_service *user_service;
+	struct vchiq_service *service;
+	struct vchiq_header *header;
+	int ret;
+
+	DEBUG_INITIALISE(g_state.local)
+	DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+	service = find_service_for_instance(instance, args->handle);
+	if (!service)
+		return -EINVAL;
+
+	user_service = (struct user_service *)service->base.userdata;
+	if (user_service->is_vchi == 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	spin_lock(&msg_queue_spinlock);
+	if (user_service->msg_remove == user_service->msg_insert) {
+		if (!args->blocking) {
+			spin_unlock(&msg_queue_spinlock);
+			DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+			ret = -EWOULDBLOCK;
+			goto out;
+		}
+		user_service->dequeue_pending = 1;
+		ret = 0;
+		do {
+			spin_unlock(&msg_queue_spinlock);
+			DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+			if (wait_for_completion_interruptible(
+				&user_service->insert_event)) {
+				vchiq_log_info(vchiq_arm_log_level,
+					"DEQUEUE_MESSAGE interrupted");
+				ret = -EINTR;
+				break;
+			}
+			spin_lock(&msg_queue_spinlock);
+		} while (user_service->msg_remove ==
+			user_service->msg_insert);
+
+		if (ret)
+			goto out;
+	}
+
+	BUG_ON((int)(user_service->msg_insert -
+		user_service->msg_remove) < 0);
+
+	header = user_service->msg_queue[user_service->msg_remove &
+		(MSG_QUEUE_SIZE - 1)];
+	user_service->msg_remove++;
+	spin_unlock(&msg_queue_spinlock);
+
+	complete(&user_service->remove_event);
+	if (!header) {
+		ret = -ENOTCONN;
+	} else if (header->size <= args->bufsize) {
+		/* Copy to user space if msgbuf is not NULL */
+		if (!args->buf || (copy_to_user((void __user *)args->buf,
+					header->data, header->size) == 0)) {
+			ret = header->size;
+			vchiq_release_message(service->handle, header);
+		} else
+			ret = -EFAULT;
+	} else {
+		vchiq_log_error(vchiq_arm_log_level,
+			"header %pK: bufsize %x < size %x",
+			header, args->bufsize, header->size);
+		WARN(1, "invalid size\n");
+		ret = -EMSGSIZE;
+	}
+	DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+out:
+	unlock_service(service);
+	return ret;
+}
+
 /****************************************************************************
 *
 *   vchiq_ioctl
@@ -1287,84 +1367,14 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 	case VCHIQ_IOC_DEQUEUE_MESSAGE: {
 		struct vchiq_dequeue_message args;
-		struct user_service *user_service;
-		struct vchiq_header *header;
 
-		DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
 		if (copy_from_user(&args, (const void __user *)arg,
 				   sizeof(args))) {
 			ret = -EFAULT;
 			break;
 		}
-		service = find_service_for_instance(instance, args.handle);
-		if (!service) {
-			ret = -EINVAL;
-			break;
-		}
-		user_service = (struct user_service *)service->base.userdata;
-		if (user_service->is_vchi == 0) {
-			ret = -EINVAL;
-			break;
-		}
-
-		spin_lock(&msg_queue_spinlock);
-		if (user_service->msg_remove == user_service->msg_insert) {
-			if (!args.blocking) {
-				spin_unlock(&msg_queue_spinlock);
-				DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
-				ret = -EWOULDBLOCK;
-				break;
-			}
-			user_service->dequeue_pending = 1;
-			do {
-				spin_unlock(&msg_queue_spinlock);
-				DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
-				if (wait_for_completion_interruptible(
-					&user_service->insert_event)) {
-					vchiq_log_info(vchiq_arm_log_level,
-						"DEQUEUE_MESSAGE interrupted");
-					ret = -EINTR;
-					break;
-				}
-				spin_lock(&msg_queue_spinlock);
-			} while (user_service->msg_remove ==
-				user_service->msg_insert);
-
-			if (ret)
-				break;
-		}
-
-		BUG_ON((int)(user_service->msg_insert -
-			user_service->msg_remove) < 0);
 
-		header = user_service->msg_queue[user_service->msg_remove &
-			(MSG_QUEUE_SIZE - 1)];
-		user_service->msg_remove++;
-		spin_unlock(&msg_queue_spinlock);
-
-		complete(&user_service->remove_event);
-		if (!header)
-			ret = -ENOTCONN;
-		else if (header->size <= args.bufsize) {
-			/* Copy to user space if msgbuf is not NULL */
-			if (!args.buf ||
-				(copy_to_user((void __user *)args.buf,
-				header->data,
-				header->size) == 0)) {
-				ret = header->size;
-				vchiq_release_message(
-					service->handle,
-					header);
-			} else
-				ret = -EFAULT;
-		} else {
-			vchiq_log_error(vchiq_arm_log_level,
-				"header %pK: bufsize %x < size %x",
-				header, args.bufsize, header->size);
-			WARN(1, "invalid size\n");
-			ret = -EMSGSIZE;
-		}
-		DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
+		ret = vchiq_ioc_dequeue_message(instance, &args);
 	} break;
 
 	case VCHIQ_IOC_GET_CLIENT_ID: {
@@ -1824,28 +1834,22 @@ struct vchiq_dequeue_message32 {
 static long
 vchiq_compat_ioctl_dequeue_message(struct file *file,
 				   unsigned int cmd,
-				   unsigned long arg)
+				   struct vchiq_dequeue_message32 __user *arg)
 {
-	struct vchiq_dequeue_message __user *args;
 	struct vchiq_dequeue_message32 args32;
+	struct vchiq_dequeue_message args;
 
-	args = compat_alloc_user_space(sizeof(*args));
-	if (!args)
-		return -EFAULT;
-
-	if (copy_from_user(&args32,
-			   (struct vchiq_dequeue_message32 __user *)arg,
-			   sizeof(args32)))
+	if (copy_from_user(&args32, arg, sizeof(args32)))
 		return -EFAULT;
 
-	if (put_user(args32.handle, &args->handle) ||
-	    put_user(args32.blocking, &args->blocking) ||
-	    put_user(args32.bufsize, &args->bufsize) ||
-	    put_user(compat_ptr(args32.buf), &args->buf))
-		return -EFAULT;
+	args = (struct vchiq_dequeue_message) {
+		.handle		= args32.handle,
+		.blocking	= args32.blocking,
+		.bufsize	= args32.bufsize,
+		.buf		= compat_ptr(args32.buf),
+	};
 
-	return vchiq_ioctl(file, VCHIQ_IOC_DEQUEUE_MESSAGE,
-			   (unsigned long)args);
+	return vchiq_ioc_dequeue_message(file->private_data, &args);
 }
 
 struct vchiq_get_config32 {
@@ -1893,7 +1897,7 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case VCHIQ_IOC_AWAIT_COMPLETION32:
 		return vchiq_compat_ioctl_await_completion(file, cmd, arg);
 	case VCHIQ_IOC_DEQUEUE_MESSAGE32:
-		return vchiq_compat_ioctl_dequeue_message(file, cmd, arg);
+		return vchiq_compat_ioctl_dequeue_message(file, cmd, argp);
 	case VCHIQ_IOC_GET_CONFIG32:
 		return vchiq_compat_ioctl_get_config(file, cmd, argp);
 	default:
-- 
2.27.0

Re: [PATCH 3/5] staging: vchiq: convert compat dequeue_message

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 7637 bytes --]

Hi Arnd,

I love your patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/staging-vchiq-rework-compat-handling/20200918-185438
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 8436f932d84b1d53d2f4a2fa88c7aacdb0313265
config: arm64-randconfig-s032-20200921 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-201-g24bdaac6-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1519:50: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected int enum vchiq_status ( *callback )( ... ) @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1519:50: sparse:     expected int enum vchiq_status ( *callback )( ... )
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1519:50: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1520:50: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *userdata @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1520:50: sparse:     expected void *userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1520:50: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1639:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1639:13: sparse:     expected void *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1639:13: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1641:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1641:13: sparse:     expected void *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1641:13: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1716:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct vchiq_completion_data *__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1716:13: sparse:     expected struct vchiq_completion_data *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1716:13: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1719:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void **__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1719:13: sparse:     expected void **__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1719:13: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1766:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct vchiq_completion_data *__pu_val @@     got struct vchiq_completion_data [noderef] __user *[assigned] completion @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1766:13: sparse:     expected struct vchiq_completion_data *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1766:13: sparse:     got struct vchiq_completion_data [noderef] __user *[assigned] completion
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1796:59: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got struct vchiq_header *[addressable] header @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1796:59: sparse:     expected void [noderef] __user *uptr
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1796:59: sparse:     got struct vchiq_header *[addressable] header
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1798:45: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got void *[addressable] service_userdata @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1798:45: sparse:     expected void [noderef] __user *uptr
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1798:45: sparse:     got void *[addressable] service_userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1800:45: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got void *[addressable] bulk_userdata @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1800:45: sparse:     expected void [noderef] __user *uptr
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1800:45: sparse:     got void *[addressable] bulk_userdata
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1849:45: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *buf @@     got void [noderef] __user * @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1849:45: sparse:     expected void *buf
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1849:45: sparse:     got void [noderef] __user *

# https://github.com/0day-ci/linux/commit/43c2a8584dc3a6fc1d6042cc06e1d1da38001ba4
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Arnd-Bergmann/staging-vchiq-rework-compat-handling/20200918-185438
git checkout 43c2a8584dc3a6fc1d6042cc06e1d1da38001ba4
vim +1849 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c

  1830	
  1831	#define VCHIQ_IOC_DEQUEUE_MESSAGE32 \
  1832		_IOWR(VCHIQ_IOC_MAGIC, 8, struct vchiq_dequeue_message32)
  1833	
  1834	static long
  1835	vchiq_compat_ioctl_dequeue_message(struct file *file,
  1836					   unsigned int cmd,
  1837					   struct vchiq_dequeue_message32 __user *arg)
  1838	{
  1839		struct vchiq_dequeue_message32 args32;
  1840		struct vchiq_dequeue_message args;
  1841	
  1842		if (copy_from_user(&args32, arg, sizeof(args32)))
  1843			return -EFAULT;
  1844	
  1845		args = (struct vchiq_dequeue_message) {
  1846			.handle		= args32.handle,
  1847			.blocking	= args32.blocking,
  1848			.bufsize	= args32.bufsize,
> 1849			.buf		= compat_ptr(args32.buf),
  1850		};
  1851	
  1852		return vchiq_ioc_dequeue_message(file->private_data, &args);
  1853	}
  1854	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33311 bytes --]

[PATCH 4/5] staging: vchiq: convert compat bulk transfer

[Arnd Bergmann]

Split out the ioctl implementation for VCHIQ_IOC_QUEUE_BULK_TRANSMIT
into a separate function so it can be shared with the compat
implementation.

Here, the input data is converted separately in the compat
handler, while the output data is passed as a __user pointer
to thec vchiq_queue_bulk_transfer->mode word that is
compatible.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 220 +++++++++---------
 1 file changed, 109 insertions(+), 111 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index cbe9583a0114..50af7f4a1b7c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -938,6 +938,95 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
 	return ret;
 }
 
+static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
+				      struct vchiq_queue_bulk_transfer *args,
+				      enum vchiq_bulk_dir dir,
+				      enum vchiq_bulk_mode __user *mode)
+{
+	struct vchiq_service *service;
+	struct bulk_waiter_node *waiter = NULL;
+	int status = 0;
+	int ret;
+
+	service = find_service_for_instance(instance, args->handle);
+	if (!service)
+		return -EINVAL;
+
+	if (args->mode == VCHIQ_BULK_MODE_BLOCKING) {
+		waiter = kzalloc(sizeof(struct bulk_waiter_node),
+			GFP_KERNEL);
+		if (!waiter) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		args->userdata = &waiter->bulk_waiter;
+	} else if (args->mode == VCHIQ_BULK_MODE_WAITING) {
+		mutex_lock(&instance->bulk_waiter_list_mutex);
+		list_for_each_entry(waiter, &instance->bulk_waiter_list,
+				    list) {
+			if (waiter->pid == current->pid) {
+				list_del(&waiter->list);
+				break;
+			}
+		}
+		mutex_unlock(&instance->bulk_waiter_list_mutex);
+		if (!waiter) {
+			vchiq_log_error(vchiq_arm_log_level,
+				"no bulk_waiter found for pid %d",
+				current->pid);
+			ret = -ESRCH;
+			goto out;
+		}
+		vchiq_log_info(vchiq_arm_log_level,
+			"found bulk_waiter %pK for pid %d", waiter,
+			current->pid);
+		args->userdata = &waiter->bulk_waiter;
+	}
+
+	status = vchiq_bulk_transfer(args->handle, args->data, args->size,
+				     args->userdata, args->mode, dir);
+
+	if (!waiter) {
+		ret = 0;
+		goto out;
+	}
+
+	if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
+		!waiter->bulk_waiter.bulk) {
+		if (waiter->bulk_waiter.bulk) {
+			/* Cancel the signal when the transfer
+			** completes. */
+			spin_lock(&bulk_waiter_spinlock);
+			waiter->bulk_waiter.bulk->userdata = NULL;
+			spin_unlock(&bulk_waiter_spinlock);
+		}
+		kfree(waiter);
+		ret = 0;
+	} else {
+		const enum vchiq_bulk_mode mode_waiting =
+			VCHIQ_BULK_MODE_WAITING;
+		waiter->pid = current->pid;
+		mutex_lock(&instance->bulk_waiter_list_mutex);
+		list_add(&waiter->list, &instance->bulk_waiter_list);
+		mutex_unlock(&instance->bulk_waiter_list_mutex);
+		vchiq_log_info(vchiq_arm_log_level,
+			"saved bulk_waiter %pK for pid %d",
+			waiter, current->pid);
+
+		ret = put_user(mode_waiting, mode);
+	}
+out:
+	unlock_service(service);
+	if (ret)
+		return ret;
+	else if (status == VCHIQ_ERROR)
+		return -EIO;
+	else if (status == VCHIQ_RETRY)
+		return -EINTR;
+	return 0;
+}
+
 /****************************************************************************
 *
 *   vchiq_ioctl
@@ -1118,90 +1207,20 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case VCHIQ_IOC_QUEUE_BULK_TRANSMIT:
 	case VCHIQ_IOC_QUEUE_BULK_RECEIVE: {
 		struct vchiq_queue_bulk_transfer args;
-		struct bulk_waiter_node *waiter = NULL;
+		struct vchiq_queue_bulk_transfer __user *argp;
 
 		enum vchiq_bulk_dir dir =
 			(cmd == VCHIQ_IOC_QUEUE_BULK_TRANSMIT) ?
 			VCHIQ_BULK_TRANSMIT : VCHIQ_BULK_RECEIVE;
 
-		if (copy_from_user(&args, (const void __user *)arg,
-				   sizeof(args))) {
+		argp = (void __user *)arg;
+		if (copy_from_user(&args, argp, sizeof(args))) {
 			ret = -EFAULT;
 			break;
 		}
 
-		service = find_service_for_instance(instance, args.handle);
-		if (!service) {
-			ret = -EINVAL;
-			break;
-		}
-
-		if (args.mode == VCHIQ_BULK_MODE_BLOCKING) {
-			waiter = kzalloc(sizeof(struct bulk_waiter_node),
-				GFP_KERNEL);
-			if (!waiter) {
-				ret = -ENOMEM;
-				break;
-			}
-
-			args.userdata = &waiter->bulk_waiter;
-		} else if (args.mode == VCHIQ_BULK_MODE_WAITING) {
-			mutex_lock(&instance->bulk_waiter_list_mutex);
-			list_for_each_entry(waiter, &instance->bulk_waiter_list,
-					    list) {
-				if (waiter->pid == current->pid) {
-					list_del(&waiter->list);
-					break;
-				}
-			}
-			mutex_unlock(&instance->bulk_waiter_list_mutex);
-			if (!waiter) {
-				vchiq_log_error(vchiq_arm_log_level,
-					"no bulk_waiter found for pid %d",
-					current->pid);
-				ret = -ESRCH;
-				break;
-			}
-			vchiq_log_info(vchiq_arm_log_level,
-				"found bulk_waiter %pK for pid %d", waiter,
-				current->pid);
-			args.userdata = &waiter->bulk_waiter;
-		}
-
-		status = vchiq_bulk_transfer(args.handle, args.data, args.size,
-					     args.userdata, args.mode, dir);
-
-		if (!waiter)
-			break;
-
-		if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
-			!waiter->bulk_waiter.bulk) {
-			if (waiter->bulk_waiter.bulk) {
-				/* Cancel the signal when the transfer
-				** completes. */
-				spin_lock(&bulk_waiter_spinlock);
-				waiter->bulk_waiter.bulk->userdata = NULL;
-				spin_unlock(&bulk_waiter_spinlock);
-			}
-			kfree(waiter);
-		} else {
-			const enum vchiq_bulk_mode mode_waiting =
-				VCHIQ_BULK_MODE_WAITING;
-			waiter->pid = current->pid;
-			mutex_lock(&instance->bulk_waiter_list_mutex);
-			list_add(&waiter->list, &instance->bulk_waiter_list);
-			mutex_unlock(&instance->bulk_waiter_list_mutex);
-			vchiq_log_info(vchiq_arm_log_level,
-				"saved bulk_waiter %pK for pid %d",
-				waiter, current->pid);
-
-			if (copy_to_user((void __user *)
-				&(((struct vchiq_queue_bulk_transfer __user *)
-					arg)->mode),
-				(const void *)&mode_waiting,
-				sizeof(mode_waiting)))
-				ret = -EFAULT;
-		}
+		ret = vchiq_irq_queue_bulk_tx_rx(instance, &args,
+						 dir, &argp->mode);
 	} break;
 
 	case VCHIQ_IOC_AWAIT_COMPLETION: {
@@ -1620,47 +1639,26 @@ struct vchiq_queue_bulk_transfer32 {
 static long
 vchiq_compat_ioctl_queue_bulk(struct file *file,
 			      unsigned int cmd,
-			      unsigned long arg)
+			      struct vchiq_queue_bulk_transfer32 __user *argp)
 {
-	struct vchiq_queue_bulk_transfer __user *args;
 	struct vchiq_queue_bulk_transfer32 args32;
-	struct vchiq_queue_bulk_transfer32 __user *ptrargs32 =
-		(struct vchiq_queue_bulk_transfer32 __user *)arg;
-	long ret;
-
-	args = compat_alloc_user_space(sizeof(*args));
-	if (!args)
-		return -EFAULT;
-
-	if (copy_from_user(&args32, ptrargs32, sizeof(args32)))
-		return -EFAULT;
-
-	if (put_user(args32.handle, &args->handle) ||
-	    put_user(compat_ptr(args32.data), &args->data) ||
-	    put_user(args32.size, &args->size) ||
-	    put_user(compat_ptr(args32.userdata), &args->userdata) ||
-	    put_user(args32.mode, &args->mode))
-		return -EFAULT;
-
-	if (cmd == VCHIQ_IOC_QUEUE_BULK_TRANSMIT32)
-		cmd = VCHIQ_IOC_QUEUE_BULK_TRANSMIT;
-	else
-		cmd = VCHIQ_IOC_QUEUE_BULK_RECEIVE;
-
-	ret = vchiq_ioctl(file, cmd, (unsigned long)args);
+	struct vchiq_queue_bulk_transfer args;
+	enum vchiq_bulk_dir dir = (cmd == VCHIQ_IOC_QUEUE_BULK_TRANSMIT) ?
+				  VCHIQ_BULK_TRANSMIT : VCHIQ_BULK_RECEIVE;
 
-	if (ret < 0)
-		return ret;
-
-	if (get_user(args32.mode, &args->mode))
+	if (copy_from_user(&args32, argp, sizeof(args32)))
 		return -EFAULT;
 
-	if (copy_to_user(&ptrargs32->mode,
-			 &args32.mode,
-			 sizeof(args32.mode)))
-		return -EFAULT;
+	args = (struct vchiq_queue_bulk_transfer) {
+		.handle   = args32.handle,
+		.data	  = compat_ptr(args32.data),
+		.size	  = args32.size,
+		.userdata = compat_ptr(args32.userdata),
+		.mode	  = args32.mode,
+	};
 
-	return 0;
+	return vchiq_irq_queue_bulk_tx_rx(file->private_data, &args,
+					  dir, &argp->mode);
 }
 
 struct vchiq_completion_data32 {
@@ -1893,7 +1891,7 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		return vchiq_compat_ioctl_queue_message(file, cmd, argp);
 	case VCHIQ_IOC_QUEUE_BULK_TRANSMIT32:
 	case VCHIQ_IOC_QUEUE_BULK_RECEIVE32:
-		return vchiq_compat_ioctl_queue_bulk(file, cmd, arg);
+		return vchiq_compat_ioctl_queue_bulk(file, cmd, argp);
 	case VCHIQ_IOC_AWAIT_COMPLETION32:
 		return vchiq_compat_ioctl_await_completion(file, cmd, arg);
 	case VCHIQ_IOC_DEQUEUE_MESSAGE32:
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/5] staging: vchiq: convert compat bulk transfer

[Arnd Bergmann]

Split out the ioctl implementation for VCHIQ_IOC_QUEUE_BULK_TRANSMIT
into a separate function so it can be shared with the compat
implementation.

Here, the input data is converted separately in the compat
handler, while the output data is passed as a __user pointer
to thec vchiq_queue_bulk_transfer->mode word that is
compatible.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 220 +++++++++---------
 1 file changed, 109 insertions(+), 111 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index cbe9583a0114..50af7f4a1b7c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -938,6 +938,95 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
 	return ret;
 }
 
+static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
+				      struct vchiq_queue_bulk_transfer *args,
+				      enum vchiq_bulk_dir dir,
+				      enum vchiq_bulk_mode __user *mode)
+{
+	struct vchiq_service *service;
+	struct bulk_waiter_node *waiter = NULL;
+	int status = 0;
+	int ret;
+
+	service = find_service_for_instance(instance, args->handle);
+	if (!service)
+		return -EINVAL;
+
+	if (args->mode == VCHIQ_BULK_MODE_BLOCKING) {
+		waiter = kzalloc(sizeof(struct bulk_waiter_node),
+			GFP_KERNEL);
+		if (!waiter) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		args->userdata = &waiter->bulk_waiter;
+	} else if (args->mode == VCHIQ_BULK_MODE_WAITING) {
+		mutex_lock(&instance->bulk_waiter_list_mutex);
+		list_for_each_entry(waiter, &instance->bulk_waiter_list,
+				    list) {
+			if (waiter->pid == current->pid) {
+				list_del(&waiter->list);
+				break;
+			}
+		}
+		mutex_unlock(&instance->bulk_waiter_list_mutex);
+		if (!waiter) {
+			vchiq_log_error(vchiq_arm_log_level,
+				"no bulk_waiter found for pid %d",
+				current->pid);
+			ret = -ESRCH;
+			goto out;
+		}
+		vchiq_log_info(vchiq_arm_log_level,
+			"found bulk_waiter %pK for pid %d", waiter,
+			current->pid);
+		args->userdata = &waiter->bulk_waiter;
+	}
+
+	status = vchiq_bulk_transfer(args->handle, args->data, args->size,
+				     args->userdata, args->mode, dir);
+
+	if (!waiter) {
+		ret = 0;
+		goto out;
+	}
+
+	if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
+		!waiter->bulk_waiter.bulk) {
+		if (waiter->bulk_waiter.bulk) {
+			/* Cancel the signal when the transfer
+			** completes. */
+			spin_lock(&bulk_waiter_spinlock);
+			waiter->bulk_waiter.bulk->userdata = NULL;
+			spin_unlock(&bulk_waiter_spinlock);
+		}
+		kfree(waiter);
+		ret = 0;
+	} else {
+		const enum vchiq_bulk_mode mode_waiting =
+			VCHIQ_BULK_MODE_WAITING;
+		waiter->pid = current->pid;
+		mutex_lock(&instance->bulk_waiter_list_mutex);
+		list_add(&waiter->list, &instance->bulk_waiter_list);
+		mutex_unlock(&instance->bulk_waiter_list_mutex);
+		vchiq_log_info(vchiq_arm_log_level,
+			"saved bulk_waiter %pK for pid %d",
+			waiter, current->pid);
+
+		ret = put_user(mode_waiting, mode);
+	}
+out:
+	unlock_service(service);
+	if (ret)
+		return ret;
+	else if (status == VCHIQ_ERROR)
+		return -EIO;
+	else if (status == VCHIQ_RETRY)
+		return -EINTR;
+	return 0;
+}
+
 /****************************************************************************
 *
 *   vchiq_ioctl
@@ -1118,90 +1207,20 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case VCHIQ_IOC_QUEUE_BULK_TRANSMIT:
 	case VCHIQ_IOC_QUEUE_BULK_RECEIVE: {
 		struct vchiq_queue_bulk_transfer args;
-		struct bulk_waiter_node *waiter = NULL;
+		struct vchiq_queue_bulk_transfer __user *argp;
 
 		enum vchiq_bulk_dir dir =
 			(cmd == VCHIQ_IOC_QUEUE_BULK_TRANSMIT) ?
 			VCHIQ_BULK_TRANSMIT : VCHIQ_BULK_RECEIVE;
 
-		if (copy_from_user(&args, (const void __user *)arg,
-				   sizeof(args))) {
+		argp = (void __user *)arg;
+		if (copy_from_user(&args, argp, sizeof(args))) {
 			ret = -EFAULT;
 			break;
 		}
 
-		service = find_service_for_instance(instance, args.handle);
-		if (!service) {
-			ret = -EINVAL;
-			break;
-		}
-
-		if (args.mode == VCHIQ_BULK_MODE_BLOCKING) {
-			waiter = kzalloc(sizeof(struct bulk_waiter_node),
-				GFP_KERNEL);
-			if (!waiter) {
-				ret = -ENOMEM;
-				break;
-			}
-
-			args.userdata = &waiter->bulk_waiter;
-		} else if (args.mode == VCHIQ_BULK_MODE_WAITING) {
-			mutex_lock(&instance->bulk_waiter_list_mutex);
-			list_for_each_entry(waiter, &instance->bulk_waiter_list,
-					    list) {
-				if (waiter->pid == current->pid) {
-					list_del(&waiter->list);
-					break;
-				}
-			}
-			mutex_unlock(&instance->bulk_waiter_list_mutex);
-			if (!waiter) {
-				vchiq_log_error(vchiq_arm_log_level,
-					"no bulk_waiter found for pid %d",
-					current->pid);
-				ret = -ESRCH;
-				break;
-			}
-			vchiq_log_info(vchiq_arm_log_level,
-				"found bulk_waiter %pK for pid %d", waiter,
-				current->pid);
-			args.userdata = &waiter->bulk_waiter;
-		}
-
-		status = vchiq_bulk_transfer(args.handle, args.data, args.size,
-					     args.userdata, args.mode, dir);
-
-		if (!waiter)
-			break;
-
-		if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
-			!waiter->bulk_waiter.bulk) {
-			if (waiter->bulk_waiter.bulk) {
-				/* Cancel the signal when the transfer
-				** completes. */
-				spin_lock(&bulk_waiter_spinlock);
-				waiter->bulk_waiter.bulk->userdata = NULL;
-				spin_unlock(&bulk_waiter_spinlock);
-			}
-			kfree(waiter);
-		} else {
-			const enum vchiq_bulk_mode mode_waiting =
-				VCHIQ_BULK_MODE_WAITING;
-			waiter->pid = current->pid;
-			mutex_lock(&instance->bulk_waiter_list_mutex);
-			list_add(&waiter->list, &instance->bulk_waiter_list);
-			mutex_unlock(&instance->bulk_waiter_list_mutex);
-			vchiq_log_info(vchiq_arm_log_level,
-				"saved bulk_waiter %pK for pid %d",
-				waiter, current->pid);
-
-			if (copy_to_user((void __user *)
-				&(((struct vchiq_queue_bulk_transfer __user *)
-					arg)->mode),
-				(const void *)&mode_waiting,
-				sizeof(mode_waiting)))
-				ret = -EFAULT;
-		}
+		ret = vchiq_irq_queue_bulk_tx_rx(instance, &args,
+						 dir, &argp->mode);
 	} break;
 
 	case VCHIQ_IOC_AWAIT_COMPLETION: {
@@ -1620,47 +1639,26 @@ struct vchiq_queue_bulk_transfer32 {
 static long
 vchiq_compat_ioctl_queue_bulk(struct file *file,
 			      unsigned int cmd,
-			      unsigned long arg)
+			      struct vchiq_queue_bulk_transfer32 __user *argp)
 {
-	struct vchiq_queue_bulk_transfer __user *args;
 	struct vchiq_queue_bulk_transfer32 args32;
-	struct vchiq_queue_bulk_transfer32 __user *ptrargs32 =
-		(struct vchiq_queue_bulk_transfer32 __user *)arg;
-	long ret;
-
-	args = compat_alloc_user_space(sizeof(*args));
-	if (!args)
-		return -EFAULT;
-
-	if (copy_from_user(&args32, ptrargs32, sizeof(args32)))
-		return -EFAULT;
-
-	if (put_user(args32.handle, &args->handle) ||
-	    put_user(compat_ptr(args32.data), &args->data) ||
-	    put_user(args32.size, &args->size) ||
-	    put_user(compat_ptr(args32.userdata), &args->userdata) ||
-	    put_user(args32.mode, &args->mode))
-		return -EFAULT;
-
-	if (cmd == VCHIQ_IOC_QUEUE_BULK_TRANSMIT32)
-		cmd = VCHIQ_IOC_QUEUE_BULK_TRANSMIT;
-	else
-		cmd = VCHIQ_IOC_QUEUE_BULK_RECEIVE;
-
-	ret = vchiq_ioctl(file, cmd, (unsigned long)args);
+	struct vchiq_queue_bulk_transfer args;
+	enum vchiq_bulk_dir dir = (cmd == VCHIQ_IOC_QUEUE_BULK_TRANSMIT) ?
+				  VCHIQ_BULK_TRANSMIT : VCHIQ_BULK_RECEIVE;
 
-	if (ret < 0)
-		return ret;
-
-	if (get_user(args32.mode, &args->mode))
+	if (copy_from_user(&args32, argp, sizeof(args32)))
 		return -EFAULT;
 
-	if (copy_to_user(&ptrargs32->mode,
-			 &args32.mode,
-			 sizeof(args32.mode)))
-		return -EFAULT;
+	args = (struct vchiq_queue_bulk_transfer) {
+		.handle   = args32.handle,
+		.data	  = compat_ptr(args32.data),
+		.size	  = args32.size,
+		.userdata = compat_ptr(args32.userdata),
+		.mode	  = args32.mode,
+	};
 
-	return 0;
+	return vchiq_irq_queue_bulk_tx_rx(file->private_data, &args,
+					  dir, &argp->mode);
 }
 
 struct vchiq_completion_data32 {
@@ -1893,7 +1891,7 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		return vchiq_compat_ioctl_queue_message(file, cmd, argp);
 	case VCHIQ_IOC_QUEUE_BULK_TRANSMIT32:
 	case VCHIQ_IOC_QUEUE_BULK_RECEIVE32:
-		return vchiq_compat_ioctl_queue_bulk(file, cmd, arg);
+		return vchiq_compat_ioctl_queue_bulk(file, cmd, argp);
 	case VCHIQ_IOC_AWAIT_COMPLETION32:
 		return vchiq_compat_ioctl_await_completion(file, cmd, arg);
 	case VCHIQ_IOC_DEQUEUE_MESSAGE32:
-- 
2.27.0

Re: [PATCH 4/5] staging: vchiq: convert compat bulk transfer

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 7985 bytes --]

Hi Arnd,

I love your patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/staging-vchiq-rework-compat-handling/20200918-185438
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 8436f932d84b1d53d2f4a2fa88c7aacdb0313265
config: arm64-randconfig-s032-20200921 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-201-g24bdaac6-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1538:50: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected int enum vchiq_status ( *callback )( ... ) @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1538:50: sparse:     expected int enum vchiq_status ( *callback )( ... )
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1538:50: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1539:50: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *userdata @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1539:50: sparse:     expected void *userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1539:50: sparse:     got void [noderef] __user *
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1654:39: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *data @@     got void [noderef] __user * @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1654:39: sparse:     expected void *data
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1654:39: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1656:39: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *userdata @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1656:39: sparse:     expected void *userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1656:39: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1714:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct vchiq_completion_data *__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1714:13: sparse:     expected struct vchiq_completion_data *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1714:13: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1717:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void **__pu_val @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1717:13: sparse:     expected void **__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1717:13: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1764:13: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct vchiq_completion_data *__pu_val @@     got struct vchiq_completion_data [noderef] __user *[assigned] completion @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1764:13: sparse:     expected struct vchiq_completion_data *__pu_val
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1764:13: sparse:     got struct vchiq_completion_data [noderef] __user *[assigned] completion
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1794:59: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got struct vchiq_header *[addressable] header @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1794:59: sparse:     expected void [noderef] __user *uptr
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1794:59: sparse:     got struct vchiq_header *[addressable] header
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1796:45: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got void *[addressable] service_userdata @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1796:45: sparse:     expected void [noderef] __user *uptr
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1796:45: sparse:     got void *[addressable] service_userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1798:45: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got void *[addressable] bulk_userdata @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1798:45: sparse:     expected void [noderef] __user *uptr
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1798:45: sparse:     got void *[addressable] bulk_userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1847:45: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *buf @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1847:45: sparse:     expected void *buf
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1847:45: sparse:     got void [noderef] __user *

# https://github.com/0day-ci/linux/commit/0e0191ba5a550b33d052c3639715f54e970f6033
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Arnd-Bergmann/staging-vchiq-rework-compat-handling/20200918-185438
git checkout 0e0191ba5a550b33d052c3639715f54e970f6033
vim +1654 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c

  1633	
  1634	#define VCHIQ_IOC_QUEUE_BULK_TRANSMIT32 \
  1635		_IOWR(VCHIQ_IOC_MAGIC, 5, struct vchiq_queue_bulk_transfer32)
  1636	#define VCHIQ_IOC_QUEUE_BULK_RECEIVE32 \
  1637		_IOWR(VCHIQ_IOC_MAGIC, 6, struct vchiq_queue_bulk_transfer32)
  1638	
  1639	static long
  1640	vchiq_compat_ioctl_queue_bulk(struct file *file,
  1641				      unsigned int cmd,
  1642				      struct vchiq_queue_bulk_transfer32 __user *argp)
  1643	{
  1644		struct vchiq_queue_bulk_transfer32 args32;
  1645		struct vchiq_queue_bulk_transfer args;
  1646		enum vchiq_bulk_dir dir = (cmd == VCHIQ_IOC_QUEUE_BULK_TRANSMIT) ?
  1647					  VCHIQ_BULK_TRANSMIT : VCHIQ_BULK_RECEIVE;
  1648	
  1649		if (copy_from_user(&args32, argp, sizeof(args32)))
  1650			return -EFAULT;
  1651	
  1652		args = (struct vchiq_queue_bulk_transfer) {
  1653			.handle   = args32.handle,
> 1654			.data	  = compat_ptr(args32.data),
  1655			.size	  = args32.size,
  1656			.userdata = compat_ptr(args32.userdata),
  1657			.mode	  = args32.mode,
  1658		};
  1659	
  1660		return vchiq_irq_queue_bulk_tx_rx(file->private_data, &args,
  1661						  dir, &argp->mode);
  1662	}
  1663	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33311 bytes --]

[PATCH 5/5] staging: vchiq: convert compat await_completion

[Arnd Bergmann]

Split out the ioctl implementation for VCHIQ_IOC_QUEUE_BULK_TRANSMIT
into a separate function so it can be shared with the compat
implementation.

This one is the trickiest conversion, as the compat implementation
is already quite different from the native one. By using a common
handler, the behavior is changed to be the same again: The
indirect __user pointer accesses are now handled through helper
functions that check for compat mode internally.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 496 ++++++++----------
 1 file changed, 205 insertions(+), 291 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 50af7f4a1b7c..bb0cc9cb96e9 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1027,6 +1027,193 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
 	return 0;
 }
 
+static inline int vchiq_get_user_ptr(void __user **buf, void __user *ubuf, int index)
+{
+	compat_uptr_t ptr32;
+	int ret;
+
+	if (in_compat_syscall()) {
+		compat_uptr_t __user *uptr = ubuf;
+		ret = get_user(ptr32, &uptr[index]);
+		*buf = compat_ptr(ptr32);
+	} else {
+		void __user *__user *uptr = ubuf;
+		ret = get_user(buf, &uptr[index]);
+	}
+	return ret;
+}
+
+struct vchiq_completion_data32 {
+	enum vchiq_reason reason;
+	compat_uptr_t header;
+	compat_uptr_t service_userdata;
+	compat_uptr_t bulk_userdata;
+};
+
+static int vchiq_put_completion(struct vchiq_completion_data __user *buf,
+				struct vchiq_completion_data *completion,
+				int index)
+{
+	struct vchiq_completion_data32 __user *buf32 = (void __user *)buf;
+
+	if (in_compat_syscall()) {
+		struct vchiq_completion_data32 tmp = {
+			.reason		  = buf->reason,
+			.header		  = ptr_to_compat(buf->header),
+			.service_userdata = ptr_to_compat(buf->service_userdata),
+			.bulk_userdata	  = ptr_to_compat(buf->bulk_userdata),
+		};
+		if (copy_to_user(&buf32[index], &tmp, sizeof(tmp)))
+			return -EFAULT;
+	} else {
+		if (copy_to_user(&buf[index], completion, sizeof(*completion)))
+			return -EFAULT;
+	}
+
+	return 0;
+}
+
+static int vchiq_ioc_await_completion(struct vchiq_instance *instance,
+				      struct vchiq_await_completion *args,
+				      int __user *msgbufcountp)
+{
+	int msgbufcount;
+	int remove;
+	int ret;
+
+	DEBUG_INITIALISE(g_state.local)
+
+	DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+	if (!instance->connected) {
+		return -ENOTCONN;
+	}
+
+	mutex_lock(&instance->completion_mutex);
+
+	DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+	while ((instance->completion_remove ==
+		instance->completion_insert)
+		&& !instance->closing) {
+		int rc;
+
+		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+		mutex_unlock(&instance->completion_mutex);
+		rc = wait_for_completion_interruptible(
+					&instance->insert_event);
+		mutex_lock(&instance->completion_mutex);
+		if (rc) {
+			DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+			vchiq_log_info(vchiq_arm_log_level,
+				"AWAIT_COMPLETION interrupted");
+			ret = -EINTR;
+			goto out;
+		}
+	}
+	DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+
+	msgbufcount = args->msgbufcount;
+	remove = instance->completion_remove;
+
+	for (ret = 0; ret < args->count; ret++) {
+		struct vchiq_completion_data *completion;
+		struct vchiq_service *service;
+		struct user_service *user_service;
+		struct vchiq_header *header;
+
+		if (remove == instance->completion_insert)
+			break;
+
+		completion = &instance->completions[
+			remove & (MAX_COMPLETIONS - 1)];
+
+		/*
+		 * A read memory barrier is needed to stop
+		 * prefetch of a stale completion record
+		 */
+		rmb();
+
+		service = completion->service_userdata;
+		user_service = service->base.userdata;
+		completion->service_userdata = user_service->userdata;
+
+		header = completion->header;
+		if (header) {
+			void __user *msgbuf;
+			int msglen;
+
+			msglen = header->size + sizeof(struct vchiq_header);
+			/* This must be a VCHIQ-style service */
+			if (args->msgbufsize < msglen) {
+				vchiq_log_error(vchiq_arm_log_level,
+					"header %pK: msgbufsize %x < msglen %x",
+					header, args->msgbufsize, msglen);
+				WARN(1, "invalid message size\n");
+				if (ret == 0)
+					ret = -EMSGSIZE;
+				break;
+			}
+			if (msgbufcount <= 0)
+				/* Stall here for lack of a
+				** buffer for the message. */
+				break;
+			/* Get the pointer from user space */
+			msgbufcount--;
+			if (vchiq_get_user_ptr(&msgbuf, &args->msgbufs,
+						msgbufcount)) {
+				if (ret == 0)
+					ret = -EFAULT;
+				break;
+			}
+
+			/* Copy the message to user space */
+			if (copy_to_user(msgbuf, header, msglen)) {
+				if (ret == 0)
+					ret = -EFAULT;
+				break;
+			}
+
+			/* Now it has been copied, the message
+			** can be released. */
+			vchiq_release_message(service->handle, header);
+
+			/* The completion must point to the
+			** msgbuf. */
+			completion->header =
+				(struct vchiq_header __force *)msgbuf;
+		}
+
+		if ((completion->reason == VCHIQ_SERVICE_CLOSED) &&
+		    !instance->use_close_delivered)
+			unlock_service(service);
+
+		if (vchiq_put_completion(args->buf, completion, ret)) {
+			if (ret == 0)
+				ret = -EFAULT;
+			break;
+		}
+
+		/*
+		 * Ensure that the above copy has completed
+		 * before advancing the remove pointer.
+		 */
+		mb();
+		remove++;
+		instance->completion_remove = remove;
+	}
+
+	if (msgbufcount != args->msgbufcount) {
+		if (put_user(msgbufcount, msgbufcountp))
+			ret = -EFAULT;
+	}
+out:
+	if (ret)
+		complete(&instance->remove_event);
+	mutex_unlock(&instance->completion_mutex);
+	DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+
+	return ret;
+}
+
 /****************************************************************************
 *
 *   vchiq_ioctl
@@ -1041,8 +1228,6 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	long ret = 0;
 	int i, rc;
 
-	DEBUG_INITIALISE(g_state.local)
-
 	vchiq_log_trace(vchiq_arm_log_level,
 		"%s - instance %pK, cmd %s, arg %lx",
 		__func__, instance,
@@ -1225,163 +1410,16 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 	case VCHIQ_IOC_AWAIT_COMPLETION: {
 		struct vchiq_await_completion args;
+		struct vchiq_await_completion __user *argp;
 
-		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-		if (!instance->connected) {
-			ret = -ENOTCONN;
-			break;
-		}
-
-		if (copy_from_user(&args, (const void __user *)arg,
-			sizeof(args))) {
+		argp = (void __user *)arg;
+		if (copy_from_user(&args, argp, sizeof(args))) {
 			ret = -EFAULT;
 			break;
 		}
 
-		mutex_lock(&instance->completion_mutex);
-
-		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-		while ((instance->completion_remove ==
-			instance->completion_insert)
-			&& !instance->closing) {
-			int rc;
-
-			DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-			mutex_unlock(&instance->completion_mutex);
-			rc = wait_for_completion_interruptible(
-						&instance->insert_event);
-			mutex_lock(&instance->completion_mutex);
-			if (rc) {
-				DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-				vchiq_log_info(vchiq_arm_log_level,
-					"AWAIT_COMPLETION interrupted");
-				ret = -EINTR;
-				break;
-			}
-		}
-		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-
-		if (ret == 0) {
-			int msgbufcount = args.msgbufcount;
-			int remove = instance->completion_remove;
-
-			for (ret = 0; ret < args.count; ret++) {
-				struct vchiq_completion_data *completion;
-				struct vchiq_service *service;
-				struct user_service *user_service;
-				struct vchiq_header *header;
-
-				if (remove == instance->completion_insert)
-					break;
-
-				completion = &instance->completions[
-					remove & (MAX_COMPLETIONS - 1)];
-
-				/*
-				 * A read memory barrier is needed to stop
-				 * prefetch of a stale completion record
-				 */
-				rmb();
-
-				service = completion->service_userdata;
-				user_service = service->base.userdata;
-				completion->service_userdata =
-					user_service->userdata;
-
-				header = completion->header;
-				if (header) {
-					void __user *msgbuf;
-					int msglen;
-
-					msglen = header->size +
-						sizeof(struct vchiq_header);
-					/* This must be a VCHIQ-style service */
-					if (args.msgbufsize < msglen) {
-						vchiq_log_error(
-							vchiq_arm_log_level,
-							"header %pK: msgbufsize %x < msglen %x",
-							header, args.msgbufsize,
-							msglen);
-						WARN(1, "invalid message "
-							"size\n");
-						if (ret == 0)
-							ret = -EMSGSIZE;
-						break;
-					}
-					if (msgbufcount <= 0)
-						/* Stall here for lack of a
-						** buffer for the message. */
-						break;
-					/* Get the pointer from user space */
-					msgbufcount--;
-					if (copy_from_user(&msgbuf,
-						(const void __user *)
-						&args.msgbufs[msgbufcount],
-						sizeof(msgbuf))) {
-						if (ret == 0)
-							ret = -EFAULT;
-						break;
-					}
-
-					/* Copy the message to user space */
-					if (copy_to_user(msgbuf, header,
-						msglen)) {
-						if (ret == 0)
-							ret = -EFAULT;
-						break;
-					}
-
-					/* Now it has been copied, the message
-					** can be released. */
-					vchiq_release_message(service->handle,
-						header);
-
-					/* The completion must point to the
-					** msgbuf. */
-					completion->header =
-						(struct vchiq_header __force *)
-						msgbuf;
-				}
-
-				if ((completion->reason ==
-					VCHIQ_SERVICE_CLOSED) &&
-					!instance->use_close_delivered)
-					unlock_service(service);
-
-				if (copy_to_user((void __user *)(
-					(size_t)args.buf + ret *
-					sizeof(struct vchiq_completion_data)),
-					completion,
-					sizeof(struct vchiq_completion_data))) {
-						if (ret == 0)
-							ret = -EFAULT;
-					break;
-				}
-
-				/*
-				 * Ensure that the above copy has completed
-				 * before advancing the remove pointer.
-				 */
-				mb();
-				remove++;
-				instance->completion_remove = remove;
-			}
-
-			if (msgbufcount != args.msgbufcount) {
-				if (copy_to_user((void __user *)
-					&((struct vchiq_await_completion *)arg)
-						->msgbufcount,
-					&msgbufcount,
-					sizeof(msgbufcount))) {
-					ret = -EFAULT;
-				}
-			}
-		}
-
-		if (ret)
-			complete(&instance->remove_event);
-		mutex_unlock(&instance->completion_mutex);
-		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+		ret = vchiq_ioc_await_completion(instance, &args,
+						 &argp->msgbufcount);
 	} break;
 
 	case VCHIQ_IOC_DEQUEUE_MESSAGE: {
@@ -1661,13 +1699,6 @@ vchiq_compat_ioctl_queue_bulk(struct file *file,
 					  dir, &argp->mode);
 }
 
-struct vchiq_completion_data32 {
-	enum vchiq_reason reason;
-	compat_uptr_t header;
-	compat_uptr_t service_userdata;
-	compat_uptr_t bulk_userdata;
-};
-
 struct vchiq_await_completion32 {
 	unsigned int count;
 	compat_uptr_t buf;
@@ -1682,141 +1713,24 @@ struct vchiq_await_completion32 {
 static long
 vchiq_compat_ioctl_await_completion(struct file *file,
 				    unsigned int cmd,
-				    unsigned long arg)
+				    struct vchiq_await_completion32 *argp)
 {
-	struct vchiq_await_completion __user *args;
-	struct vchiq_completion_data __user *completion;
-	struct vchiq_completion_data completiontemp;
+	struct vchiq_await_completion args;
 	struct vchiq_await_completion32 args32;
-	struct vchiq_completion_data32 completion32;
-	unsigned int __user *msgbufcount32;
-	unsigned int msgbufcount_native;
-	compat_uptr_t msgbuf32;
-	void __user *msgbuf;
-	void * __user *msgbufptr;
-	long ret;
-
-	args = compat_alloc_user_space(sizeof(*args) +
-				       sizeof(*completion) +
-				       sizeof(*msgbufptr));
-	if (!args)
-		return -EFAULT;
-
-	completion = (struct vchiq_completion_data __user *)(args + 1);
-	msgbufptr = (void * __user *)(completion + 1);
-
-	if (copy_from_user(&args32,
-			   (struct vchiq_completion_data32 __user *)arg,
-			   sizeof(args32)))
-		return -EFAULT;
-
-	if (put_user(args32.count, &args->count) ||
-	    put_user(compat_ptr(args32.buf), &args->buf) ||
-	    put_user(args32.msgbufsize, &args->msgbufsize) ||
-	    put_user(args32.msgbufcount, &args->msgbufcount) ||
-	    put_user(compat_ptr(args32.msgbufs), &args->msgbufs))
-		return -EFAULT;
-
-	/* These are simple cases, so just fall into the native handler */
-	if (!args32.count || !args32.buf || !args32.msgbufcount)
-		return vchiq_ioctl(file,
-				   VCHIQ_IOC_AWAIT_COMPLETION,
-				   (unsigned long)args);
-
-	/*
-	 * These are the more complex cases.  Typical applications of this
-	 * ioctl will use a very large count, with a very large msgbufcount.
-	 * Since the native ioctl can asynchronously fill in the returned
-	 * buffers and the application can in theory begin processing messages
-	 * even before the ioctl returns, a bit of a trick is used here.
-	 *
-	 * By forcing both count and msgbufcount to be 1, it forces the native
-	 * ioctl to only claim at most 1 message is available.   This tricks
-	 * the calling application into thinking only 1 message was actually
-	 * available in the queue so like all good applications it will retry
-	 * waiting until all the required messages are received.
-	 *
-	 * This trick has been tested and proven to work with vchiq_test,
-	 * Minecraft_PI, the "hello pi" examples, and various other
-	 * applications that are included in Raspbian.
-	 */
-
-	if (copy_from_user(&msgbuf32,
-			   compat_ptr(args32.msgbufs) +
-			   (sizeof(compat_uptr_t) *
-			   (args32.msgbufcount - 1)),
-			   sizeof(msgbuf32)))
-		return -EFAULT;
-
-	msgbuf = compat_ptr(msgbuf32);
 
-	if (copy_to_user(msgbufptr,
-			 &msgbuf,
-			 sizeof(msgbuf)))
-		return -EFAULT;
-
-	if (copy_to_user(&args->msgbufs,
-			 &msgbufptr,
-			 sizeof(msgbufptr)))
-		return -EFAULT;
-
-	if (put_user(1U, &args->count) ||
-	    put_user(completion, &args->buf) ||
-	    put_user(1U, &args->msgbufcount))
-		return -EFAULT;
-
-	ret = vchiq_ioctl(file,
-			  VCHIQ_IOC_AWAIT_COMPLETION,
-			  (unsigned long)args);
-
-	/*
-	 * An return value of 0 here means that no messages where available
-	 * in the message queue.  In this case the native ioctl does not
-	 * return any data to the application at all.  Not even to update
-	 * msgbufcount.  This functionality needs to be kept here for
-	 * compatibility.
-	 *
-	 * Of course, < 0 means that an error occurred and no data is being
-	 * returned.
-	 *
-	 * Since count and msgbufcount was forced to 1, that means
-	 * the only other possible return value is 1. Meaning that 1 message
-	 * was available, so that multiple message case does not need to be
-	 * handled here.
-	 */
-	if (ret <= 0)
-		return ret;
-
-	if (copy_from_user(&completiontemp, completion, sizeof(*completion)))
-		return -EFAULT;
-
-	completion32.reason = completiontemp.reason;
-	completion32.header = ptr_to_compat(completiontemp.header);
-	completion32.service_userdata =
-		ptr_to_compat(completiontemp.service_userdata);
-	completion32.bulk_userdata =
-		ptr_to_compat(completiontemp.bulk_userdata);
-
-	if (copy_to_user(compat_ptr(args32.buf),
-			 &completion32,
-			 sizeof(completion32)))
-		return -EFAULT;
-
-	if (get_user(msgbufcount_native, &args->msgbufcount))
+	if (copy_from_user(&args32, argp, sizeof(args32)))
 		return -EFAULT;
 
-	if (!msgbufcount_native)
-		args32.msgbufcount--;
-
-	msgbufcount32 =
-		&((struct vchiq_await_completion32 __user *)arg)->msgbufcount;
-
-	if (copy_to_user(msgbufcount32,
-			 &args32.msgbufcount,
-			 sizeof(args32.msgbufcount)))
-		return -EFAULT;
+	args = (struct vchiq_await_completion) {
+		.count		= args32.count,
+		.buf		= compat_ptr(args32.buf),
+		.msgbufsize	= args32.msgbufsize,
+		.msgbufcount	= args32.msgbufcount,
+		.msgbufs	= compat_ptr(args32.msgbufs),
+	};
 
-	return 1;
+	return vchiq_ioc_await_completion(file->private_data, &args,
+					  &argp->msgbufcount);
 }
 
 struct vchiq_dequeue_message32 {
@@ -1893,7 +1807,7 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case VCHIQ_IOC_QUEUE_BULK_RECEIVE32:
 		return vchiq_compat_ioctl_queue_bulk(file, cmd, argp);
 	case VCHIQ_IOC_AWAIT_COMPLETION32:
-		return vchiq_compat_ioctl_await_completion(file, cmd, arg);
+		return vchiq_compat_ioctl_await_completion(file, cmd, argp);
 	case VCHIQ_IOC_DEQUEUE_MESSAGE32:
 		return vchiq_compat_ioctl_dequeue_message(file, cmd, argp);
 	case VCHIQ_IOC_GET_CONFIG32:
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/5] staging: vchiq: convert compat await_completion

[Arnd Bergmann]

Split out the ioctl implementation for VCHIQ_IOC_QUEUE_BULK_TRANSMIT
into a separate function so it can be shared with the compat
implementation.

This one is the trickiest conversion, as the compat implementation
is already quite different from the native one. By using a common
handler, the behavior is changed to be the same again: The
indirect __user pointer accesses are now handled through helper
functions that check for compat mode internally.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 496 ++++++++----------
 1 file changed, 205 insertions(+), 291 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 50af7f4a1b7c..bb0cc9cb96e9 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1027,6 +1027,193 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
 	return 0;
 }
 
+static inline int vchiq_get_user_ptr(void __user **buf, void __user *ubuf, int index)
+{
+	compat_uptr_t ptr32;
+	int ret;
+
+	if (in_compat_syscall()) {
+		compat_uptr_t __user *uptr = ubuf;
+		ret = get_user(ptr32, &uptr[index]);
+		*buf = compat_ptr(ptr32);
+	} else {
+		void __user *__user *uptr = ubuf;
+		ret = get_user(buf, &uptr[index]);
+	}
+	return ret;
+}
+
+struct vchiq_completion_data32 {
+	enum vchiq_reason reason;
+	compat_uptr_t header;
+	compat_uptr_t service_userdata;
+	compat_uptr_t bulk_userdata;
+};
+
+static int vchiq_put_completion(struct vchiq_completion_data __user *buf,
+				struct vchiq_completion_data *completion,
+				int index)
+{
+	struct vchiq_completion_data32 __user *buf32 = (void __user *)buf;
+
+	if (in_compat_syscall()) {
+		struct vchiq_completion_data32 tmp = {
+			.reason		  = buf->reason,
+			.header		  = ptr_to_compat(buf->header),
+			.service_userdata = ptr_to_compat(buf->service_userdata),
+			.bulk_userdata	  = ptr_to_compat(buf->bulk_userdata),
+		};
+		if (copy_to_user(&buf32[index], &tmp, sizeof(tmp)))
+			return -EFAULT;
+	} else {
+		if (copy_to_user(&buf[index], completion, sizeof(*completion)))
+			return -EFAULT;
+	}
+
+	return 0;
+}
+
+static int vchiq_ioc_await_completion(struct vchiq_instance *instance,
+				      struct vchiq_await_completion *args,
+				      int __user *msgbufcountp)
+{
+	int msgbufcount;
+	int remove;
+	int ret;
+
+	DEBUG_INITIALISE(g_state.local)
+
+	DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+	if (!instance->connected) {
+		return -ENOTCONN;
+	}
+
+	mutex_lock(&instance->completion_mutex);
+
+	DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+	while ((instance->completion_remove ==
+		instance->completion_insert)
+		&& !instance->closing) {
+		int rc;
+
+		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+		mutex_unlock(&instance->completion_mutex);
+		rc = wait_for_completion_interruptible(
+					&instance->insert_event);
+		mutex_lock(&instance->completion_mutex);
+		if (rc) {
+			DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+			vchiq_log_info(vchiq_arm_log_level,
+				"AWAIT_COMPLETION interrupted");
+			ret = -EINTR;
+			goto out;
+		}
+	}
+	DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+
+	msgbufcount = args->msgbufcount;
+	remove = instance->completion_remove;
+
+	for (ret = 0; ret < args->count; ret++) {
+		struct vchiq_completion_data *completion;
+		struct vchiq_service *service;
+		struct user_service *user_service;
+		struct vchiq_header *header;
+
+		if (remove == instance->completion_insert)
+			break;
+
+		completion = &instance->completions[
+			remove & (MAX_COMPLETIONS - 1)];
+
+		/*
+		 * A read memory barrier is needed to stop
+		 * prefetch of a stale completion record
+		 */
+		rmb();
+
+		service = completion->service_userdata;
+		user_service = service->base.userdata;
+		completion->service_userdata = user_service->userdata;
+
+		header = completion->header;
+		if (header) {
+			void __user *msgbuf;
+			int msglen;
+
+			msglen = header->size + sizeof(struct vchiq_header);
+			/* This must be a VCHIQ-style service */
+			if (args->msgbufsize < msglen) {
+				vchiq_log_error(vchiq_arm_log_level,
+					"header %pK: msgbufsize %x < msglen %x",
+					header, args->msgbufsize, msglen);
+				WARN(1, "invalid message size\n");
+				if (ret == 0)
+					ret = -EMSGSIZE;
+				break;
+			}
+			if (msgbufcount <= 0)
+				/* Stall here for lack of a
+				** buffer for the message. */
+				break;
+			/* Get the pointer from user space */
+			msgbufcount--;
+			if (vchiq_get_user_ptr(&msgbuf, &args->msgbufs,
+						msgbufcount)) {
+				if (ret == 0)
+					ret = -EFAULT;
+				break;
+			}
+
+			/* Copy the message to user space */
+			if (copy_to_user(msgbuf, header, msglen)) {
+				if (ret == 0)
+					ret = -EFAULT;
+				break;
+			}
+
+			/* Now it has been copied, the message
+			** can be released. */
+			vchiq_release_message(service->handle, header);
+
+			/* The completion must point to the
+			** msgbuf. */
+			completion->header =
+				(struct vchiq_header __force *)msgbuf;
+		}
+
+		if ((completion->reason == VCHIQ_SERVICE_CLOSED) &&
+		    !instance->use_close_delivered)
+			unlock_service(service);
+
+		if (vchiq_put_completion(args->buf, completion, ret)) {
+			if (ret == 0)
+				ret = -EFAULT;
+			break;
+		}
+
+		/*
+		 * Ensure that the above copy has completed
+		 * before advancing the remove pointer.
+		 */
+		mb();
+		remove++;
+		instance->completion_remove = remove;
+	}
+
+	if (msgbufcount != args->msgbufcount) {
+		if (put_user(msgbufcount, msgbufcountp))
+			ret = -EFAULT;
+	}
+out:
+	if (ret)
+		complete(&instance->remove_event);
+	mutex_unlock(&instance->completion_mutex);
+	DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+
+	return ret;
+}
+
 /****************************************************************************
 *
 *   vchiq_ioctl
@@ -1041,8 +1228,6 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	long ret = 0;
 	int i, rc;
 
-	DEBUG_INITIALISE(g_state.local)
-
 	vchiq_log_trace(vchiq_arm_log_level,
 		"%s - instance %pK, cmd %s, arg %lx",
 		__func__, instance,
@@ -1225,163 +1410,16 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 	case VCHIQ_IOC_AWAIT_COMPLETION: {
 		struct vchiq_await_completion args;
+		struct vchiq_await_completion __user *argp;
 
-		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-		if (!instance->connected) {
-			ret = -ENOTCONN;
-			break;
-		}
-
-		if (copy_from_user(&args, (const void __user *)arg,
-			sizeof(args))) {
+		argp = (void __user *)arg;
+		if (copy_from_user(&args, argp, sizeof(args))) {
 			ret = -EFAULT;
 			break;
 		}
 
-		mutex_lock(&instance->completion_mutex);
-
-		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-		while ((instance->completion_remove ==
-			instance->completion_insert)
-			&& !instance->closing) {
-			int rc;
-
-			DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-			mutex_unlock(&instance->completion_mutex);
-			rc = wait_for_completion_interruptible(
-						&instance->insert_event);
-			mutex_lock(&instance->completion_mutex);
-			if (rc) {
-				DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-				vchiq_log_info(vchiq_arm_log_level,
-					"AWAIT_COMPLETION interrupted");
-				ret = -EINTR;
-				break;
-			}
-		}
-		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-
-		if (ret == 0) {
-			int msgbufcount = args.msgbufcount;
-			int remove = instance->completion_remove;
-
-			for (ret = 0; ret < args.count; ret++) {
-				struct vchiq_completion_data *completion;
-				struct vchiq_service *service;
-				struct user_service *user_service;
-				struct vchiq_header *header;
-
-				if (remove == instance->completion_insert)
-					break;
-
-				completion = &instance->completions[
-					remove & (MAX_COMPLETIONS - 1)];
-
-				/*
-				 * A read memory barrier is needed to stop
-				 * prefetch of a stale completion record
-				 */
-				rmb();
-
-				service = completion->service_userdata;
-				user_service = service->base.userdata;
-				completion->service_userdata =
-					user_service->userdata;
-
-				header = completion->header;
-				if (header) {
-					void __user *msgbuf;
-					int msglen;
-
-					msglen = header->size +
-						sizeof(struct vchiq_header);
-					/* This must be a VCHIQ-style service */
-					if (args.msgbufsize < msglen) {
-						vchiq_log_error(
-							vchiq_arm_log_level,
-							"header %pK: msgbufsize %x < msglen %x",
-							header, args.msgbufsize,
-							msglen);
-						WARN(1, "invalid message "
-							"size\n");
-						if (ret == 0)
-							ret = -EMSGSIZE;
-						break;
-					}
-					if (msgbufcount <= 0)
-						/* Stall here for lack of a
-						** buffer for the message. */
-						break;
-					/* Get the pointer from user space */
-					msgbufcount--;
-					if (copy_from_user(&msgbuf,
-						(const void __user *)
-						&args.msgbufs[msgbufcount],
-						sizeof(msgbuf))) {
-						if (ret == 0)
-							ret = -EFAULT;
-						break;
-					}
-
-					/* Copy the message to user space */
-					if (copy_to_user(msgbuf, header,
-						msglen)) {
-						if (ret == 0)
-							ret = -EFAULT;
-						break;
-					}
-
-					/* Now it has been copied, the message
-					** can be released. */
-					vchiq_release_message(service->handle,
-						header);
-
-					/* The completion must point to the
-					** msgbuf. */
-					completion->header =
-						(struct vchiq_header __force *)
-						msgbuf;
-				}
-
-				if ((completion->reason ==
-					VCHIQ_SERVICE_CLOSED) &&
-					!instance->use_close_delivered)
-					unlock_service(service);
-
-				if (copy_to_user((void __user *)(
-					(size_t)args.buf + ret *
-					sizeof(struct vchiq_completion_data)),
-					completion,
-					sizeof(struct vchiq_completion_data))) {
-						if (ret == 0)
-							ret = -EFAULT;
-					break;
-				}
-
-				/*
-				 * Ensure that the above copy has completed
-				 * before advancing the remove pointer.
-				 */
-				mb();
-				remove++;
-				instance->completion_remove = remove;
-			}
-
-			if (msgbufcount != args.msgbufcount) {
-				if (copy_to_user((void __user *)
-					&((struct vchiq_await_completion *)arg)
-						->msgbufcount,
-					&msgbufcount,
-					sizeof(msgbufcount))) {
-					ret = -EFAULT;
-				}
-			}
-		}
-
-		if (ret)
-			complete(&instance->remove_event);
-		mutex_unlock(&instance->completion_mutex);
-		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
+		ret = vchiq_ioc_await_completion(instance, &args,
+						 &argp->msgbufcount);
 	} break;
 
 	case VCHIQ_IOC_DEQUEUE_MESSAGE: {
@@ -1661,13 +1699,6 @@ vchiq_compat_ioctl_queue_bulk(struct file *file,
 					  dir, &argp->mode);
 }
 
-struct vchiq_completion_data32 {
-	enum vchiq_reason reason;
-	compat_uptr_t header;
-	compat_uptr_t service_userdata;
-	compat_uptr_t bulk_userdata;
-};
-
 struct vchiq_await_completion32 {
 	unsigned int count;
 	compat_uptr_t buf;
@@ -1682,141 +1713,24 @@ struct vchiq_await_completion32 {
 static long
 vchiq_compat_ioctl_await_completion(struct file *file,
 				    unsigned int cmd,
-				    unsigned long arg)
+				    struct vchiq_await_completion32 *argp)
 {
-	struct vchiq_await_completion __user *args;
-	struct vchiq_completion_data __user *completion;
-	struct vchiq_completion_data completiontemp;
+	struct vchiq_await_completion args;
 	struct vchiq_await_completion32 args32;
-	struct vchiq_completion_data32 completion32;
-	unsigned int __user *msgbufcount32;
-	unsigned int msgbufcount_native;
-	compat_uptr_t msgbuf32;
-	void __user *msgbuf;
-	void * __user *msgbufptr;
-	long ret;
-
-	args = compat_alloc_user_space(sizeof(*args) +
-				       sizeof(*completion) +
-				       sizeof(*msgbufptr));
-	if (!args)
-		return -EFAULT;
-
-	completion = (struct vchiq_completion_data __user *)(args + 1);
-	msgbufptr = (void * __user *)(completion + 1);
-
-	if (copy_from_user(&args32,
-			   (struct vchiq_completion_data32 __user *)arg,
-			   sizeof(args32)))
-		return -EFAULT;
-
-	if (put_user(args32.count, &args->count) ||
-	    put_user(compat_ptr(args32.buf), &args->buf) ||
-	    put_user(args32.msgbufsize, &args->msgbufsize) ||
-	    put_user(args32.msgbufcount, &args->msgbufcount) ||
-	    put_user(compat_ptr(args32.msgbufs), &args->msgbufs))
-		return -EFAULT;
-
-	/* These are simple cases, so just fall into the native handler */
-	if (!args32.count || !args32.buf || !args32.msgbufcount)
-		return vchiq_ioctl(file,
-				   VCHIQ_IOC_AWAIT_COMPLETION,
-				   (unsigned long)args);
-
-	/*
-	 * These are the more complex cases.  Typical applications of this
-	 * ioctl will use a very large count, with a very large msgbufcount.
-	 * Since the native ioctl can asynchronously fill in the returned
-	 * buffers and the application can in theory begin processing messages
-	 * even before the ioctl returns, a bit of a trick is used here.
-	 *
-	 * By forcing both count and msgbufcount to be 1, it forces the native
-	 * ioctl to only claim at most 1 message is available.   This tricks
-	 * the calling application into thinking only 1 message was actually
-	 * available in the queue so like all good applications it will retry
-	 * waiting until all the required messages are received.
-	 *
-	 * This trick has been tested and proven to work with vchiq_test,
-	 * Minecraft_PI, the "hello pi" examples, and various other
-	 * applications that are included in Raspbian.
-	 */
-
-	if (copy_from_user(&msgbuf32,
-			   compat_ptr(args32.msgbufs) +
-			   (sizeof(compat_uptr_t) *
-			   (args32.msgbufcount - 1)),
-			   sizeof(msgbuf32)))
-		return -EFAULT;
-
-	msgbuf = compat_ptr(msgbuf32);
 
-	if (copy_to_user(msgbufptr,
-			 &msgbuf,
-			 sizeof(msgbuf)))
-		return -EFAULT;
-
-	if (copy_to_user(&args->msgbufs,
-			 &msgbufptr,
-			 sizeof(msgbufptr)))
-		return -EFAULT;
-
-	if (put_user(1U, &args->count) ||
-	    put_user(completion, &args->buf) ||
-	    put_user(1U, &args->msgbufcount))
-		return -EFAULT;
-
-	ret = vchiq_ioctl(file,
-			  VCHIQ_IOC_AWAIT_COMPLETION,
-			  (unsigned long)args);
-
-	/*
-	 * An return value of 0 here means that no messages where available
-	 * in the message queue.  In this case the native ioctl does not
-	 * return any data to the application at all.  Not even to update
-	 * msgbufcount.  This functionality needs to be kept here for
-	 * compatibility.
-	 *
-	 * Of course, < 0 means that an error occurred and no data is being
-	 * returned.
-	 *
-	 * Since count and msgbufcount was forced to 1, that means
-	 * the only other possible return value is 1. Meaning that 1 message
-	 * was available, so that multiple message case does not need to be
-	 * handled here.
-	 */
-	if (ret <= 0)
-		return ret;
-
-	if (copy_from_user(&completiontemp, completion, sizeof(*completion)))
-		return -EFAULT;
-
-	completion32.reason = completiontemp.reason;
-	completion32.header = ptr_to_compat(completiontemp.header);
-	completion32.service_userdata =
-		ptr_to_compat(completiontemp.service_userdata);
-	completion32.bulk_userdata =
-		ptr_to_compat(completiontemp.bulk_userdata);
-
-	if (copy_to_user(compat_ptr(args32.buf),
-			 &completion32,
-			 sizeof(completion32)))
-		return -EFAULT;
-
-	if (get_user(msgbufcount_native, &args->msgbufcount))
+	if (copy_from_user(&args32, argp, sizeof(args32)))
 		return -EFAULT;
 
-	if (!msgbufcount_native)
-		args32.msgbufcount--;
-
-	msgbufcount32 =
-		&((struct vchiq_await_completion32 __user *)arg)->msgbufcount;
-
-	if (copy_to_user(msgbufcount32,
-			 &args32.msgbufcount,
-			 sizeof(args32.msgbufcount)))
-		return -EFAULT;
+	args = (struct vchiq_await_completion) {
+		.count		= args32.count,
+		.buf		= compat_ptr(args32.buf),
+		.msgbufsize	= args32.msgbufsize,
+		.msgbufcount	= args32.msgbufcount,
+		.msgbufs	= compat_ptr(args32.msgbufs),
+	};
 
-	return 1;
+	return vchiq_ioc_await_completion(file->private_data, &args,
+					  &argp->msgbufcount);
 }
 
 struct vchiq_dequeue_message32 {
@@ -1893,7 +1807,7 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case VCHIQ_IOC_QUEUE_BULK_RECEIVE32:
 		return vchiq_compat_ioctl_queue_bulk(file, cmd, argp);
 	case VCHIQ_IOC_AWAIT_COMPLETION32:
-		return vchiq_compat_ioctl_await_completion(file, cmd, arg);
+		return vchiq_compat_ioctl_await_completion(file, cmd, argp);
 	case VCHIQ_IOC_DEQUEUE_MESSAGE32:
 		return vchiq_compat_ioctl_dequeue_message(file, cmd, argp);
 	case VCHIQ_IOC_GET_CONFIG32:
-- 
2.27.0

Re: [PATCH 5/5] staging: vchiq: convert compat await_completion

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 16010 bytes --]

Hi Arnd,

I love your patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/staging-vchiq-rework-compat-handling/20200918-185438
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 8436f932d84b1d53d2f4a2fa88c7aacdb0313265
config: arm64-randconfig-s032-20200921 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-201-g24bdaac6-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1062:62: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got struct vchiq_header *[noderef] __user header @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1062:62: sparse:     expected void [noderef] __user *uptr
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1062:62: sparse:     got struct vchiq_header *[noderef] __user header
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1063:62: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got void *[noderef] __user service_userdata @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1063:62: sparse:     expected void [noderef] __user *uptr
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1063:62: sparse:     got void *[noderef] __user service_userdata
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1064:62: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void [noderef] __user *uptr @@     got void *[noderef] __user bulk_userdata @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1064:62: sparse:     expected void [noderef] __user *uptr
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1064:62: sparse:     got void *[noderef] __user bulk_userdata
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1161:58: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void [noderef] __user *ubuf @@     got void *** @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1161:58: sparse:     expected void [noderef] __user *ubuf
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1161:58: sparse:     got void ***
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1189:46: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct vchiq_completion_data [noderef] __user *buf @@     got struct vchiq_completion_data *buf @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1189:46: sparse:     expected struct vchiq_completion_data [noderef] __user *buf
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1189:46: sparse:     got struct vchiq_completion_data *buf
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1576:50: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected int enum vchiq_status ( *callback )( ... ) @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1576:50: sparse:     expected int enum vchiq_status ( *callback )( ... )
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1576:50: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1577:50: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *userdata @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1577:50: sparse:     expected void *userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1577:50: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1692:39: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *data @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1692:39: sparse:     expected void *data
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1692:39: sparse:     got void [noderef] __user *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1694:39: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *userdata @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1694:39: sparse:     expected void *userdata
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1694:39: sparse:     got void [noderef] __user *
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1721:37: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void const [noderef] __user *from @@     got struct vchiq_await_completion32 *argp @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1721:37: sparse:     expected void const [noderef] __user *from
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1721:37: sparse:     got struct vchiq_await_completion32 *argp
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1726:45: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct vchiq_completion_data *buf @@     got void [noderef] __user * @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1726:45: sparse:     expected struct vchiq_completion_data *buf
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1726:45: sparse:     got void [noderef] __user *
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1729:45: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void **msgbufs @@     got void [noderef] __user * @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1729:45: sparse:     expected void **msgbufs
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1729:45: sparse:     got void [noderef] __user *
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1733:44: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected int [noderef] __user *msgbufcountp @@     got unsigned int * @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1733:44: sparse:     expected int [noderef] __user *msgbufcountp
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1733:44: sparse:     got unsigned int *
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1761:45: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void *buf @@     got void [noderef] __user * @@
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1761:45: sparse:     expected void *buf
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1761:45: sparse:     got void [noderef] __user *
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1810:71: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected struct vchiq_await_completion32 *argp @@     got void [noderef] __user *argp @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1810:71: sparse:     expected struct vchiq_await_completion32 *argp
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1810:71: sparse:     got void [noderef] __user *argp
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1061:45: sparse: sparse: dereference of noderef expression
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1062:59: sparse: sparse: dereference of noderef expression
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1062:59: sparse: sparse: dereference of noderef expression
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1063:59: sparse: sparse: dereference of noderef expression
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1063:59: sparse: sparse: dereference of noderef expression
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1064:59: sparse: sparse: dereference of noderef expression
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1064:59: sparse: sparse: dereference of noderef expression
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1041:23: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void [noderef] __user **buf @@     got void [noderef] __user * @@
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1041:23: sparse:     expected void [noderef] __user **buf
   drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1041:23: sparse:     got void [noderef] __user *

# https://github.com/0day-ci/linux/commit/48ebebc07ae630cd52c464744a288cea110d8bcb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Arnd-Bergmann/staging-vchiq-rework-compat-handling/20200918-185438
git checkout 48ebebc07ae630cd52c464744a288cea110d8bcb
vim +1062 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c

  1029	
  1030	static inline int vchiq_get_user_ptr(void __user **buf, void __user *ubuf, int index)
  1031	{
  1032		compat_uptr_t ptr32;
  1033		int ret;
  1034	
  1035		if (in_compat_syscall()) {
  1036			compat_uptr_t __user *uptr = ubuf;
  1037			ret = get_user(ptr32, &uptr[index]);
  1038			*buf = compat_ptr(ptr32);
  1039		} else {
  1040			void __user *__user *uptr = ubuf;
> 1041			ret = get_user(buf, &uptr[index]);
  1042		}
  1043		return ret;
  1044	}
  1045	
  1046	struct vchiq_completion_data32 {
  1047		enum vchiq_reason reason;
  1048		compat_uptr_t header;
  1049		compat_uptr_t service_userdata;
  1050		compat_uptr_t bulk_userdata;
  1051	};
  1052	
  1053	static int vchiq_put_completion(struct vchiq_completion_data __user *buf,
  1054					struct vchiq_completion_data *completion,
  1055					int index)
  1056	{
  1057		struct vchiq_completion_data32 __user *buf32 = (void __user *)buf;
  1058	
  1059		if (in_compat_syscall()) {
  1060			struct vchiq_completion_data32 tmp = {
> 1061				.reason		  = buf->reason,
> 1062				.header		  = ptr_to_compat(buf->header),
> 1063				.service_userdata = ptr_to_compat(buf->service_userdata),
> 1064				.bulk_userdata	  = ptr_to_compat(buf->bulk_userdata),
  1065			};
  1066			if (copy_to_user(&buf32[index], &tmp, sizeof(tmp)))
  1067				return -EFAULT;
  1068		} else {
  1069			if (copy_to_user(&buf[index], completion, sizeof(*completion)))
  1070				return -EFAULT;
  1071		}
  1072	
  1073		return 0;
  1074	}
  1075	
  1076	static int vchiq_ioc_await_completion(struct vchiq_instance *instance,
  1077					      struct vchiq_await_completion *args,
  1078					      int __user *msgbufcountp)
  1079	{
  1080		int msgbufcount;
  1081		int remove;
  1082		int ret;
  1083	
  1084		DEBUG_INITIALISE(g_state.local)
  1085	
  1086		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
  1087		if (!instance->connected) {
  1088			return -ENOTCONN;
  1089		}
  1090	
  1091		mutex_lock(&instance->completion_mutex);
  1092	
  1093		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
  1094		while ((instance->completion_remove ==
  1095			instance->completion_insert)
  1096			&& !instance->closing) {
  1097			int rc;
  1098	
  1099			DEBUG_TRACE(AWAIT_COMPLETION_LINE);
  1100			mutex_unlock(&instance->completion_mutex);
  1101			rc = wait_for_completion_interruptible(
  1102						&instance->insert_event);
  1103			mutex_lock(&instance->completion_mutex);
  1104			if (rc) {
  1105				DEBUG_TRACE(AWAIT_COMPLETION_LINE);
  1106				vchiq_log_info(vchiq_arm_log_level,
  1107					"AWAIT_COMPLETION interrupted");
  1108				ret = -EINTR;
  1109				goto out;
  1110			}
  1111		}
  1112		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
  1113	
  1114		msgbufcount = args->msgbufcount;
  1115		remove = instance->completion_remove;
  1116	
  1117		for (ret = 0; ret < args->count; ret++) {
  1118			struct vchiq_completion_data *completion;
  1119			struct vchiq_service *service;
  1120			struct user_service *user_service;
  1121			struct vchiq_header *header;
  1122	
  1123			if (remove == instance->completion_insert)
  1124				break;
  1125	
  1126			completion = &instance->completions[
  1127				remove & (MAX_COMPLETIONS - 1)];
  1128	
  1129			/*
  1130			 * A read memory barrier is needed to stop
  1131			 * prefetch of a stale completion record
  1132			 */
  1133			rmb();
  1134	
  1135			service = completion->service_userdata;
  1136			user_service = service->base.userdata;
  1137			completion->service_userdata = user_service->userdata;
  1138	
  1139			header = completion->header;
  1140			if (header) {
  1141				void __user *msgbuf;
  1142				int msglen;
  1143	
  1144				msglen = header->size + sizeof(struct vchiq_header);
  1145				/* This must be a VCHIQ-style service */
  1146				if (args->msgbufsize < msglen) {
  1147					vchiq_log_error(vchiq_arm_log_level,
  1148						"header %pK: msgbufsize %x < msglen %x",
  1149						header, args->msgbufsize, msglen);
  1150					WARN(1, "invalid message size\n");
  1151					if (ret == 0)
  1152						ret = -EMSGSIZE;
  1153					break;
  1154				}
  1155				if (msgbufcount <= 0)
  1156					/* Stall here for lack of a
  1157					** buffer for the message. */
  1158					break;
  1159				/* Get the pointer from user space */
  1160				msgbufcount--;
> 1161				if (vchiq_get_user_ptr(&msgbuf, &args->msgbufs,
  1162							msgbufcount)) {
  1163					if (ret == 0)
  1164						ret = -EFAULT;
  1165					break;
  1166				}
  1167	
  1168				/* Copy the message to user space */
  1169				if (copy_to_user(msgbuf, header, msglen)) {
  1170					if (ret == 0)
  1171						ret = -EFAULT;
  1172					break;
  1173				}
  1174	
  1175				/* Now it has been copied, the message
  1176				** can be released. */
  1177				vchiq_release_message(service->handle, header);
  1178	
  1179				/* The completion must point to the
  1180				** msgbuf. */
  1181				completion->header =
  1182					(struct vchiq_header __force *)msgbuf;
  1183			}
  1184	
  1185			if ((completion->reason == VCHIQ_SERVICE_CLOSED) &&
  1186			    !instance->use_close_delivered)
  1187				unlock_service(service);
  1188	
> 1189			if (vchiq_put_completion(args->buf, completion, ret)) {
  1190				if (ret == 0)
  1191					ret = -EFAULT;
  1192				break;
  1193			}
  1194	
  1195			/*
  1196			 * Ensure that the above copy has completed
  1197			 * before advancing the remove pointer.
  1198			 */
  1199			mb();
  1200			remove++;
  1201			instance->completion_remove = remove;
  1202		}
  1203	
  1204		if (msgbufcount != args->msgbufcount) {
  1205			if (put_user(msgbufcount, msgbufcountp))
  1206				ret = -EFAULT;
  1207		}
  1208	out:
  1209		if (ret)
  1210			complete(&instance->remove_event);
  1211		mutex_unlock(&instance->completion_mutex);
  1212		DEBUG_TRACE(AWAIT_COMPLETION_LINE);
  1213	
  1214		return ret;
  1215	}
  1216	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33311 bytes --]