From patchwork Mon Nov 21 17:38:20 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leon Romanovsky X-Patchwork-Id: 83281 Delivered-To: patch@linaro.org Received: by 10.182.1.168 with SMTP id 8csp1673288obn; Mon, 21 Nov 2016 09:38:32 -0800 (PST) X-Received: by 10.13.251.135 with SMTP id l129mr16358238ywf.353.1479749912790; Mon, 21 Nov 2016 09:38:32 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id w66si5008202ywc.388.2016.11.21.09.38.32; Mon, 21 Nov 2016 09:38:32 -0800 (PST) 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 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753055AbcKURi1 (ORCPT + 26 others); Mon, 21 Nov 2016 12:38:27 -0500 Received: from mail.kernel.org ([198.145.29.136]:34536 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752371AbcKURi0 (ORCPT ); Mon, 21 Nov 2016 12:38:26 -0500 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8052120155; Mon, 21 Nov 2016 17:38:24 +0000 (UTC) Received: from localhost (unknown [213.57.247.249]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id D2D1920148; Mon, 21 Nov 2016 17:38:22 +0000 (UTC) Date: Mon, 21 Nov 2016 19:38:20 +0200 From: Leon Romanovsky To: Jason Gunthorpe Cc: Dmitry Vyukov , syzkaller , Valdis.Kletnieks@vt.edu, dledford@redhat.com, sean.hefty@intel.com, Hal Rosenstock , linux-rdma@vger.kernel.org, LKML Subject: Re: [PATCH v2] infiniband: remove WARN that is not kernel bug Message-ID: <20161121173820.GC23083@leon.nu> References: <1479723531-17940-1-git-send-email-dvyukov@google.com> <20161121114417.GA4158@leon.nu> <20161121121408.GC4158@leon.nu> <20161121165253.GA22237@obsidianresearch.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161121165253.GA22237@obsidianresearch.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 21, 2016 at 09:52:53AM -0700, Jason Gunthorpe wrote: > On Mon, Nov 21, 2016 at 02:14:08PM +0200, Leon Romanovsky wrote: > > > > > > In ib_ucm_write function there is a wrong prefix: > > > > > > + pr_err_once("ucm_write: process %d (%s) tried to do something hinky\n", > > > > I did it intentionally to have the same errors for all flows. > > Lets actually use a good message too please? > > pr_err_once("ucm_write: process %d (%s) changed security contexts after opening FD, this is not allowed.\n", > > Jason >From 70f95b2d35aea42e5b97e7d27ab2f4e8effcbe67 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Mon, 21 Nov 2016 13:30:59 +0200 Subject: [PATCH rdma-next V2] IB/{core, qib}: Remove WARN that is not kernel bug WARNINGs mean kernel bugs, in this case, they are placed to mark programming errors and/or malicious attempts. BUG/WARNs that are not kernel bugs hinder automated testing efforts. Signed-off-by: Dmitry Vyukov Signed-off-by: Leon Romanovsky --- * Improved error prints. --- drivers/infiniband/core/ucm.c | 5 ++++- drivers/infiniband/core/ucma.c | 5 ++++- drivers/infiniband/core/uverbs_main.c | 5 ++++- drivers/infiniband/hw/qib/qib_file_ops.c | 5 ++++- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c index 7713ef0..579f9a7 100644 --- a/drivers/infiniband/core/ucm.c +++ b/drivers/infiniband/core/ucm.c @@ -1104,8 +1104,11 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf, struct ib_ucm_cmd_hdr hdr; ssize_t result; - if (WARN_ON_ONCE(!ib_safe_file_access(filp))) + if (!ib_safe_file_access(filp)) { + pr_err_once("ucm_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n", + task_tgid_vnr(current), current->comm); return -EACCES; + } if (len < sizeof(hdr)) return -EINVAL; diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c index 9520154..e12f8fa 100644 --- a/drivers/infiniband/core/ucma.c +++ b/drivers/infiniband/core/ucma.c @@ -1584,8 +1584,11 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf, struct rdma_ucm_cmd_hdr hdr; ssize_t ret; - if (WARN_ON_ONCE(!ib_safe_file_access(filp))) + if (!ib_safe_file_access(filp)) { + pr_err_once("ucma_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n", + task_tgid_vnr(current), current->comm); return -EACCES; + } if (len < sizeof(hdr)) return -EINVAL; diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c index 44b1104..38c79ad 100644 --- a/drivers/infiniband/core/uverbs_main.c +++ b/drivers/infiniband/core/uverbs_main.c @@ -746,8 +746,11 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf, int srcu_key; ssize_t ret; - if (WARN_ON_ONCE(!ib_safe_file_access(filp))) + if (!ib_safe_file_access(filp)) { + pr_err_once("uverbs_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n", + task_tgid_vnr(current), current->comm); return -EACCES; + } if (count < sizeof hdr) return -EINVAL; diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c index 382466a..2d1eacf 100644 --- a/drivers/infiniband/hw/qib/qib_file_ops.c +++ b/drivers/infiniband/hw/qib/qib_file_ops.c @@ -2066,8 +2066,11 @@ static ssize_t qib_write(struct file *fp, const char __user *data, ssize_t ret = 0; void *dest; - if (WARN_ON_ONCE(!ib_safe_file_access(fp))) + if (!ib_safe_file_access(fp)) { + pr_err_once("qib_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n", + task_tgid_vnr(current), current->comm); return -EACCES; + } if (count < sizeof(cmd.type)) { ret = -EINVAL; -- 2.7.4