Re: [PATCH net v2] tls: fix the open record check in the max payload size setsockopt

[Jiayuan Chen <jiayuan.chen@linux.dev>]

在 9/3/26 5:06 PM, Paolo Abeni 写道:
> On 9/1/26 9:29 AM, Jiayuan Chen wrote:
>> @@ -862,6 +876,7 @@ static int do_tls_setsockopt_tx_payload_len(struct sock *sk, sockptr_t optval,
>>   static int do_tls_setsockopt(struct sock *sk, int optname, sockptr_t optval,
>>   			     unsigned int optlen)
>>   {
>> +	struct tls_context *ctx;
>>   	int rc = 0;
>>   
>>   	switch (optname) {
>> @@ -881,9 +896,17 @@ static int do_tls_setsockopt(struct sock *sk, int optname, sockptr_t optval,
>>   		rc = do_tls_setsockopt_no_pad(sk, optval, optlen);
>>   		break;
>>   	case TLS_TX_MAX_PAYLOAD_LEN:
>> +		/* Take tx_lock like the sendmsg paths do, the socket lock is
>> +		 * dropped while a sender waits for memory, with no record open.
>> +		 */
>> +		ctx = tls_get_ctx(sk);
>> +		rc = mutex_lock_interruptible(&ctx->tx_lock);
>> +		if (rc)
> Why using the interruptible variant? the blocking lock just after will
> still ignore signals, and this sockopt will now surprisingly fail if a
> signal happens at the wrong time.
>
> /P


Hi Paolo,


The two waits are very different. The xmit path holds tx_lock across
sk_stream_wait_memory(), which can sleep for an undetermined time (until
the peer reads):

   tls_device_sendmsg()
      mutex_lock(&tls_ctx->tx_lock);
      lock_sock(sk);
      tls_push_data()
        sk_stream_wait_memory()   <- releases sk lock, keeps tx_lock
      release_sock(sk);
      mutex_unlock(&tls_ctx->tx_lock);


So waiting for tx_lock with plain mutex_lock() can leave the process in

D state for a long time. The lock_sock() after it is fine: the sleeping

sender drops the socket lock, so that wait is only for short critical 
sections, never across the long sleep.


tls_sw_sendmsg() also uses  mutex_lock_interruptible but 
tls_device_sendmsg() still

uses plain mutex_lock() indeed, but that's another topic.