This series fixes 2 issues introduced by commit 5e2ff6704a27 ("scm: add
SO_PASSPIDFD and SCM_PIDFD").
The 1st patch fixes a warning in scm_pidfd_recv() reported by syzkaller.
The 2nd patch fixes a regression that bluetooth can't be built as module.
Alexander Mikhalitsyn (1):
net: scm: introduce and use scm_recv_unix helper
Kuniyuki Iwashima (1):
af_unix: Skip SCM_PIDFD if scm->pid is NULL.
include/net/scm.h | 39 ++++++++++++++++++++++++++++-----------
net/unix/af_unix.c | 4 ++--
2 files changed, 30 insertions(+), 13 deletions(-)
--
2.30.2
From: Alexander Mikhalitsyn <[email protected]>
Recently, our friends from bluetooth subsystem reported [1] that after
commit 5e2ff6704a27 ("scm: add SO_PASSPIDFD and SCM_PIDFD") scm_recv()
helper become unusable in kernel modules (because it uses unexported
pidfd_prepare() API).
We were aware of this issue and workarounded it in a hard way
by commit 97154bcf4d1b ("af_unix: Kconfig: make CONFIG_UNIX bool").
But recently a new functionality was added in the scope of commit
817efd3cad74 ("Bluetooth: hci_sock: Forward credentials to monitor")
and after that bluetooth can't be compiled as a kernel module.
After some discussion in [1] we decided to split scm_recv() into
two helpers, one won't support SCM_PIDFD (used for unix sockets),
and another one will be completely the same as it was before commit
5e2ff6704a27 ("scm: add SO_PASSPIDFD and SCM_PIDFD").
Link: https://lore.kernel.org/lkml/CAJqdLrpFcga4n7wxBhsFqPQiN8PKFVr6U10fKcJ9W7AcZn+o6Q@mail.gmail.com/ [1]
Fixes: 5e2ff6704a27 ("scm: add SO_PASSPIDFD and SCM_PIDFD")
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
Reviewed-by: Kuniyuki Iwashima <[email protected]>
---
v2:
* Resolve conflict with https://lore.kernel.org/netdev/[email protected]/
* Add SHA1 in changelog
* Fix checkpatch warnings for mismatch open parenthesis and unwrapped link.
* Add Fixes and Reviewed-by tags
v1: https://lore.kernel.org/all/[email protected]/
---
include/net/scm.h | 35 +++++++++++++++++++++++++----------
net/unix/af_unix.c | 4 ++--
2 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/include/net/scm.h b/include/net/scm.h
index d456fc41b8bf..c5bcdf65f55c 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -153,8 +153,8 @@ static __inline__ void scm_pidfd_recv(struct msghdr *msg, struct scm_cookie *scm
fd_install(pidfd, pidfd_file);
}
-static __inline__ void scm_recv(struct socket *sock, struct msghdr *msg,
- struct scm_cookie *scm, int flags)
+static inline bool __scm_recv_common(struct socket *sock, struct msghdr *msg,
+ struct scm_cookie *scm, int flags)
{
if (!msg->msg_control) {
if (test_bit(SOCK_PASSCRED, &sock->flags) ||
@@ -162,7 +162,7 @@ static __inline__ void scm_recv(struct socket *sock, struct msghdr *msg,
scm->fp || scm_has_secdata(sock))
msg->msg_flags |= MSG_CTRUNC;
scm_destroy(scm);
- return;
+ return false;
}
if (test_bit(SOCK_PASSCRED, &sock->flags)) {
@@ -175,19 +175,34 @@ static __inline__ void scm_recv(struct socket *sock, struct msghdr *msg,
put_cmsg(msg, SOL_SOCKET, SCM_CREDENTIALS, sizeof(ucreds), &ucreds);
}
- if (test_bit(SOCK_PASSPIDFD, &sock->flags))
- scm_pidfd_recv(msg, scm);
+ scm_passec(sock, msg, scm);
- scm_destroy_cred(scm);
+ if (scm->fp)
+ scm_detach_fds(msg, scm);
- scm_passec(sock, msg, scm);
+ return true;
+}
- if (!scm->fp)
+static inline void scm_recv(struct socket *sock, struct msghdr *msg,
+ struct scm_cookie *scm, int flags)
+{
+ if (!__scm_recv_common(sock, msg, scm, flags))
return;
-
- scm_detach_fds(msg, scm);
+
+ scm_destroy_cred(scm);
}
+static inline void scm_recv_unix(struct socket *sock, struct msghdr *msg,
+ struct scm_cookie *scm, int flags)
+{
+ if (!__scm_recv_common(sock, msg, scm, flags))
+ return;
+
+ if (test_bit(SOCK_PASSPIDFD, &sock->flags))
+ scm_pidfd_recv(msg, scm);
+
+ scm_destroy_cred(scm);
+}
#endif /* __LINUX_NET_SCM_H */
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 3953daa2e1d0..123b35ddfd71 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2427,7 +2427,7 @@ int __unix_dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t size,
}
err = (flags & MSG_TRUNC) ? skb->len - skip : size;
- scm_recv(sock, msg, &scm, flags);
+ scm_recv_unix(sock, msg, &scm, flags);
out_free:
skb_free_datagram(sk, skb);
@@ -2808,7 +2808,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
mutex_unlock(&u->iolock);
if (state->msg)
- scm_recv(sock, state->msg, &scm, flags);
+ scm_recv_unix(sock, state->msg, &scm, flags);
else
scm_destroy(&scm);
out:
--
2.30.2
syzkaller hit a WARN_ON_ONCE(!scm->pid) in scm_pidfd_recv().
In unix_stream_read_generic(), if there is no skb in the queue, we could
bail out the do-while loop without calling scm_set_cred():
1. No skb in the queue
2. sk is non-blocking
or
shutdown(sk, RCV_SHUTDOWN) is called concurrently
or
peer calls close()
If the socket is configured with SO_PASSPIDFD, scm_pidfd_recv() would
populate cmsg with garbage emitting the warning.
Let's skip SCM_PIDFD if scm->pid is NULL in scm_pidfd_recv().
Note another way would be skip calling scm_recv() in such cases, but this
caused a regression resulting in commit 9d797ee2dce1 ("Revert "af_unix:
Call scm_recv() only after scm_set_cred()."").
WARNING: CPU: 1 PID: 3245 at include/net/scm.h:138 scm_pidfd_recv include/net/scm.h:138 [inline]
WARNING: CPU: 1 PID: 3245 at include/net/scm.h:138 scm_recv.constprop.0+0x754/0x850 include/net/scm.h:177
Modules linked in:
CPU: 1 PID: 3245 Comm: syz-executor.1 Not tainted 6.4.0-rc5-01219-gfa0e21fa4443 #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
RIP: 0010:scm_pidfd_recv include/net/scm.h:138 [inline]
RIP: 0010:scm_recv.constprop.0+0x754/0x850 include/net/scm.h:177
Code: 67 fd e9 55 fd ff ff e8 4a 70 67 fd e9 7f fd ff ff e8 40 70 67 fd e9 3e fb ff ff e8 36 70 67 fd e9 02 fd ff ff e8 8c 3a 20 fd <0f> 0b e9 fe fb ff ff e8 50 70 67 fd e9 2e f9 ff ff e8 46 70 67 fd
RSP: 0018:ffffc90009af7660 EFLAGS: 00010216
RAX: 00000000000000a1 RBX: ffff888041e58a80 RCX: ffffc90003852000
RDX: 0000000000040000 RSI: ffffffff842675b4 RDI: 0000000000000007
RBP: ffffc90009af7810 R08: 0000000000000007 R09: 0000000000000013
R10: 00000000000000f8 R11: 0000000000000001 R12: ffffc90009af7db0
R13: 0000000000000000 R14: ffff888041e58a88 R15: 1ffff9200135eecc
FS: 00007f6b7113f640(0000) GS:ffff88806cf00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f6b7111de38 CR3: 0000000012a6e002 CR4: 0000000000770ee0
PKRU: 55555554
Call Trace:
<TASK>
unix_stream_read_generic+0x5fe/0x1f50 net/unix/af_unix.c:2830
unix_stream_recvmsg+0x194/0x1c0 net/unix/af_unix.c:2880
sock_recvmsg_nosec net/socket.c:1019 [inline]
sock_recvmsg+0x188/0x1d0 net/socket.c:1040
____sys_recvmsg+0x210/0x610 net/socket.c:2712
___sys_recvmsg+0xff/0x190 net/socket.c:2754
do_recvmmsg+0x25d/0x6c0 net/socket.c:2848
__sys_recvmmsg net/socket.c:2927 [inline]
__do_sys_recvmmsg net/socket.c:2950 [inline]
__se_sys_recvmmsg net/socket.c:2943 [inline]
__x64_sys_recvmmsg+0x224/0x290 net/socket.c:2943
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3f/0x90 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x72/0xdc
RIP: 0033:0x7f6b71da2e5d
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 73 9f 1b 00 f7 d8 64 89 01 48
RSP: 002b:00007f6b7113ecc8 EFLAGS: 00000246 ORIG_RAX: 000000000000012b
RAX: ffffffffffffffda RBX: 00000000004bc050 RCX: 00007f6b71da2e5d
RDX: 0000000000000007 RSI: 0000000020006600 RDI: 000000000000000b
RBP: 00000000004bc050 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000120 R11: 0000000000000246 R12: 0000000000000000
R13: 000000000000006e R14: 00007f6b71e03530 R15: 0000000000000000
</TASK>
Fixes: 5e2ff6704a27 ("scm: add SO_PASSPIDFD and SCM_PIDFD")
Reported-by: syzkaller <[email protected]>
Signed-off-by: Kuniyuki Iwashima <[email protected]>
---
include/net/scm.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/net/scm.h b/include/net/scm.h
index c67f765a165b..d456fc41b8bf 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -135,7 +135,9 @@ static __inline__ void scm_pidfd_recv(struct msghdr *msg, struct scm_cookie *scm
return;
}
- WARN_ON_ONCE(!scm->pid);
+ if (!scm->pid)
+ return;
+
pidfd = pidfd_prepare(scm->pid, 0, &pidfd_file);
if (put_cmsg(msg, SOL_SOCKET, SCM_PIDFD, sizeof(int), &pidfd)) {
--
2.30.2
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <[email protected]>:
On Tue, 27 Jun 2023 10:43:12 -0700 you wrote:
> This series fixes 2 issues introduced by commit 5e2ff6704a27 ("scm: add
> SO_PASSPIDFD and SCM_PIDFD").
>
> The 1st patch fixes a warning in scm_pidfd_recv() reported by syzkaller.
> The 2nd patch fixes a regression that bluetooth can't be built as module.
>
>
> [...]
Here is the summary with links:
- [v1,net-next,1/2] af_unix: Skip SCM_PIDFD if scm->pid is NULL.
https://git.kernel.org/netdev/net-next/c/603fc57ab70c
- [v1,net-next,2/2] net: scm: introduce and use scm_recv_unix helper
https://git.kernel.org/netdev/net-next/c/a9c49cc2f5b5
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=760703
---Test result---
Test Summary:
CheckPatch FAIL 1.86 seconds
GitLint FAIL 0.98 seconds
SubjectPrefix FAIL 0.52 seconds
BuildKernel PASS 32.39 seconds
CheckAllWarning PASS 35.01 seconds
CheckSparse PASS 39.57 seconds
CheckSmatch PASS 112.14 seconds
BuildKernel32 PASS 31.26 seconds
TestRunnerSetup PASS 442.82 seconds
TestRunner_l2cap-tester PASS 16.57 seconds
TestRunner_iso-tester PASS 22.53 seconds
TestRunner_bnep-tester PASS 5.30 seconds
TestRunner_mgmt-tester PASS 128.55 seconds
TestRunner_rfcomm-tester PASS 8.45 seconds
TestRunner_sco-tester PASS 7.85 seconds
TestRunner_ioctl-tester PASS 9.22 seconds
TestRunner_mesh-tester PASS 6.67 seconds
TestRunner_smp-tester PASS 7.81 seconds
TestRunner_userchan-tester PASS 5.58 seconds
IncrementalBuild PASS 56.70 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[v1,net-next,1/2] af_unix: Skip SCM_PIDFD if scm->pid is NULL.
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#148:
Reported-by: syzkaller <[email protected]>
Signed-off-by: Kuniyuki Iwashima <[email protected]>
total: 0 errors, 1 warnings, 10 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13294846.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v1,net-next,1/2] af_unix: Skip SCM_PIDFD if scm->pid is NULL.
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T3 Title has trailing punctuation (.): "[v1,net-next,1/2] af_unix: Skip SCM_PIDFD if scm->pid is NULL."
24: B1 Line exceeds max length (96>80): "WARNING: CPU: 1 PID: 3245 at include/net/scm.h:138 scm_pidfd_recv include/net/scm.h:138 [inline]"
25: B1 Line exceeds max length (105>80): "WARNING: CPU: 1 PID: 3245 at include/net/scm.h:138 scm_recv.constprop.0+0x754/0x850 include/net/scm.h:177"
27: B1 Line exceeds max length (82>80): "CPU: 1 PID: 3245 Comm: syz-executor.1 Not tainted 6.4.0-rc5-01219-gfa0e21fa4443 #2"
28: B1 Line exceeds max length (115>80): "Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014"
31: B1 Line exceeds max length (199>80): "Code: 67 fd e9 55 fd ff ff e8 4a 70 67 fd e9 7f fd ff ff e8 40 70 67 fd e9 3e fb ff ff e8 36 70 67 fd e9 02 fd ff ff e8 8c 3a 20 fd <0f> 0b e9 fe fb ff ff e8 50 70 67 fd e9 2e f9 ff ff e8 46 70 67 fd"
59: B1 Line exceeds max length (199>80): "Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 73 9f 1b 00 f7 d8 64 89 01 48"
[v1,net-next,2/2] net: scm: introduce and use scm_recv_unix helper
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
22: B1 Line exceeds max length (106>80): "Link: https://lore.kernel.org/lkml/CAJqdLrpFcga4n7wxBhsFqPQiN8PKFVr6U10fKcJ9W7AcZn+o6Q@mail.gmail.com/ [1]"
27: B1 Line exceeds max length (98>80): " * Resolve conflict with https://lore.kernel.org/netdev/[email protected]/"
32: B1 Line exceeds max length (92>80): "v1: https://lore.kernel.org/all/[email protected]/"
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
"Bluetooth: " prefix is not specified in the subject
---
Regards,
Linux Bluetooth
Hello:
This series was applied to bluetooth/bluetooth-next.git (master)
by Jakub Kicinski <[email protected]>:
On Tue, 27 Jun 2023 10:43:12 -0700 you wrote:
> This series fixes 2 issues introduced by commit 5e2ff6704a27 ("scm: add
> SO_PASSPIDFD and SCM_PIDFD").
>
> The 1st patch fixes a warning in scm_pidfd_recv() reported by syzkaller.
> The 2nd patch fixes a regression that bluetooth can't be built as module.
>
>
> [...]
Here is the summary with links:
- [v1,net-next,1/2] af_unix: Skip SCM_PIDFD if scm->pid is NULL.
https://git.kernel.org/bluetooth/bluetooth-next/c/603fc57ab70c
- [v1,net-next,2/2] net: scm: introduce and use scm_recv_unix helper
https://git.kernel.org/bluetooth/bluetooth-next/c/a9c49cc2f5b5
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html