On 2018/7/6 4:56, Jann Horn wrote:
On Thu, Jul 5, 2018 at 10:53 PM <[email protected]> wrote:
From: Xiubo Li <[email protected]>

For the target_core_user use case, after the device is unregistered
it maybe still opened in user space, then the kernel will crash, like:

[...]
Signed-off-by: Xiubo Li <[email protected]>
---
  drivers/uio/uio.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++--------
  1 file changed, 86 insertions(+), 15 deletions(-)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 33c3bfe..2b9268a 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
[...]
@@ -720,30 +775,46 @@ static int uio_mmap(struct file *filep, struct 
vm_area_struct *vma)

         vma->vm_private_data = idev;

+       mutex_lock(&idev->info_lock);
+       if (!idev->info) {
+               ret = -EINVAL;
+               goto out;
+       }
+
         mi = uio_find_mem_index(vma);
-       if (mi < 0)
-               return -EINVAL;
+       if (mi < 0) {
+               ret = -EINVAL;
+               goto out;
+       }

         requested_pages = vma_pages(vma);
         actual_pages = ((idev->info->mem[mi].addr & ~PAGE_MASK)
                         + idev->info->mem[mi].size + PAGE_SIZE -1) >> 
PAGE_SHIFT;
-       if (requested_pages > actual_pages)
-               return -EINVAL;
+       if (requested_pages > actual_pages) {
+               ret = -EINVAL;
+               goto out;
+       }

         if (idev->info->mmap) {
                 ret = idev->info->mmap(idev->info, vma);
-               return ret;
+               goto out;
         }

         switch (idev->info->mem[mi].memtype) {
                 case UIO_MEM_PHYS:
-                       return uio_mmap_physical(vma);
+                       ret = uio_mmap_physical(vma);
+                       break;
                 case UIO_MEM_LOGICAL:
                 case UIO_MEM_VIRTUAL:
-                       return uio_mmap_logical(vma);
+                       ret = uio_mmap_logical(vma);
+                       break;
                 default:
-                       return -EINVAL;
+                       ret = -EINVAL;
         }
+
+out:
+       mutex_lock(&idev->info_lock);
This is probably supposed to be mutex_unlock(...)?

Yeah yeah, right, Good catch :-)

Locally I had fixed this, but after my building and testing just forgot to amend it.

Will fix it.

Thanks very much.

BRs

Reply via email to