RE: [PATCH for-rc] RDMA/qedr: Fix doorbell bar mapping for dpi > 1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Leon Romanovsky [mailto:leon@xxxxxxxxxx]
> Sent: Tuesday, May 15, 2018 2:46 PM
> To: Kalderon, Michal <Michal.Kalderon@xxxxxxxxxx>
> Cc: jgg@xxxxxxxxxxxx; dledford@xxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx;
> Bason, Yuval <Yuval.Bason@xxxxxxxxxx>; Elior, Ariel
> <Ariel.Elior@xxxxxxxxxx>
> Subject: Re: [PATCH for-rc] RDMA/qedr: Fix doorbell bar mapping for dpi > 1
> 
> On Mon, May 14, 2018 at 07:46:55PM +0000, Kalderon, Michal wrote:
> > From: linux-rdma-owner@xxxxxxxxxxxxxxx
> > <linux-rdma-owner@xxxxxxxxxxxxxxx> on behalf of Leon Romanovsky
> > <leon@xxxxxxxxxx>
> > Sent: Monday, May 14, 2018 11:06 AM
> > > > >
> > > > > Thanks
> > > > Thanks for pointing these out. I was not familiar with this
> > > > convention. However, the addresses Which are printed below are the
> > > > physical doorbell addresses which don't Seem to fall into the
> > > > category of the kernel pointers described in the article as the ones that
> should Be protected. Using the %pk doesn't print the value correctly.
> > > > Thanks,
> >
> > > Can you share the example code for such case? AFAIK, the %pK is
> > > actually censored equivalent of %px and that is equal to your %lx.
> >
> > > Also prints of vma->vm_start and vma->vm_end are for sure not
> > > physical doorbells.
> >
> > Modified the code as follows for comparison:
> > DP_DEBUG(dev, QEDR_MSG_INIT,
> >                  "mmap (pk) invoked with vm_start=0x%pk,x
> vm_end=0x%pk,vm_pgoff=0x%pk; dpi_start=0x%pk dpi_size=0x%x\n",
> >                  (void *)vma->vm_start, (void *)vma->vm_end, (void *)vma-
> >vm_pgoff, (void *)dpi_start,
> >                  ucontext->dpi_size);
> >
> >          DP_DEBUG(dev, QEDR_MSG_INIT,
> >                  "mmap (px) invoked with vm_start=0x%px,x
> vm_end=0x%px,vm_pgoff=0x%px; dpi_start=0x%px dpi_size=0x%x\n",
> >                   (void *)vma->vm_start, (void *)vma->vm_end, (void *)vma-
> >vm_pgoff, (void *)dpi_start,
> >                   ucontext->dpi_size);
> >
> >
> >         DP_DEBUG(dev, QEDR_MSG_INIT,
> >                  "mmap invoked with vm_start=0x%lx,x
> vm_end=0x%lx,vm_pgoff=0x%lx; dpi_start=0x%lx dpi_size=0x%x\n",
> >                  vma->vm_start, vma->vm_end, vma->vm_pgoff, dpi_start,
> >                  ucontext->dpi_size);
> >
> > and got the following output:
> >
> > [32937.851202] (qedr1) INIT: mmap (pk) invoked with
> > vm_start=0x000000009dacc20a,x
> > vm_end=0x0000000040480bf1,vm_pgoff=0x00000000d9849dd6;
> > dpi_start=0x00000000c74fd5ea dpi_size=0x2000 [32937.851203] (qedr1)
> > INIT: mmap (px) invoked with vm_start=0x00007f20575ad000,x
> > vm_end=0x00007f20575af000,vm_pgoff=0x0000000000091b03;
> > dpi_start=0x0000000091b03000 dpi_size=0x2000 [32937.851205] (qedr1)
> > INIT: mmap invoked with vm_start=0x7f20575ad000,x
> > vm_end=0x7f20575af000,vm_pgoff=0x91b03; dpi_start=0x91b03000
> > dpi_size=0x2000
> >
> > looks like the %pk gives hashed results similar to the %p even if the
> kptr_restrict is zero.
> > sysctl kernel/kptr_restrict
> > kernel.kptr_restrict = 0
> 
> I found the reason to such behavior:
> commit ef0010a30935de4e0211cbc7bdffc30446cdee9b
> Author: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Date:   Wed Nov 29 11:28:09 2017 -0800
> 
>     vsprintf: don't use 'restricted_pointer()' when not restricting
> 
>     Instead, just fall back on the new '%p' behavior which hashes the
>     pointer.
> 
>     Otherwise, '%pK' - that was intended to mark a pointer as restricted -
>     just ends up leaking pointers that a normal '%p' wouldn't leak.  Which
>     just make the whole thing pointless.
> 
>     I suspect we should actually get rid of '%pK' entirely, and make it just
>     work as '%p' regardless, but this is the minimal obvious fix.  People
>     who actually use 'kptr_restrict' should weigh in on which behavior they
>     want.
> 
>     Cc: Tobin Harding <me@xxxxxxxx>
>     Cc: Kees Cook <keescook@xxxxxxxxxxxx>
>     Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> 
> 
> This commit fixes the 7b1924a1d930 ("vsprintf: add printk specifier %px")

Thanks Leon, was just looking at this fix and realized I had to set kptr_restrict to 1 to get actual address.
V2 on it's way.


> 
> Thanks
> 
> >
> >
> > > Thanks
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> > in the body of a message to majordomo@xxxxxxxxxxxxxxx More
> majordomo
> > info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux