linux-kernel - Re: [for-next PATCH 4/5] RDMA/rxe: refactor iova_to

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: <2158152.NgBsaNRSFp@suse>
Date:   Wed, 23 Nov 2022 00:24:26 +0100
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Jason Gunthorpe <jgg@...pe.ca>
Cc:     Li Zhijian <lizhijian@...itsu.com>,
        Zhu Yanjun <zyjzyj2000@...il.com>,
        Leon Romanovsky <leon@...nel.org>, linux-rdma@...r.kernel.org,
        linux-kernel@...r.kernel.org, Ira Weiny <ira.weiny@...el.com>
Subject: Re: [for-next PATCH 4/5] RDMA/rxe: refactor iova_to_vaddr

On lunedì 21 novembre 2022 19:53:54 CET Jason Gunthorpe wrote:
> On Wed, Nov 16, 2022 at 01:37:14PM +0100, Fabio M. De Francesco wrote:
> > > -     return (void *)(uintptr_t)mr->map[m]->buf[n].addr + offset;
> > > +     if (mr->ibmr.type == IB_MR_TYPE_USER) {
> > > +             char *paddr;
> > > +             struct page *pg = mr->map[m]->buf[n].page;
> > > +
> > > +             paddr = kmap_local_page(pg);
> > > +             if (paddr == NULL) {
> > > +                     pr_warn("Failed to map page");
> > > +                     return NULL;
> > > +             }
> > 
> > I know nothing about this code but I am here as a result of regular checks
> > for changes to HIGHMEM mappings across the entire kernel. So please 
forgive
> > me if I'm objecting to the correct changes.
> > 
> > 1) It looks like this code had a call to page_address() and you converted 
it
> > to mapping with kmap_local_page().
> 
> It only worked properly because the kconfig is blocking CONFIG_HIGHMEM
> so ZONE_HIGHMEM doesn't exist. These pages are obtained from GUP and
> could be anything.
> 
> This is done indirectly through
> 
> config INFINIBAND_VIRT_DMA
>         def_bool !HIGHMEM
> But this patch is undoing parts of what are causing that restriction
> by using the proper APIs. Though I'm not sure if we can eventually get
> to remove it for RXE completely.
> Jason

Ah, OK. This code was for !HIGHMEM kernels...

I understand but, FWIW I doubt that the use of page_address(), instead of 
kmapping, should ever be made on the bases that kconfig is blocking HIGHMEM.  

However, if I understand correctly, that restriction (!HIGHMEM) is going to be 
removed. Therefore, page_address() will break on HIGHMEM and Jason is 
correctly converting to kmap_local_page().

There is only one thing left... I think that he missed another mail from me. 
The one you responded to was missing the final paragraph, so I sent another 
message few minutes later.

kmap_local_page() returns always valid pointers to kernel virtual addresses. I 
can't see any ways for kmap_local_page() to return NULL. Therefore, I was 
asking for the reason to check for NULL.

Thanks,

Fabio