* [PATCH v2] infiniband: remove WARN that is not kernel bug @ 2016-11-21 10:18 Dmitry Vyukov 2016-11-21 10:25 ` Miroslav Benes 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Vyukov @ 2016-11-21 10:18 UTC (permalink / raw) To: jgunthorpe, Valdis.Kletnieks, dledford, sean.hefty, hal.rosenstock, leon Cc: Dmitry Vyukov, linux-rdma, linux-kernel, syzkaller WARNINGs mean kernel bugs. The one in ucma_write() points to user programming error or a malicious attempt. This is not a kernel bug, remove it. BUG/WARNs that are not kernel bugs hinder automated testing effots. Signed-off-by: Dmitry Vyukov <dvyukov@google.com> Cc: Doug Ledford <dledford@redhat.com> Cc: Sean Hefty <sean.hefty@intel.com> Cc: Hal Rosenstock <hal.rosenstock@gmail.com> Cc: Leon Romanovsky <leon@kernel.org> Cc: linux-rdma@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: syzkaller@googlegroups.com --- Changes since v1: - added printk_once --- drivers/infiniband/core/ucma.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c index 9520154..405d0ce 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)) { + printk_once("ucma_write: process %d (%s) tried to do something hinky\n", + task_tgid_vnr(current), current->comm); return -EACCES; + } if (len < sizeof(hdr)) return -EINVAL; -- 2.8.0.rc3.226.g39d4020 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] infiniband: remove WARN that is not kernel bug 2016-11-21 10:18 [PATCH v2] infiniband: remove WARN that is not kernel bug Dmitry Vyukov @ 2016-11-21 10:25 ` Miroslav Benes [not found] ` <alpine.LNX.2.00.1611211123450.22752-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Miroslav Benes @ 2016-11-21 10:25 UTC (permalink / raw) To: Dmitry Vyukov Cc: jgunthorpe, Valdis.Kletnieks, dledford, sean.hefty, hal.rosenstock, leon, linux-rdma, linux-kernel, syzkaller On Mon, 21 Nov 2016, Dmitry Vyukov wrote: > WARNINGs mean kernel bugs. > The one in ucma_write() points to user programming error > or a malicious attempt. This is not a kernel bug, remove it. > > BUG/WARNs that are not kernel bugs hinder automated testing effots. > > Signed-off-by: Dmitry Vyukov <dvyukov@google.com> > Cc: Doug Ledford <dledford@redhat.com> > Cc: Sean Hefty <sean.hefty@intel.com> > Cc: Hal Rosenstock <hal.rosenstock@gmail.com> > Cc: Leon Romanovsky <leon@kernel.org> > Cc: linux-rdma@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: syzkaller@googlegroups.com > > --- > Changes since v1: > - added printk_once > --- > drivers/infiniband/core/ucma.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c > index 9520154..405d0ce 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)) { > + printk_once("ucma_write: process %d (%s) tried to do something hinky\n", > + task_tgid_vnr(current), current->comm); > return -EACCES; > + } > > if (len < sizeof(hdr)) > return -EINVAL; FWIW, WARN_ON_ONCE came with commit e6bd18f57aad ("IB/security: Restrict use of the write() interface"). Would it make sense to change the other places as well? Regards, Miroslav ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <alpine.LNX.2.00.1611211123450.22752-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>]
* Re: [PATCH v2] infiniband: remove WARN that is not kernel bug [not found] ` <alpine.LNX.2.00.1611211123450.22752-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org> @ 2016-11-21 10:30 ` Dmitry Vyukov [not found] ` <CACT4Y+ZO9sTbBGwNUUMkAQ5m0N-4aT0S9xypCRbBUch1pQNw9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Vyukov @ 2016-11-21 10:30 UTC (permalink / raw) To: syzkaller Cc: Jason Gunthorpe, Valdis.Kletnieks-PjAqaU27lzQ, dledford-H+wXaHxf7aLQT0dZR+AlfA, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, Hal Rosenstock, leon-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA, LKML On Mon, Nov 21, 2016 at 11:25 AM, Miroslav Benes <mbenes-AlSwsSmVLrQ@public.gmane.org> wrote: > On Mon, 21 Nov 2016, Dmitry Vyukov wrote: > >> WARNINGs mean kernel bugs. >> The one in ucma_write() points to user programming error >> or a malicious attempt. This is not a kernel bug, remove it. >> >> BUG/WARNs that are not kernel bugs hinder automated testing effots. >> >> Signed-off-by: Dmitry Vyukov <dvyukov-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> >> Cc: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >> Cc: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> >> Cc: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >> Cc: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> >> Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >> Cc: syzkaller-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org >> >> --- >> Changes since v1: >> - added printk_once >> --- >> drivers/infiniband/core/ucma.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c >> index 9520154..405d0ce 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)) { >> + printk_once("ucma_write: process %d (%s) tried to do something hinky\n", >> + task_tgid_vnr(current), current->comm); >> return -EACCES; >> + } >> >> if (len < sizeof(hdr)) >> return -EINVAL; > > FWIW, WARN_ON_ONCE came with commit e6bd18f57aad ("IB/security: Restrict > use of the write() interface"). Would it make sense to change the other > places as well? I guess so. Can I ask somebody of infiniband maintainers to take care of this? I just hit the warning in my automated testing environment when a thread executed key_add in between open and write, then spent some time debugging to figure out that this is an "invalid user input" rather than a kernel bug. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CACT4Y+ZO9sTbBGwNUUMkAQ5m0N-4aT0S9xypCRbBUch1pQNw9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] infiniband: remove WARN that is not kernel bug [not found] ` <CACT4Y+ZO9sTbBGwNUUMkAQ5m0N-4aT0S9xypCRbBUch1pQNw9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2016-11-21 11:44 ` Leon Romanovsky 2016-11-21 11:48 ` Dmitry Vyukov 0 siblings, 1 reply; 10+ messages in thread From: Leon Romanovsky @ 2016-11-21 11:44 UTC (permalink / raw) To: Dmitry Vyukov Cc: syzkaller, Jason Gunthorpe, Valdis.Kletnieks-PjAqaU27lzQ, dledford-H+wXaHxf7aLQT0dZR+AlfA, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, Hal Rosenstock, linux-rdma-u79uwXL29TY76Z2rM5mHXA, LKML [-- Attachment #1.1: Type: text/plain, Size: 2469 bytes --] On Mon, Nov 21, 2016 at 11:30:21AM +0100, Dmitry Vyukov wrote: > On Mon, Nov 21, 2016 at 11:25 AM, Miroslav Benes <mbenes-AlSwsSmVLrQ@public.gmane.org> wrote: > > On Mon, 21 Nov 2016, Dmitry Vyukov wrote: > > > >> WARNINGs mean kernel bugs. > >> The one in ucma_write() points to user programming error > >> or a malicious attempt. This is not a kernel bug, remove it. > >> > >> BUG/WARNs that are not kernel bugs hinder automated testing effots. > >> > >> Signed-off-by: Dmitry Vyukov <dvyukov-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> > >> Cc: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > >> Cc: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > >> Cc: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > >> Cc: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > >> Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > >> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > >> Cc: syzkaller-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org > >> > >> --- > >> Changes since v1: > >> - added printk_once > >> --- > >> drivers/infiniband/core/ucma.c | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c > >> index 9520154..405d0ce 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)) { > >> + printk_once("ucma_write: process %d (%s) tried to do something hinky\n", > >> + task_tgid_vnr(current), current->comm); > >> return -EACCES; > >> + } > >> > >> if (len < sizeof(hdr)) > >> return -EINVAL; > > > > FWIW, WARN_ON_ONCE came with commit e6bd18f57aad ("IB/security: Restrict > > use of the write() interface"). Would it make sense to change the other > > places as well? > > > I guess so. > Can I ask somebody of infiniband maintainers to take care of this? Please see below, Hope it helps. > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html [-- Attachment #1.2: 0001-IB-core-qib-Remove-WARN-that-is-not-kernel-bug.patch --] [-- Type: text/x-diff, Size: 3383 bytes --] From e0bfe4771591f9ffbcaa4e66d6257ed95e2d7188 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> Date: Mon, 21 Nov 2016 13:30:59 +0200 Subject: [PATCH] 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 <dvyukov-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> --- 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..0076b55 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) tried to do something hinky\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..31fa27f 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) tried to do something hinky\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..329eb15 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) tried to do something hinky\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..b43ea52 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) tried to do something hinky\n", + task_tgid_vnr(current), current->comm); return -EACCES; + } if (count < sizeof(cmd.type)) { ret = -EINVAL; -- 2.7.4 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] infiniband: remove WARN that is not kernel bug 2016-11-21 11:44 ` Leon Romanovsky @ 2016-11-21 11:48 ` Dmitry Vyukov [not found] ` <CACT4Y+Y1sL-8K7HhZ5M5ABVPG0kM=dKZh+6gvuNRvzXFtiA5aQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Vyukov @ 2016-11-21 11:48 UTC (permalink / raw) To: Leon Romanovsky Cc: syzkaller, Jason Gunthorpe, Valdis.Kletnieks, dledford, sean.hefty, Hal Rosenstock, linux-rdma, LKML On Mon, Nov 21, 2016 at 12:44 PM, Leon Romanovsky <leon@kernel.org> wrote: > On Mon, Nov 21, 2016 at 11:30:21AM +0100, Dmitry Vyukov wrote: >> On Mon, Nov 21, 2016 at 11:25 AM, Miroslav Benes <mbenes@suse.cz> wrote: >> > On Mon, 21 Nov 2016, Dmitry Vyukov wrote: >> > >> >> WARNINGs mean kernel bugs. >> >> The one in ucma_write() points to user programming error >> >> or a malicious attempt. This is not a kernel bug, remove it. >> >> >> >> BUG/WARNs that are not kernel bugs hinder automated testing effots. >> >> >> >> Signed-off-by: Dmitry Vyukov <dvyukov@google.com> >> >> Cc: Doug Ledford <dledford@redhat.com> >> >> Cc: Sean Hefty <sean.hefty@intel.com> >> >> Cc: Hal Rosenstock <hal.rosenstock@gmail.com> >> >> Cc: Leon Romanovsky <leon@kernel.org> >> >> Cc: linux-rdma@vger.kernel.org >> >> Cc: linux-kernel@vger.kernel.org >> >> Cc: syzkaller@googlegroups.com >> >> >> >> --- >> >> Changes since v1: >> >> - added printk_once >> >> --- >> >> drivers/infiniband/core/ucma.c | 5 ++++- >> >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c >> >> index 9520154..405d0ce 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)) { >> >> + printk_once("ucma_write: process %d (%s) tried to do something hinky\n", >> >> + task_tgid_vnr(current), current->comm); >> >> return -EACCES; >> >> + } >> >> >> >> if (len < sizeof(hdr)) >> >> return -EINVAL; >> > >> > FWIW, WARN_ON_ONCE came with commit e6bd18f57aad ("IB/security: Restrict >> > use of the write() interface"). Would it make sense to change the other >> > places as well? >> >> >> I guess so. >> Can I ask somebody of infiniband maintainers to take care of this? > > Please see below, > Hope it helps. In ib_ucm_write function there is a wrong prefix: + pr_err_once("ucm_write: process %d (%s) tried to do something hinky\n", Otherwise looks good. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CACT4Y+Y1sL-8K7HhZ5M5ABVPG0kM=dKZh+6gvuNRvzXFtiA5aQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] infiniband: remove WARN that is not kernel bug [not found] ` <CACT4Y+Y1sL-8K7HhZ5M5ABVPG0kM=dKZh+6gvuNRvzXFtiA5aQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2016-11-21 12:14 ` Leon Romanovsky 2016-11-21 16:52 ` Jason Gunthorpe 0 siblings, 1 reply; 10+ messages in thread From: Leon Romanovsky @ 2016-11-21 12:14 UTC (permalink / raw) To: Dmitry Vyukov Cc: syzkaller, Jason Gunthorpe, Valdis.Kletnieks-PjAqaU27lzQ, dledford-H+wXaHxf7aLQT0dZR+AlfA, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, Hal Rosenstock, linux-rdma-u79uwXL29TY76Z2rM5mHXA, LKML [-- Attachment #1: Type: text/plain, Size: 2805 bytes --] On Mon, Nov 21, 2016 at 12:48:35PM +0100, Dmitry Vyukov wrote: > On Mon, Nov 21, 2016 at 12:44 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: > > On Mon, Nov 21, 2016 at 11:30:21AM +0100, Dmitry Vyukov wrote: > >> On Mon, Nov 21, 2016 at 11:25 AM, Miroslav Benes <mbenes-AlSwsSmVLrQ@public.gmane.org> wrote: > >> > On Mon, 21 Nov 2016, Dmitry Vyukov wrote: > >> > > >> >> WARNINGs mean kernel bugs. > >> >> The one in ucma_write() points to user programming error > >> >> or a malicious attempt. This is not a kernel bug, remove it. > >> >> > >> >> BUG/WARNs that are not kernel bugs hinder automated testing effots. > >> >> > >> >> Signed-off-by: Dmitry Vyukov <dvyukov-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> > >> >> Cc: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > >> >> Cc: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > >> >> Cc: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > >> >> Cc: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > >> >> Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > >> >> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > >> >> Cc: syzkaller-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org > >> >> > >> >> --- > >> >> Changes since v1: > >> >> - added printk_once > >> >> --- > >> >> drivers/infiniband/core/ucma.c | 5 ++++- > >> >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> >> > >> >> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c > >> >> index 9520154..405d0ce 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)) { > >> >> + printk_once("ucma_write: process %d (%s) tried to do something hinky\n", > >> >> + task_tgid_vnr(current), current->comm); > >> >> return -EACCES; > >> >> + } > >> >> > >> >> if (len < sizeof(hdr)) > >> >> return -EINVAL; > >> > > >> > FWIW, WARN_ON_ONCE came with commit e6bd18f57aad ("IB/security: Restrict > >> > use of the write() interface"). Would it make sense to change the other > >> > places as well? > >> > >> > >> I guess so. > >> Can I ask somebody of infiniband maintainers to take care of this? > > > > Please see below, > > Hope it helps. > > 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. > > Otherwise looks good. Thanks. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] infiniband: remove WARN that is not kernel bug 2016-11-21 12:14 ` Leon Romanovsky @ 2016-11-21 16:52 ` Jason Gunthorpe [not found] ` <20161121165253.GA22237-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jason Gunthorpe @ 2016-11-21 16:52 UTC (permalink / raw) To: Leon Romanovsky Cc: Dmitry Vyukov, syzkaller, Valdis.Kletnieks, dledford, sean.hefty, Hal Rosenstock, linux-rdma, LKML 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20161121165253.GA22237-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [PATCH v2] infiniband: remove WARN that is not kernel bug [not found] ` <20161121165253.GA22237-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2016-11-21 17:38 ` Leon Romanovsky [not found] ` <20161121173820.GC23083-2ukJVAZIZ/Y@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Leon Romanovsky @ 2016-11-21 17:38 UTC (permalink / raw) To: Jason Gunthorpe Cc: Dmitry Vyukov, syzkaller, Valdis.Kletnieks-PjAqaU27lzQ, dledford-H+wXaHxf7aLQT0dZR+AlfA, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, Hal Rosenstock, linux-rdma-u79uwXL29TY76Z2rM5mHXA, LKML [-- Attachment #1.1: Type: text/plain, Size: 523 bytes --] 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 [-- Attachment #1.2: 0001-IB-core-qib-Remove-WARN-that-is-not-kernel-bug.patch --] [-- Type: text/x-diff, Size: 3630 bytes --] From 70f95b2d35aea42e5b97e7d27ab2f4e8effcbe67 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> 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 <dvyukov-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> --- * 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20161121173820.GC23083-2ukJVAZIZ/Y@public.gmane.org>]
* Re: [PATCH v2] infiniband: remove WARN that is not kernel bug [not found] ` <20161121173820.GC23083-2ukJVAZIZ/Y@public.gmane.org> @ 2016-12-14 18:16 ` Doug Ledford 2016-12-14 18:27 ` Leon Romanovsky 0 siblings, 1 reply; 10+ messages in thread From: Doug Ledford @ 2016-12-14 18:16 UTC (permalink / raw) To: Leon Romanovsky, Jason Gunthorpe Cc: Dmitry Vyukov, syzkaller, Valdis.Kletnieks-PjAqaU27lzQ, sean.hefty-ral2JQCrhuEAvxtiuMwx3w, Hal Rosenstock, linux-rdma-u79uwXL29TY76Z2rM5mHXA, LKML [-- Attachment #1.1: Type: text/plain, Size: 750 bytes --] On 11/21/2016 12:38 PM, Leon Romanovsky wrote: > 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 I applied Leon's reworked version of this patch, thanks. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG Key ID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] infiniband: remove WARN that is not kernel bug 2016-12-14 18:16 ` Doug Ledford @ 2016-12-14 18:27 ` Leon Romanovsky 0 siblings, 0 replies; 10+ messages in thread From: Leon Romanovsky @ 2016-12-14 18:27 UTC (permalink / raw) To: Doug Ledford Cc: Jason Gunthorpe, Dmitry Vyukov, syzkaller, Valdis.Kletnieks, sean.hefty, Hal Rosenstock, linux-rdma, LKML [-- Attachment #1: Type: text/plain, Size: 848 bytes --] On Wed, Dec 14, 2016 at 01:16:45PM -0500, Doug Ledford wrote: > On 11/21/2016 12:38 PM, Leon Romanovsky wrote: > > 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 > > I applied Leon's reworked version of this patch, thanks. Thanks Doug, I already forgot about it :) > > -- > Doug Ledford <dledford@redhat.com> > GPG Key ID: 0E572FDD > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-12-14 18:27 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-21 10:18 [PATCH v2] infiniband: remove WARN that is not kernel bug Dmitry Vyukov
2016-11-21 10:25 ` Miroslav Benes
[not found] ` <alpine.LNX.2.00.1611211123450.22752-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
2016-11-21 10:30 ` Dmitry Vyukov
[not found] ` <CACT4Y+ZO9sTbBGwNUUMkAQ5m0N-4aT0S9xypCRbBUch1pQNw9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-21 11:44 ` Leon Romanovsky
2016-11-21 11:48 ` Dmitry Vyukov
[not found] ` <CACT4Y+Y1sL-8K7HhZ5M5ABVPG0kM=dKZh+6gvuNRvzXFtiA5aQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-21 12:14 ` Leon Romanovsky
2016-11-21 16:52 ` Jason Gunthorpe
[not found] ` <20161121165253.GA22237-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2016-11-21 17:38 ` Leon Romanovsky
[not found] ` <20161121173820.GC23083-2ukJVAZIZ/Y@public.gmane.org>
2016-12-14 18:16 ` Doug Ledford
2016-12-14 18:27 ` Leon Romanovsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).