linux-kernel - Re: [for-next PATCH v5 05/11] RDMA/rxe: Allow registering persistent flag for pmem MR only

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]

Date:   Fri, 28 Oct 2022 14:53:19 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Li Zhijian <lizhijian@...itsu.com>
Cc:     Bob Pearson <rpearsonhpe@...il.com>,
        Leon Romanovsky <leon@...nel.org>, linux-rdma@...r.kernel.org,
        Zhu Yanjun <zyjzyj2000@...il.com>, yangx.jy@...itsu.com,
        y-goto@...itsu.com, mbloch@...dia.com, liangwenpeng@...wei.com,
        tom@...pey.com, tomasz.gromadzki@...el.com,
        dan.j.williams@...el.com, linux-kernel@...r.kernel.org
Subject: Re: [for-next PATCH v5 05/11] RDMA/rxe: Allow registering persistent
 flag for pmem MR only

On Tue, Sep 27, 2022 at 01:53:31PM +0800, Li Zhijian wrote:
> @@ -122,6 +129,7 @@ int rxe_mr_init_user(struct rxe_dev *rxe, u64 start, u64 length, u64 iova,
>  	int			num_buf;
>  	void			*vaddr;
>  	int err;
> +	bool is_pmem = false;
>  	int i;
>  
>  	umem = ib_umem_get(&rxe->ib_dev, start, length, access);
> @@ -149,6 +157,7 @@ int rxe_mr_init_user(struct rxe_dev *rxe, u64 start, u64 length, u64 iova,
>  	num_buf			= 0;
>  	map = mr->map;
>  	if (length > 0) {
> +		is_pmem = true;
>  		buf = map[0]->buf;
>  
>  		for_each_sgtable_page (&umem->sgt_append.sgt, &sg_iter, 0) {
> @@ -166,6 +175,10 @@ int rxe_mr_init_user(struct rxe_dev *rxe, u64 start, u64 length, u64 iova,
>  				goto err_cleanup_map;
>  			}
>  
> +			/* True only if the *whole* MR is pmem */
> +			if (is_pmem)
> +				is_pmem = vaddr_in_pmem(vaddr);
> +

I'm not so keen on this use of resources, but this should be written more
like

phys = page_to_phys(sg_page_iter_page(&sg_iter))
region_intersects(phys + sg_iter->offset, sg_iter->length,.. )

And you understand this will make memory registration of every RXE
user a bit slower? And actual pmem will be painfully slow.

It seems like we are doing something wrong here..

> @@ -174,6 +187,12 @@ int rxe_mr_init_user(struct rxe_dev *rxe, u64 start, u64 length, u64 iova,
>  		}
>  	}
>  
> +	if (!is_pmem && access & IB_ACCESS_FLUSH_PERSISTENT) {
> +		pr_warn("Cannot register IB_ACCESS_FLUSH_PERSISTENT for non-pmem memory\n");
> +		err = -EINVAL;
> +		goto err_release_umem;
> +	}

Do not pr_warn on syscall paths

Jason