netdev - Re: [PATCH v1 net-next 04/15] af_unix: Remove redundant SEND_SHUTDOWN check in unix_stream

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

Message-ID: <20241213081817.6524-1-kuniyu@amazon.com>
Date: Fri, 13 Dec 2024 17:18:17 +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 04/15] af_unix: Remove redundant SEND_SHUTDOWN check in unix_stream_sendmsg().

Sorry for the delay!

From: Paolo Abeni <pabeni@...hat.com>
Date: Tue, 10 Dec 2024 12:48:53 +0100
> On 12/6/24 06:25, Kuniyuki Iwashima wrote:
> > sock_alloc_send_pskb() in the following while loop checks if
> > SEND_SHUTDOWN is set to sk->sk_shutdown.
> > 
> > Let's remove the redundant check in unix_stream_sendmsg().
> 
> If socket error is != 0, the user shutsdown for write and than does a
> (stream) sendmsg, AFAICS prior to this patch it will get a piper error,
> but now it will get the socket error.
> 
> I'm unsure if we should preserve the old behavior, weird applications
> could rely on that ?!? usually there are more weird applications around
> that what I suspect.

Ah, you're right.

When the peer is closed, -ECONNRESET is set to sk_err.
Then, sendmsg() will return it, but even in that case,
we currently return -EPIPE for the peer's SOCK_DEAD.

So the current app can live without -ECONNRESET :)


> 
> At least the behavior change should be noted. If it does not impact too
> much the series and drop reasons addition, perhaps just drop this
> cleanup? (Assuming my initial statement is correct).

Will drop this patch, thanks!