[PATCH v1 net-next] Revert "af_unix: Call scm_recv() only after scm_set_cred()."

[PATCH v1 net-next] Revert "af_unix: Call scm_recv() only after scm_set_cred()."

[Kuniyuki Iwashima]

This reverts commit 3f5f118bb657f94641ea383c7c1b8c09a5d46ea2.

Konrad reported that desktop environment below cannot be reached after
commit 3f5f118bb657 ("af_unix: Call scm_recv() only after scm_set_cred().")

  - postmarketOS (Alpine Linux w/ musl 1.2.4)
  - busybox 1.36.1
  - GNOME 44.1
  - networkmanager 1.42.6
  - openrc 0.47

Regarding to the warning of SO_PASSPIDFD, I'll post another patch to
suppress it by skipping SCM_PIDFD if scm->pid == NULL in scm_pidfd_recv().

Reported-by: Konrad Dybcio <konradybcio@kernel.org>
Link: https://lore.kernel.org/netdev/8c7f9abd-4f84-7296-2788-1e130d6304a0@kernel.org/
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
To maintainers

Sorry for bothering, but can this make it to the v6.5 train to
avoid regression reports ?
---
 net/unix/af_unix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f2f234f0b92c..3953daa2e1d0 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2807,7 +2807,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 	} while (size);
 
 	mutex_unlock(&u->iolock);
-	if (state->msg && check_creds)
+	if (state->msg)
 		scm_recv(sock, state->msg, &scm, flags);
 	else
 		scm_destroy(&scm);
-- 
2.30.2

Re: [PATCH v1 net-next] Revert "af_unix: Call scm_recv() only after scm_set_cred()."

[Ido Schimmel]

On Mon, Jun 26, 2023 at 01:58:37PM -0700, Kuniyuki Iwashima wrote:
> This reverts commit 3f5f118bb657f94641ea383c7c1b8c09a5d46ea2.
> 
> Konrad reported that desktop environment below cannot be reached after
> commit 3f5f118bb657 ("af_unix: Call scm_recv() only after scm_set_cred().")
> 
>   - postmarketOS (Alpine Linux w/ musl 1.2.4)
>   - busybox 1.36.1
>   - GNOME 44.1
>   - networkmanager 1.42.6
>   - openrc 0.47
> 
> Regarding to the warning of SO_PASSPIDFD, I'll post another patch to
> suppress it by skipping SCM_PIDFD if scm->pid == NULL in scm_pidfd_recv().
> 
> Reported-by: Konrad Dybcio <konradybcio@kernel.org>
> Link: https://lore.kernel.org/netdev/8c7f9abd-4f84-7296-2788-1e130d6304a0@kernel.org/
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Tested-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH v1 net-next] Revert "af_unix: Call scm_recv() only after scm_set_cred()."

[Jiri Olsa]

On Mon, Jun 26, 2023 at 01:58:37PM -0700, Kuniyuki Iwashima wrote:
> This reverts commit 3f5f118bb657f94641ea383c7c1b8c09a5d46ea2.
> 
> Konrad reported that desktop environment below cannot be reached after
> commit 3f5f118bb657 ("af_unix: Call scm_recv() only after scm_set_cred().")
> 
>   - postmarketOS (Alpine Linux w/ musl 1.2.4)
>   - busybox 1.36.1
>   - GNOME 44.1
>   - networkmanager 1.42.6
>   - openrc 0.47
> 
> Regarding to the warning of SO_PASSPIDFD, I'll post another patch to
> suppress it by skipping SCM_PIDFD if scm->pid == NULL in scm_pidfd_recv().
> 
> Reported-by: Konrad Dybcio <konradybcio@kernel.org>
> Link: https://lore.kernel.org/netdev/8c7f9abd-4f84-7296-2788-1e130d6304a0@kernel.org/
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

hit the same issue, this revert fixes it for me, fwiw:

Teste-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
> To maintainers
> 
> Sorry for bothering, but can this make it to the v6.5 train to
> avoid regression reports ?
> ---
>  net/unix/af_unix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index f2f234f0b92c..3953daa2e1d0 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -2807,7 +2807,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
>  	} while (size);
>  
>  	mutex_unlock(&u->iolock);
> -	if (state->msg && check_creds)
> +	if (state->msg)
>  		scm_recv(sock, state->msg, &scm, flags);
>  	else
>  		scm_destroy(&scm);
> -- 
> 2.30.2
> 

