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

[Jiayuan Chen]

在 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.