Received: by 2002:ac0:a591:0:0:0:0:0 with SMTP id m17-v6csp256285imm; Fri, 6 Jul 2018 19:07:14 -0700 (PDT) X-Google-Smtp-Source: AAOMgpdnjF/IFYc3GWkG9lYGt2TA0IaGzfSJ6gKCvirvJ4mR2iQs83nAmv1MNgvTycp2vr+lMVqt X-Received: by 2002:a17:902:8c95:: with SMTP id t21-v6mr3278083plo.306.1530929234557; Fri, 06 Jul 2018 19:07:14 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1530929234; cv=none; d=google.com; s=arc-20160816; b=S5HvSvjI/Z+XlEcmqOFt/0gm0lHiWpF4xdWkIvl/TznUWYIT7b4QhzpHKcBRtY62I/ UL7rIDFDUmDFgqqK4okrv6TtYJPBbq6XJ+5ZwDfe3XihbbddATYJ/uS2Xsh0eUs4aTiN DLrm4t+ap0s3R4mDuWxkf83EYcD6l7yk/7uUq33ilRUPffH4WP65yfcib8EHl8n/QwjP uNHRHjtEHIrbf8Gl4kNYuiwQDHXwb2NAyE/Jg5wAD1hn280Fs46YVPo4Akw8yRLiXpRw 4Qa5xRG/gs6J2N72XNNv5w9OzdA+TkPcTCl5jVuJjrsEE16MTd7onLYayVCb1VDD0mwo kAJg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from:arc-authentication-results; bh=bnUA75ipc/ffYJm3yJPnXgRWy9Lg6jnSqTkTfZ86tfw=; b=jgR4JOKI/Yh3mJSlxL7loEwaD6hrsnYJaF1DbxSWmEiIjmqmSspn7bpVjqA09ZKl1J bcMn/pM4qxJ9WDcMWGjwzZNUPNlFK4DLIDGVXmxc0jomghLaX1Hlwus93lEYOFMkZWBz c3YXNOutxviVZVbPCl95w/ib6EkYOkLO2Px2Ntu2NnoNTaEgMO5pCl4qmtNrUSghMdIS eFy24CeA4dszdB0OaQrHOpDiOblgkpCt/pvBOIIQQTijQOM1bAnfZ2B1Fk/yLBUn6ZAV 7d45IOsLVn/rTb79/AltsqlKSyMu1VOjsBKzdITuh+15uvFzE1HeT8dLROG/LtjngX53 OlCQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 67-v6si8924332pgi.456.2018.07.06.19.07.00; Fri, 06 Jul 2018 19:07:14 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932910AbeGGCGB (ORCPT + 99 others); Fri, 6 Jul 2018 22:06:01 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:37484 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932701AbeGGCF4 (ORCPT ); Fri, 6 Jul 2018 22:05:56 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 45195DFDA; Sat, 7 Jul 2018 02:05:56 +0000 (UTC) Received: from gblock2.localdomain (ovpn-12-18.pek2.redhat.com [10.72.12.18]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5B6672166BA9; Sat, 7 Jul 2018 02:05:52 +0000 (UTC) From: xiubli@redhat.com To: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org Cc: hamish.martin@alliedtelesis.co.nz, jannh@google.com, pkalever@redhat.com, pkarampu@redhat.com, atumball@redhat.com, sabose@redhat.com, mchristi@redhat.com Subject: [PATCH v4 2/3] uio: change to use the mutex lock instead of the spin lock Date: Fri, 6 Jul 2018 22:05:38 -0400 Message-Id: <1530929139-13652-3-git-send-email-xiubli@redhat.com> In-Reply-To: <1530929139-13652-1-git-send-email-xiubli@redhat.com> References: <1530929139-13652-1-git-send-email-xiubli@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Sat, 07 Jul 2018 02:05:56 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Sat, 07 Jul 2018 02:05:56 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'xiubli@redhat.com' RCPT:'' Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Xiubo Li We are hitting a regression with the following commit: commit a93e7b331568227500186a465fee3c2cb5dffd1f Author: Hamish Martin Date: Mon May 14 13:32:23 2018 +1200 uio: Prevent device destruction while fds are open The problem is the addition of spin_lock_irqsave in uio_write. This leads to hitting uio_write -> copy_from_user -> _copy_from_user -> might_fault and the logs filling up with sleeping warnings. I also noticed some uio drivers allocate memory, sleep, grab mutexes from callouts like open() and release and uio is now doing spin_lock_irqsave while calling them. Reported-by: Mike Christie CC: Hamish Martin Reviewed-by: Hamish Martin Signed-off-by: Xiubo Li --- drivers/uio/uio.c | 32 +++++++++++++------------------- include/linux/uio_driver.h | 2 +- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index b4b2ae1..655ade4 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -433,7 +433,6 @@ static int uio_open(struct inode *inode, struct file *filep) struct uio_device *idev; struct uio_listener *listener; int ret = 0; - unsigned long flags; mutex_lock(&minor_lock); idev = idr_find(&uio_idr, iminor(inode)); @@ -460,10 +459,10 @@ static int uio_open(struct inode *inode, struct file *filep) listener->event_count = atomic_read(&idev->event); filep->private_data = listener; - spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); if (idev->info && idev->info->open) ret = idev->info->open(idev->info, inode); - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock); if (ret) goto err_infoopen; @@ -495,12 +494,11 @@ static int uio_release(struct inode *inode, struct file *filep) int ret = 0; struct uio_listener *listener = filep->private_data; struct uio_device *idev = listener->dev; - unsigned long flags; - spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); if (idev->info && idev->info->release) ret = idev->info->release(idev->info, inode); - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock); module_put(idev->owner); kfree(listener); @@ -513,12 +511,11 @@ static __poll_t uio_poll(struct file *filep, poll_table *wait) struct uio_listener *listener = filep->private_data; struct uio_device *idev = listener->dev; __poll_t ret = 0; - unsigned long flags; - spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); if (!idev->info || !idev->info->irq) ret = -EIO; - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock); if (ret) return ret; @@ -537,12 +534,11 @@ static ssize_t uio_read(struct file *filep, char __user *buf, DECLARE_WAITQUEUE(wait, current); ssize_t retval = 0; s32 event_count; - unsigned long flags; - spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); if (!idev->info || !idev->info->irq) retval = -EIO; - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock); if (retval) return retval; @@ -592,9 +588,8 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, struct uio_device *idev = listener->dev; ssize_t retval; s32 irq_on; - unsigned long flags; - spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); if (!idev->info || !idev->info->irq) { retval = -EIO; goto out; @@ -618,7 +613,7 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, retval = idev->info->irqcontrol(idev->info, irq_on); out: - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock); return retval ? retval : sizeof(s32); } @@ -865,7 +860,7 @@ int __uio_register_device(struct module *owner, idev->owner = owner; idev->info = info; - spin_lock_init(&idev->info_lock); + mutex_init(&idev->info_lock); init_waitqueue_head(&idev->wait); atomic_set(&idev->event, 0); @@ -929,7 +924,6 @@ int __uio_register_device(struct module *owner, void uio_unregister_device(struct uio_info *info) { struct uio_device *idev; - unsigned long flags; if (!info || !info->uio_dev) return; @@ -943,9 +937,9 @@ void uio_unregister_device(struct uio_info *info) if (info->irq && info->irq != UIO_IRQ_CUSTOM) free_irq(info->irq, idev); - spin_lock_irqsave(&idev->info_lock, flags); + mutex_lock(&idev->info_lock); idev->info = NULL; - spin_unlock_irqrestore(&idev->info_lock, flags); + mutex_unlock(&idev->info_lock); device_unregister(&idev->dev); diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h index 6c5f207..6f8b68c 100644 --- a/include/linux/uio_driver.h +++ b/include/linux/uio_driver.h @@ -75,7 +75,7 @@ struct uio_device { struct fasync_struct *async_queue; wait_queue_head_t wait; struct uio_info *info; - spinlock_t info_lock; + struct mutex info_lock; struct kobject *map_dir; struct kobject *portio_dir; }; -- 1.8.3.1