|Summary:||[patch] [linux] getsockopt(SOL_SOCKET, SO_PEERCRED) on a Unix socket fails silently|
|Product:||Base System||Reporter:||Dániel Bakai <bakaidl>|
|Component:||kern||Assignee:||Mark Johnston <markj>|
|Severity:||Affects Some People||CC:||bakaidl, markj|
Description Dániel Bakai 2019-01-07 23:16:47 UTC
Created attachment 200893 [details] Source for reproduction and patch When using the SO_PEERCRED getsockopt for a Unix socket from a Linux program with the Linux compatibility layer, it returns with no error, but the resulting struct ucred contains garbage. To reproduce, compile the attached linux_peercred_test.c for Linux, set up a listening Unix socket named test.sock on FreeBSD (nc -l -U test.sock or socat UNIX-LISTEN:test.sock -) and run linux_peercred_test. It will return something along the line of this: [bakaid@freebsd ~]$ ./linux_peercred_test uid: 4294965248, gid: 6, pid: 0 The cause seem to be the following: The Linux compat layer translates the Linux getsockopt for level SOL_SOCKET (1), optname SO_PEERCRED (17) to level SOL_SOCKET (0xffff), optname LOCAL_PEERCRED (1), and calls kern_getsockopt (sys/kern/uipc_syscalls.c) with these parameters (sys/compat/linux/linux_socket.c). However, LOCAL_PEERCRED should be called with level 0, not level SOL_SOCKET, as demonstrated (among others) by the getpeereid implementation (lib/libc/gen/getpeereid.c). Even worse, calling it with SOL_SOCKET will cause the getsockopt being served as a SOL_SOCKET request (kern_getsockopt -> sogetopt (sys/kern/uipc_socket.c)) and interpreted as SO_DEBUG (1), which will return successfully, causing the failure to be silent. I've only tried this on FreeBSD 12.0-RELEASE r341666 GENERIC amd64, but looking through the relevant git history I don't see positive evidence that this function ever worked properly. The attached patch (compat_linux_so_peercred_fix.patch) fixes the issue, which can be verified by running linux_peercred_test again: [bakaid@freebsd ~]$ ./linux_peercred_test uid: 1001, gid: 1001, pid: 0
Comment 1 Mark Johnston 2019-01-08 00:06:12 UTC
The analysis seems right to me. I think the patch should add a comment explaining why we ignore the BSD level - do you mind adding one? SO_PASSCRED would need similar handling if it were ever implemented, so a comment would be a nice hint.
Comment 2 Dániel Bakai 2019-01-08 00:38:52 UTC
Created attachment 200896 [details] Patch with added comments
Comment 3 Dániel Bakai 2019-01-08 00:41:55 UTC
(In reply to Mark Johnston from comment #1) Thank you for the quick response. You're right, it's better to comment this, I've added a new patch (compat_linux_so_peercred_fix_v2.patch) containing one, I hope it's correct.
Comment 4 commit-hook 2019-01-08 17:22:04 UTC
A commit references this bug: Author: markj Date: Tue Jan 8 17:21:59 UTC 2019 New revision: 342864 URL: https://svnweb.freebsd.org/changeset/base/342864 Log: Specify the correct option level when emulating SO_PEERCRED. Our equivalent to SO_PEERCRED, LOCAL_PEERCRED, is implemented at socket option level 0, not SOL_SOCKET. PR: 234722 Submitted by: D?niel Bakai <email@example.com> MFC after: 2 weeks Changes: head/sys/compat/linux/linux_socket.c
Comment 5 Mark Johnston 2019-01-08 17:23:26 UTC
(In reply to Dániel Bakai from comment #3) Thank you. I committed a minor variation of your patch to head.
Comment 6 commit-hook 2019-01-22 02:02:29 UTC
A commit references this bug: Author: markj Date: Tue Jan 22 02:02:13 UTC 2019 New revision: 343293 URL: https://svnweb.freebsd.org/changeset/base/343293 Log: MFC r342864: Specify the correct option level when emulating SO_PEERCRED. PR: 234722 Changes: _U stable/12/ stable/12/sys/compat/linux/linux_socket.c
Comment 7 commit-hook 2019-01-22 02:05:33 UTC
A commit references this bug: Author: markj Date: Tue Jan 22 02:04:37 UTC 2019 New revision: 343294 URL: https://svnweb.freebsd.org/changeset/base/343294 Log: MFC r342864: Specify the correct option level when emulating SO_PEERCRED. PR: 234722 Changes: _U stable/11/ stable/11/sys/compat/linux/linux_socket.c
Comment 8 Mark Johnston 2019-01-22 02:05:55 UTC
Thank you for the patch.