linux-kernel - Re: Re: [PATCH] rdma/siw: avoid smp_store

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190712151423.GG27512@ziepe.ca>
Date:   Fri, 12 Jul 2019 12:14:23 -0300
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Bernard Metzler <BMT@...ich.ibm.com>,
        Doug Ledford <dledford@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-rdma <linux-rdma@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Re: [PATCH] rdma/siw: avoid smp_store_mb() on a u64

On Fri, Jul 12, 2019 at 03:22:35PM +0200, Arnd Bergmann wrote:
> On Fri, Jul 12, 2019 at 3:05 PM Bernard Metzler <BMT@...ich.ibm.com> wrote:
> 
> >
> > We share CQ (completion queue) notification flags between application
> > (which may be user land) and producer (kernel QP's (queue pairs)).
> > Those flags can be written by both application and QP's. The application
> > writes those flags to let the driver know if it shall inform about new
> > work completions. It can write those flags at any time.
> > Only a kernel producer reads those flags to decide if
> > the CQ notification handler shall be kicked, if a new CQ element gets
> > added to the CQ. When kicking the completion handler, the driver resets the
> > notification flag, which must get re-armed by the application.
> >
> > We use READ_ONCE() and WRITE_ONCE(), since the flags are potentially
> > shared (mmap'd) between user and kernel land.
> >
> > siw_req_notify_cq() is being called only by kernel consumers to change
> > (write) the CQ notification state. We use smp_store_mb() to make sure
> > the new value becomes visible to all kernel producers (QP's) asap.
> >
> >
> > From cfb861a09dcfb24a98ba0f1e26bdaa1529d1b006 Mon Sep 17 00:00:00 2001
> > From: Bernard Metzler <bmt@...ich.ibm.com>
> > Date: Fri, 12 Jul 2019 13:19:27 +0200
> > Subject: [PATCH] Make shared CQ notification flags 32bit to respect 32bit
> >  architectures
> >
> > Signed-off-by: Bernard Metzler <bmt@...ich.ibm.com>
> 
> This fixes the build for me, thanks!
> 
> Tested-by: Arnd Bergmann <arnd@...db.de>

Since this is coming up so late in the merge window, I'm inclined to
take the simple path while Bernard makes a complete solution
here. What do you think Arnd?

>From 0b043644c0ca601cb19943a81aa1f1455dbe9461 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgg@...lanox.com>
Date: Fri, 12 Jul 2019 12:12:06 -0300
Subject: [PATCH] RMDA/siw: Require a 64 bit arch

The new siw driver fails to build on i386 with

drivers/infiniband/sw/siw/siw_qp.c:1025:3: error: invalid output size for constraint '+q'
                smp_store_mb(*cq->notify, SIW_NOTIFY_NOT);

As it is using 64 bit values with the smp_store_mb.

Since the entire scheme here seems questionable, and we are in the merge
window, fix the compile failures by disabling 32 bit support on this
driver.

A proper fix will be reviewed post merge window.

Fixes: c0cf5bdde46c ("rdma/siw: addition to kernel build environment")
Reported-by: Arnd Bergmann <arnd@...db.de>
Signed-off-by: Jason Gunthorpe <jgg@...lanox.com>
---
 drivers/infiniband/sw/siw/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/siw/Kconfig b/drivers/infiniband/sw/siw/Kconfig
index b622fc62f2cd6d..dace276aea1413 100644
--- a/drivers/infiniband/sw/siw/Kconfig
+++ b/drivers/infiniband/sw/siw/Kconfig
@@ -1,6 +1,6 @@
 config RDMA_SIW
 	tristate "Software RDMA over TCP/IP (iWARP) driver"
-	depends on INET && INFINIBAND && LIBCRC32C
+	depends on INET && INFINIBAND && LIBCRC32C && 64BIT
 	select DMA_VIRT_OPS
 	help
 	This driver implements the iWARP RDMA transport over
-- 
2.21.0