netdev - Re: [PATCH V2 net-next 5/7] rds: zerocopy Tx support.

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: <c737a91d-0414-f285-3887-786363de3f93@oracle.com>
Date:   Wed, 14 Feb 2018 13:14:08 -0800
From:   Santosh Shilimkar <santosh.shilimkar@...cle.com>
To:     Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:     netdev@...r.kernel.org, willemdebruijn.kernel@...il.com,
        davem@...emloft.net, rds-devel@....oracle.com
Subject: Re: [PATCH V2 net-next 5/7] rds: zerocopy Tx support.

On 2/14/2018 11:49 AM, Sowmini Varadhan wrote:
> On (02/14/18 11:10), Santosh Shilimkar wrote:
>> s/RDS_CMSG_ZCOPY_COOKIE/RDS_CMSG_ZMSGCOPY_COOKIE	
>>
> 
> Please see https://www.spinics.net/lists/netdev/msg483627.html
>
Just saw it and responded to Dave.


>>> @@ -356,6 +358,53 @@ int rds_message_copy_from_user(struct rds_message *rm, struct iov_iter *from)
>>>   	sg = rm->data.op_sg;
>>>   	sg_off = 0; /* Dear gcc, sg->page will be null from kzalloc. */
>>> +	if (zcopy) {
>>> +		int total_copied = 0;
>>> +		struct sk_buff *skb;
>>> +
>>> +		skb = alloc_skb(SO_EE_ORIGIN_MAX_ZCOOKIES * sizeof(u32),
>>> +				GFP_KERNEL);
>> This can sleep so you might want to check if you want to use ATOMIC version
>> here.
> 
> I think it should be fine: rds_message_copy_from_user() is called
> in process context, and if you notice, the calling function rds_sendmsg()
> also has this
>     1100         rm = rds_message_alloc(ret, GFP_KERNEL);
>     1101         if (!rm) {
>     1102                 ret = -ENOMEM;
>     1103                 goto out;
>     1104         }
> 
>      :
>     1106         /* Attach data to the rm */
>      :
>     1113                 ret = rds_message_copy_from_user(rm, &msg->msg_iter);
> 
> So using GFP_KERNEL is as safe as the call at line 1100.
>
Was just asking you to check if it is safe. The path already
does that so we are good.

> 
>>> +			return -ENOMEM;
>>> +		}
>> NOMEM new application visible change but probably the right one for this
>> particular case. Just need to make sure application can handle this
>> error.
> 
> I think the application already handles this correctly (see line 1102 above)
> 
Indeed. Thanks for checking.

Regards,
Santosh