| Message ID | 20250326150148.72d9138d@canb.auug.org.au (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | linux-next: build failure after merge of the apparmor tree | expand |
On Wed, 26 Mar 2025 15:01:48 +1100 Stephen Rothwell wrote: > After merging the apparmor tree, today's linux-next build (x86_64 > allmodconfig) failed like this: > > security/apparmor/af_unix.c: In function 'unix_state_double_lock': > security/apparmor/af_unix.c:627:17: error: implicit declaration of function 'unix_state_lock'; did you mean 'unix_state_double_lock'? [-Wimplicit-function-declaration] > 627 | unix_state_lock(sk1); > | ^~~~~~~~~~~~~~~ > | unix_state_double_lock > security/apparmor/af_unix.c: In function 'unix_state_double_unlock': > security/apparmor/af_unix.c:642:17: error: implicit declaration of function 'unix_state_unlock'; did you mean 'unix_state_double_lock'? [-Wimplicit-function-declaration] > 642 | unix_state_unlock(sk1); > | ^~~~~~~~~~~~~~~~~ > | unix_state_double_lock Thanks Stephen! I'll pop this into the tree in a few hours, just giving Kuniyuki a bit more time to ack.
From: Jakub Kicinski <kuba@kernel.org> Date: Wed, 26 Mar 2025 04:26:55 -0700 > On Wed, 26 Mar 2025 15:01:48 +1100 Stephen Rothwell wrote: > > After merging the apparmor tree, today's linux-next build (x86_64 > > allmodconfig) failed like this: > > > > security/apparmor/af_unix.c: In function 'unix_state_double_lock': > > security/apparmor/af_unix.c:627:17: error: implicit declaration of function 'unix_state_lock'; did you mean 'unix_state_double_lock'? [-Wimplicit-function-declaration] > > 627 | unix_state_lock(sk1); > > | ^~~~~~~~~~~~~~~ > > | unix_state_double_lock > > security/apparmor/af_unix.c: In function 'unix_state_double_unlock': > > security/apparmor/af_unix.c:642:17: error: implicit declaration of function 'unix_state_unlock'; did you mean 'unix_state_double_lock'? [-Wimplicit-function-declaration] > > 642 | unix_state_unlock(sk1); > > | ^~~~~~~~~~~~~~~~~ > > | unix_state_double_lock > > Thanks Stephen! I'll pop this into the tree in a few hours, > just giving Kuniyuki a bit more time to ack. Thanks for catching this, Stephen ! The patch itself looks good, for the patch: Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com> John: I had a cursory look at this commit and the exact user of unix_state_lock() is broken for SOCK_DGRAM. https://web.git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor.git/commit/?h=apparmor-next&id=c05e705812d179f4b85aeacc34a555a42bc4f9ac ---8<--- + + /* TODO: update sock label with new task label */ + unix_state_lock(sock->sk); + peer_sk = unix_peer(sock->sk); + if (peer_sk) + sock_hold(peer_sk); + + is_sk_fs = is_unix_fs(sock->sk); + if (is_sk_fs && peer_sk) + sk_req = request; + if (sk_req) + error = unix_label_sock_perm(subj_cred, label, op, sk_req, + sock); + unix_state_unlock(sock->sk); + if (!peer_sk) + return error; + + unix_state_double_lock(sock->sk, peer_sk); Here, unix_peer(sock->sk) could have been changed and must be double checked. See unix_dgram_sendmsg(). The patch seems to be written in 2022 and recently merged. I'm not sure if it's reviewed by netdev folks at that time, but please cc me and netdev next time for patches regarding AF_UNIX. Thanks! + if (!is_sk_fs && is_unix_fs(peer_sk)) { + last_error(error, + unix_fs_perm(op, request, subj_cred, label, + unix_sk(peer_sk))); + } else if (!is_sk_fs) { + struct aa_sk_ctx *pctx = aa_sock(peer_sk); + + last_error(error, + xcheck(aa_unix_peer_perm(subj_cred, label, op, + MAY_READ | MAY_WRITE, + sock->sk, peer_sk, NULL), + aa_unix_peer_perm(file->f_cred, pctx->label, op, + MAY_READ | MAY_WRITE, + peer_sk, sock->sk, label))); + } + unix_state_double_unlock(sock->sk, peer_sk); ---8<---
On Wed, 26 Mar 2025 15:01:48 +1100 Stephen Rothwell wrote: > From: Stephen Rothwell <sfr@canb.auug.org.au> > Date: Wed, 26 Mar 2025 14:31:44 +1100 > Subject: [PATCH] fix up for "apparmor: add fine grained af_unix mediation" > > interacting with "af_unix: Move internal definitions to net/unix/." > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> Applied to net-next, and included in our 6.15 PR. Thanks!
diff --git a/include/net/af_unix.h b/include/net/af_unix.h index b588069ece7e..1af1841b7601 100644 --- a/include/net/af_unix.h +++ b/include/net/af_unix.h @@ -55,4 +55,7 @@ struct unix_sock { #define unix_sk(ptr) container_of_const(ptr, struct unix_sock, sk) #define unix_peer(sk) (unix_sk(sk)->peer) +#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock) +#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock) + #endif diff --git a/net/unix/af_unix.h b/net/unix/af_unix.h index ed4aedc42813..59db179df9bb 100644 --- a/net/unix/af_unix.h +++ b/net/unix/af_unix.h @@ -8,9 +8,6 @@ #define UNIX_HASH_SIZE (256 * 2) #define UNIX_HASH_BITS 8 -#define unix_state_lock(s) spin_lock(&unix_sk(s)->lock) -#define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock) - struct sock *unix_peer_get(struct sock *sk); struct unix_skb_parms {
Hi all, After merging the apparmor tree, today's linux-next build (x86_64 allmodconfig) failed like this: security/apparmor/af_unix.c: In function 'unix_state_double_lock': security/apparmor/af_unix.c:627:17: error: implicit declaration of function 'unix_state_lock'; did you mean 'unix_state_double_lock'? [-Wimplicit-function-declaration] 627 | unix_state_lock(sk1); | ^~~~~~~~~~~~~~~ | unix_state_double_lock security/apparmor/af_unix.c: In function 'unix_state_double_unlock': security/apparmor/af_unix.c:642:17: error: implicit declaration of function 'unix_state_unlock'; did you mean 'unix_state_double_lock'? [-Wimplicit-function-declaration] 642 | unix_state_unlock(sk1); | ^~~~~~~~~~~~~~~~~ | unix_state_double_lock Caused by commit c05e705812d1 ("apparmor: add fine grained af_unix mediation") interacting with commit 84960bf24031 ("af_unix: Move internal definitions to net/unix/.") from the net-next tree. I have applied the following patch for today. From: Stephen Rothwell <sfr@canb.auug.org.au> Date: Wed, 26 Mar 2025 14:31:44 +1100 Subject: [PATCH] fix up for "apparmor: add fine grained af_unix mediation" interacting with "af_unix: Move internal definitions to net/unix/." Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> --- include/net/af_unix.h | 3 +++ net/unix/af_unix.h | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-)