netdev - Re: [net-next][PATCH v2 1/2] rds: handle unsupported rdma request to fs dax memory

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:   Mon, 6 May 2019 09:39:15 -0700
From:   Santosh Shilimkar <santosh.shilimkar@...cle.com>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        Moni Shoua <monis@...lanox.com>
Subject: Re: [net-next][PATCH v2 1/2] rds: handle unsupported rdma request to
 fs dax memory



On 5/4/2019 11:28 PM, Leon Romanovsky wrote:
> On Thu, May 02, 2019 at 10:52:23AM -0700, Santosh Shilimkar wrote:
>> On 5/1/2019 11:21 PM, Leon Romanovsky wrote:
>>> On Wed, May 01, 2019 at 10:54:00AM -0700, Santosh Shilimkar wrote:
>>>> On 5/1/2019 12:44 AM, Leon Romanovsky wrote:
>>>>> On Mon, Apr 29, 2019 at 04:37:19PM -0700, Santosh Shilimkar wrote:
>>>>>> From: Hans Westgaard Ry <hans.westgaard.ry@...cle.com>
>>>>>>
>>>>>> RDS doesn't support RDMA on memory apertures that require On Demand
>>>>>> Paging (ODP), such as FS DAX memory. User applications can try to use
>>>>>> RDS to perform RDMA over such memories and since it doesn't report any
>>>>>> failure, it can lead to unexpected issues like memory corruption when
>>>>>> a couple of out of sync file system operations like ftruncate etc. are
>>>>>> performed.
>>>>>>
>>>>>> The patch adds a check so that such an attempt to RDMA to/from memory
>>>>>> apertures requiring ODP will fail.
>>>>>>
>>>>>> Reviewed-by: H??kon Bugge <haakon.bugge@...cle.com>
>>>>>> Reviewed-tested-by: Zhu Yanjun <yanjun.zhu@...cle.com>
>>>>>> Signed-off-by: Hans Westgaard Ry <hans.westgaard.ry@...cle.com>
>>>>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@...cle.com>
>>>>>> ---
>>>>>>     net/rds/rdma.c | 5 +++--
>>>>>>     1 file changed, 3 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/net/rds/rdma.c b/net/rds/rdma.c
>>>>>> index 182ab84..e0a6b72 100644
>>>>>> --- a/net/rds/rdma.c
>>>>>> +++ b/net/rds/rdma.c
>>>>>> @@ -158,8 +158,9 @@ static int rds_pin_pages(unsigned long user_addr, unsigned int nr_pages,
>>>>>>     {
>>>>>>     	int ret;
>>>>>>
>>>>>> -	ret = get_user_pages_fast(user_addr, nr_pages, write, pages);
>>>>>> -
>>>>>> +	/* get_user_pages return -EOPNOTSUPP for fs_dax memory */
>>>>>> +	ret = get_user_pages_longterm(user_addr, nr_pages,
>>>>>> +				      write, pages, NULL);
>>>>>
>>>>> I'm not RDS expert, but from what I see in net/rds/rdma.c and this code,
>>>>> you tried to mimic ib_umem_get() without protection, checks and native
>>>>> ODP, FS and DAX supports.
>>>>>
>>>>> The real way to solve your ODP problem will require to extend
>>>>> ib_umem_get() to work for kernel ULPs too and use it instead of
>>>>> get_user_pages(). We are working on that and it is in internal review now.
>>>>>
>>>> Yes am aware of it. For FS_DAX like memory,  get_user_pages_longterm()
>>>> fails and then using ib_reg_user_mr() the memory is registered as
>>>> ODP regsion. This work is not ready yet and without above check,
>>>> one can do RDMA on FS DAX memory with Fast Reg or FMR memory
>>>> registration which is not safe and hence need to fail the operation.
>>>>
>>>> Once the support is added to RDS, this code path will make that
>>>> registration go through.
>>>>
>>>> Hope it clarifies.
>>>
>>> Only partial, why don't you check if user asked ODP through verbs
>>> interface and return EOPNOTSUPP in such case?
>>>
>> I think you are mixing two separate things. ODP is just one way of
>> supporting RDMA on FS DAX memory. Tomorrow, some other mechanism
>> can be used as well. RDS is just using inbuilt kernel mm API
>> to find out if its FS DAX memory(get_user_pages_longterm).
>> Current code will make RDS get_mr fail if RDS application issues
>> memory registration request on FS DAX memory and in future when
>> support gets added, it will do the ODP registration and return
>> the key.
> 
> But we are talking about kernel code only, right?
> Future support will be added if it exists.
> 
yes kernel code only.

>>
>>> It will ensure that once your code will support ODP properly written
>>> applications will work with/without ODP natively.
>>>
>> Application shouldn't care if RDS ULP internally uses ODP
>> or some other mechanism to support RDMA on FS DAX memory.
>> This makes it transparent it to RDS application.
> 
> ODP checks need to be internal to kernel, user won't see those ODP
> checks.
> 
Correct. The check is within RDS kernel module.