On 04/01/2021 18:31, Dan Carpenter wrote:
On Mon, Jan 04, 2021 at 12:09:27PM +0000, Phil Elwell wrote:The addition of the local 'userdata' pointer to vchiq_irq_queue_bulk_tx_rx omitted the case where neither BLOCKING nor WAITING modes are used, in which case the value provided by the caller is replaced with a NULL. Fixes: 4184da4f316a ("staging: vchiq: fix __user annotations") Signed-off-by: Phil Elwell <phil@xxxxxxxxxxxxxxx> --- drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 f500a7043805..2a8883673ba1 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -958,7 +958,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance, struct vchiq_service *service; struct bulk_waiter_node *waiter = NULL; bool found = false; - void *userdata = NULL; + void *userdata; int status = 0; int ret;@@ -997,6 +997,8 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,"found bulk_waiter %pK for pid %d", waiter, current->pid); userdata = &waiter->bulk_waiter; + } else { + userdata = args->userdata;"args->userdata" is marked as a user pointer so we really don't want to mix user and kernel pointers here. Presumably this opens up a large security hole.
It's an opaque, pointer-sized token that only exists to bereturned to userspace (or not, without this patch) - it's hard to see that as a security hole. Phil _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
