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

[Hannes Reinecke <hare@suse.de>]

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)
+               eor = true;
+
         ret = mutex_lock_interruptible(&tls_ctx->tx_lock);
         if (ret)
                 return ret;

Cheers,

Hannes