lkml.org 
[lkml]   [2019]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 180/267] usb: mon: Fix a deadlock in usbmon between mmap and read
    Date
    From: Pete Zaitcev <zaitcev@redhat.com>

    commit 19e6317d24c25ee737c65d1ffb7483bdda4bb54a upstream.

    The problem arises because our read() function grabs a lock of the
    circular buffer, finds something of interest, then invokes copy_to_user()
    straight from the buffer, which in turn takes mm->mmap_sem. In the same
    time, the callback mon_bin_vma_fault() is invoked under mm->mmap_sem.
    It attempts to take the fetch lock and deadlocks.

    This patch does away with protecting of our page list with any
    semaphores, and instead relies on the kernel not close the device
    while mmap is active in a process.

    In addition, we prohibit re-sizing of a buffer while mmap is active.
    This way, when (now unlocked) fault is processed, it works with the
    page that is intended to be mapped-in, and not some other random page.
    Note that this may have an ABI impact, but hopefully no legitimate
    program is this wrong.

    Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
    Reported-by: syzbot+56f9673bb4cdcbeb0e92@syzkaller.appspotmail.com
    Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
    Fixes: 46eb14a6e158 ("USB: fix usbmon BUG trigger")
    Cc: <stable@vger.kernel.org>
    Link: https://lore.kernel.org/r/20191204203941.3503452b@suzdal.zaitcev.lan
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/usb/mon/mon_bin.c | 32 +++++++++++++++++++++-----------
    1 file changed, 21 insertions(+), 11 deletions(-)

    --- a/drivers/usb/mon/mon_bin.c
    +++ b/drivers/usb/mon/mon_bin.c
    @@ -1038,12 +1038,18 @@ static long mon_bin_ioctl(struct file *f

    mutex_lock(&rp->fetch_lock);
    spin_lock_irqsave(&rp->b_lock, flags);
    - mon_free_buff(rp->b_vec, rp->b_size/CHUNK_SIZE);
    - kfree(rp->b_vec);
    - rp->b_vec = vec;
    - rp->b_size = size;
    - rp->b_read = rp->b_in = rp->b_out = rp->b_cnt = 0;
    - rp->cnt_lost = 0;
    + if (rp->mmap_active) {
    + mon_free_buff(vec, size/CHUNK_SIZE);
    + kfree(vec);
    + ret = -EBUSY;
    + } else {
    + mon_free_buff(rp->b_vec, rp->b_size/CHUNK_SIZE);
    + kfree(rp->b_vec);
    + rp->b_vec = vec;
    + rp->b_size = size;
    + rp->b_read = rp->b_in = rp->b_out = rp->b_cnt = 0;
    + rp->cnt_lost = 0;
    + }
    spin_unlock_irqrestore(&rp->b_lock, flags);
    mutex_unlock(&rp->fetch_lock);
    }
    @@ -1215,13 +1221,21 @@ mon_bin_poll(struct file *file, struct p
    static void mon_bin_vma_open(struct vm_area_struct *vma)
    {
    struct mon_reader_bin *rp = vma->vm_private_data;
    + unsigned long flags;
    +
    + spin_lock_irqsave(&rp->b_lock, flags);
    rp->mmap_active++;
    + spin_unlock_irqrestore(&rp->b_lock, flags);
    }

    static void mon_bin_vma_close(struct vm_area_struct *vma)
    {
    + unsigned long flags;
    +
    struct mon_reader_bin *rp = vma->vm_private_data;
    + spin_lock_irqsave(&rp->b_lock, flags);
    rp->mmap_active--;
    + spin_unlock_irqrestore(&rp->b_lock, flags);
    }

    /*
    @@ -1233,16 +1247,12 @@ static int mon_bin_vma_fault(struct vm_f
    unsigned long offset, chunk_idx;
    struct page *pageptr;

    - mutex_lock(&rp->fetch_lock);
    offset = vmf->pgoff << PAGE_SHIFT;
    - if (offset >= rp->b_size) {
    - mutex_unlock(&rp->fetch_lock);
    + if (offset >= rp->b_size)
    return VM_FAULT_SIGBUS;
    - }
    chunk_idx = offset / CHUNK_SIZE;
    pageptr = rp->b_vec[chunk_idx].pg;
    get_page(pageptr);
    - mutex_unlock(&rp->fetch_lock);
    vmf->page = pageptr;
    return 0;
    }

    \
     
     \ /
      Last update: 2019-12-16 18:58    [W:2.481 / U:0.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site