netdev - Re: [PATCH v1 net-next 14/15] af_unix: Remove sk_locked logic in unix_dgram

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] [day] [month] [year] [list]

Message-ID: <20241213082642.7931-1-kuniyu@amazon.com>
Date: Fri, 13 Dec 2024 17:26:42 +0900
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <pabeni@...hat.com>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<kuni1840@...il.com>, <kuniyu@...zon.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH v1 net-next 14/15] af_unix: Remove sk_locked logic in unix_dgram_sendmsg().

From: Paolo Abeni <pabeni@...hat.com>
Date: Tue, 10 Dec 2024 12:42:04 +0100
> On 12/6/24 06:26, Kuniyuki Iwashima wrote:
> > @@ -2136,27 +2133,21 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
> >  			goto restart;
> >  		}
> >  
> > -		if (!sk_locked) {
> > -			unix_state_unlock(other);
> > -			unix_state_double_lock(sk, other);
> > -		}
> > +		unix_state_unlock(other);
> > +		unix_state_double_lock(sk, other);
> >  
> >  		if (unix_peer(sk) != other ||
> > -		    unix_dgram_peer_wake_me(sk, other)) {
> > +		    unix_dgram_peer_wake_me(sk, other))
> >  			err = -EAGAIN;
> > -			sk_locked = 1;
> > +
> > +		unix_state_unlock(sk);
> > +
> > +		if (err)
> >  			goto out_unlock;
> > -		}
> >  
> > -		if (!sk_locked) {
> > -			sk_locked = 1;
> > -			goto restart_locked;
> > -		}
> > +		goto restart_locked;
> 
> I'm likely lost, but AFAICS the old code ensured that 'restart_locked'
> was attempted at most once, while now there is no such constraint. Can
> this loop forever under some not trivial condition?!?

Sorry, I forgot to restore if (!timeo) condition.

The at-most-once-loop was introduced by 7d267278a9ec that allows
us to queue skb for SOCK_DEAD socket..

I think this patch should be separated from this cleanup series.
Will drop this in v2.

Thanks!