Re: [PATCH v1 net-next] Revert "af_unix: Call scm_recv() only after scm_set_cred()."

[Gal Pressman]

On 26/06/2023 23:58, Kuniyuki Iwashima wrote:
> This reverts commit 3f5f118bb657f94641ea383c7c1b8c09a5d46ea2.
> 
> Konrad reported that desktop environment below cannot be reached after
> commit 3f5f118bb657 ("af_unix: Call scm_recv() only after scm_set_cred().")
> 
>   - postmarketOS (Alpine Linux w/ musl 1.2.4)
>   - busybox 1.36.1
>   - GNOME 44.1
>   - networkmanager 1.42.6
>   - openrc 0.47
> 
> Regarding to the warning of SO_PASSPIDFD, I'll post another patch to
> suppress it by skipping SCM_PIDFD if scm->pid == NULL in scm_pidfd_recv().
> 
> Reported-by: Konrad Dybcio <konradybcio@kernel.org>
> Link: https://lore.kernel.org/netdev/8c7f9abd-4f84-7296-2788-1e130d6304a0@kernel.org/
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Encountered the same issue.

Tested-by: Gal Pressman <gal@nvidia.com>

Re: [PATCH v1 net-next] Revert "af_unix: Call scm_recv() only after scm_set_cred()."

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 26 Jun 2023 13:58:37 -0700 you wrote:
> This reverts commit 3f5f118bb657f94641ea383c7c1b8c09a5d46ea2.
> 
> Konrad reported that desktop environment below cannot be reached after
> commit 3f5f118bb657 ("af_unix: Call scm_recv() only after scm_set_cred().")
> 
>   - postmarketOS (Alpine Linux w/ musl 1.2.4)
>   - busybox 1.36.1
>   - GNOME 44.1
>   - networkmanager 1.42.6
>   - openrc 0.47
> 
> [...]

Here is the summary with links:
  - [v1,net-next] Revert "af_unix: Call scm_recv() only after scm_set_cred()."
    https://git.kernel.org/netdev/net-next/c/9d797ee2dce1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v1 net-next] Revert "af_unix: Call scm_recv() only after scm_set_cred()."

[Jakub Kicinski]

On Mon, 26 Jun 2023 13:58:37 -0700 Kuniyuki Iwashima wrote:
> Regarding to the warning of SO_PASSPIDFD, I'll post another patch to
> suppress it by skipping SCM_PIDFD if scm->pid == NULL in scm_pidfd_recv().

> Sorry for bothering, but can this make it to the v6.5 train to
> avoid regression reports ?

I'm merging the trees now, could you send the follow up soon?
Could you respin Alexander's fix for me as well (make it a two patch
series?):
https://lore.kernel.org/all/20230626215951.563715-1-aleksandr.mikhalitsyn@canonical.com/
It trivially conflicts after applying this patch.

Re: [PATCH v1 net-next] Revert "af_unix: Call scm_recv() only after scm_set_cred()."

[Kuniyuki Iwashima]

From: Jakub Kicinski <kuba@kernel.org>
Date: Tue, 27 Jun 2023 09:42:45 -0700
> On Mon, 26 Jun 2023 13:58:37 -0700 Kuniyuki Iwashima wrote:
> > Regarding to the warning of SO_PASSPIDFD, I'll post another patch to
> > suppress it by skipping SCM_PIDFD if scm->pid == NULL in scm_pidfd_recv().
> 
> > Sorry for bothering, but can this make it to the v6.5 train to
> > avoid regression reports ?
> 
> I'm merging the trees now, could you send the follow up soon?
> Could you respin Alexander's fix for me as well (make it a two patch
> series?):
> https://lore.kernel.org/all/20230626215951.563715-1-aleksandr.mikhalitsyn@canonical.com/
> It trivially conflicts after applying this patch.

I see.  I'll post 2 patches soon.

Thank you, Jakub!