[PATCH 07/18] net/tls: sanitize MSG_EOR handling

[Sagi Grimberg]

On 4/18/23 13:39, Hannes Reinecke wrote:
> On 4/18/23 12:24, Hannes Reinecke wrote:
>> On 4/18/23 12:07, Sagi Grimberg wrote:
>>>
>>>>>> The TLS stack is using MSG_EOR internally, so the flag cannot be
>>>>>> set for sendmsg()/sendpage(). But to avoid having the caller to
>>>>>> check whether TLS is active modify the code to clear the MSG_EOR
>>>>>> flag. And blank out MSG_MORE / MSG_SENDPAGE_NOTLAST, too, as they
>>>>>> conflict with MSG_EOR anyway.
>>>>>
>>>>> This looks like a temporary workaround to me.
>>>>>
>>>>> The networking folks really need to be CC'd on this (same for patch 
>>>>> 6).
>>>>
>>>> Thanks, when I said "we can support EOR" I obviously meant support
>>>> not ignore it :(  No ack.
>>>
>>> Obviously neither Hannes or I have sufficient tls knowledge to properly
>>> support it... It needs to be done by someone who knows the
>>> implementation.
>>>
>>> Is this a large scope to add support for it? Because if it is, I'd
>>> simply change nvme to clear MSG_EOR when tls is used (despite my
>>> preference to not do special things for tls).
>>
>> There's a rather simple patch for tls_sw. It already paces the data 
>> internally by an 'eor' variable, which is currently set to !MSG_MORE.
>> So evaluating MSG_EOR here is really trivial, and doesn't seem to 
>> cause any issues.
>>
>> Or, at least, none which would show up with NVMe-over-TLS :-)
>>
> Suggestion is to change it like this (sendpage follows suit):
> 
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 827292e29f99..7b28b11ff611 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -953,9 +953,12 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr 
> *msg, size_t size)
>          int pending;
> 
>          if (msg->msg_flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL |
> -                              MSG_CMSG_COMPAT))
> +                              MSG_EOR | MSG_CMSG_COMPAT))
>                  return -EOPNOTSUPP;
> 
> +       if (MSG_EOR)

You probably meant if (msg->msg_flags & MSG_EOR)...