linux-kernel - RE: Re: [PATCH] RDMA/siw: fix pointer cast warning

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] [day] [month] [year] [list]

Date:   Fri, 16 Dec 2022 13:46:39 +0000
From:   Bernard Metzler <BMT@...ich.ibm.com>
To:     David Laight <David.Laight@...LAB.COM>,
        Linus Walleij <linus.walleij@...aro.org>
CC:     Arnd Bergmann <arnd@...nel.org>, Jason Gunthorpe <jgg@...pe.ca>,
        Leon Romanovsky <leon@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: Re: [PATCH] RDMA/siw: fix pointer cast warning



> -----Original Message-----
> From: David Laight <David.Laight@...LAB.COM>
> Sent: Friday, 16 December 2022 11:23
> To: Bernard Metzler <BMT@...ich.ibm.com>; Linus Walleij
> <linus.walleij@...aro.org>
> Cc: Arnd Bergmann <arnd@...nel.org>; Jason Gunthorpe <jgg@...pe.ca>; Leon
> Romanovsky <leon@...nel.org>; Arnd Bergmann <arnd@...db.de>; linux-
> rdma@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: [EXTERNAL] RE: Re: [PATCH] RDMA/siw: fix pointer cast warning
> 
> From: Bernard Metzler
> > Sent: 16 December 2022 10:01
> >
> > > -----Original Message-----
> > > From: Linus Walleij <linus.walleij@...aro.org>
> > > Sent: Friday, 16 December 2022 08:47
> > > To: David Laight <David.Laight@...lab.com>
> > > Cc: Arnd Bergmann <arnd@...nel.org>; Bernard Metzler
> <BMT@...ich.ibm.com>;
> > > Jason Gunthorpe <jgg@...pe.ca>; Leon Romanovsky <leon@...nel.org>; Arnd
> > > Bergmann <arnd@...db.de>; linux-rdma@...r.kernel.org; linux-
> > > kernel@...r.kernel.org
> > > Subject: [EXTERNAL] Re: [PATCH] RDMA/siw: fix pointer cast warning
> > >
> > > On Thu, Dec 15, 2022 at 11:20 PM David Laight <David.Laight@...lab.com>
> > > wrote:
> > >
> > > > From: Arnd Bergmann
> > > > > Sent: 15 December 2022 17:04
> > > > >
> > > > > From: Arnd Bergmann <arnd@...db.de>
> > > > >
> > > > > The previous build fix left a remaining issue in configurations
> > > > > with 64-bit dma_addr_t on 32-bit architectures:
> > > > >
> > > > > drivers/infiniband/sw/siw/siw_qp_tx.c: In function
> 'siw_get_pblpage':
> > > > > drivers/infiniband/sw/siw/siw_qp_tx.c:32:37: error: cast to pointer
> > > from integer of different size [-
> > > > > Werror=int-to-pointer-cast]
> > > > >    32 |                 return virt_to_page((void *)paddr);
> > > > >       |                                     ^
> > > > >
> > > > > Use the same double cast here that the driver uses elsewhere
> > > > > to convert between dma_addr_t and void*.
> > > > >
> > > > > It took me a while to figure out why this driver does it
> > > > > like this, as there is no hardware access and it just stores
> > > > > kernel pointers in place of device addresses when communicating
> > > > > with the rdma core and with user space.
> > > >
> > > > I hope that doesn't mean it is relying on user space only
> > > > giving it back valid values?
> > >
> > > It looks to me like this driver totally trusts userspace.
> > >
> >
> > Shame on me. Yes, somehow, an access_ok((void __user *)start, len)
> > is missing! Let me fix that when I am back at my desk. Seems it needs
> > immediate action.
> 
> That wasn't the sort of issue I was thinking about.
> I was worried that it was putting the addresses of kernel memory
> into buffers written to userspace and then later reading the
> addresses back from userspace and accessing them.
> 

Oh, no, that is not the case. The address mapping is not accessible
from userspace. It is local to the kernel driver only to translate
user virtual addresses to kernel pages during transmit/receive of 
application data. The user only knows about its own VA's it uses
in its work requests, and during buffer registration.

BUT, you pointed me to something bad. Checking the users permission
to register requested memory with the driver is definitely missing.
I was under the wrong impression it would be checked by used
pin_user_pages(), but that is not the case. pin_user_pages_fast()
does that check, other drivers using it, and it looks like a good fix.

Other rdma drivers, like infiniband/hw/usnic, wich also use
pin_user_pages(), may suffer from the same problem. There is no
checking of user permissions for the memory being registered
from user land.

Thanks very much,
Bernard.




> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)