From: Greg Kroah-Hartman Date: Mon, 11 Dec 2023 13:32:38 +0000 (+0100) Subject: 5.4-stable patches X-Git-Tag: v4.14.333~30 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=07175a6f30c47ac4671737d281023af70ea0f964;p=thirdparty%2Fkernel%2Fstable-queue.git 5.4-stable patches added patches: drop_monitor-require-cap_sys_admin-when-joining-events-group.patch genetlink-add-cap_net_admin-test-for-multicast-bind.patch netlink-don-t-call-netlink_bind-with-table-lock-held.patch psample-require-cap_net_admin-when-joining-packets-group.patch --- diff --git a/queue-5.4/drop_monitor-require-cap_sys_admin-when-joining-events-group.patch b/queue-5.4/drop_monitor-require-cap_sys_admin-when-joining-events-group.patch new file mode 100644 index 00000000000..0cfe8bb5197 --- /dev/null +++ b/queue-5.4/drop_monitor-require-cap_sys_admin-when-joining-events-group.patch @@ -0,0 +1,158 @@ +From stable+bounces-5302-greg=kroah.com@vger.kernel.org Mon Dec 11 13:42:35 2023 +From: Ido Schimmel +Date: Mon, 11 Dec 2023 14:41:33 +0200 +Subject: drop_monitor: Require 'CAP_SYS_ADMIN' when joining "events" group +To: +Cc: , , , , , , , , , +Message-ID: <20231211124133.822891-5-idosch@nvidia.com> + +From: Ido Schimmel + +commit e03781879a0d524ce3126678d50a80484a513c4b upstream. + +The "NET_DM" generic netlink family notifies drop locations over the +"events" multicast group. This is problematic since by default generic +netlink allows non-root users to listen to these notifications. + +Fix by adding a new field to the generic netlink multicast group +structure that when set prevents non-root users or root without the +'CAP_SYS_ADMIN' capability (in the user namespace owning the network +namespace) from joining the group. Set this field for the "events" +group. Use 'CAP_SYS_ADMIN' rather than 'CAP_NET_ADMIN' because of the +nature of the information that is shared over this group. + +Note that the capability check in this case will always be performed +against the initial user namespace since the family is not netns aware +and only operates in the initial network namespace. + +A new field is added to the structure rather than using the "flags" +field because the existing field uses uAPI flags and it is inappropriate +to add a new uAPI flag for an internal kernel check. In net-next we can +rework the "flags" field to use internal flags and fold the new field +into it. But for now, in order to reduce the amount of changes, add a +new field. + +Since the information can only be consumed by root, mark the control +plane operations that start and stop the tracing as root-only using the +'GENL_ADMIN_PERM' flag. + +Tested using [1]. + +Before: + + # capsh -- -c ./dm_repo + # capsh --drop=cap_sys_admin -- -c ./dm_repo + +After: + + # capsh -- -c ./dm_repo + # capsh --drop=cap_sys_admin -- -c ./dm_repo + Failed to join "events" multicast group + +[1] + $ cat dm.c + #include + #include + #include + #include + + int main(int argc, char **argv) + { + struct nl_sock *sk; + int grp, err; + + sk = nl_socket_alloc(); + if (!sk) { + fprintf(stderr, "Failed to allocate socket\n"); + return -1; + } + + err = genl_connect(sk); + if (err) { + fprintf(stderr, "Failed to connect socket\n"); + return err; + } + + grp = genl_ctrl_resolve_grp(sk, "NET_DM", "events"); + if (grp < 0) { + fprintf(stderr, + "Failed to resolve \"events\" multicast group\n"); + return grp; + } + + err = nl_socket_add_memberships(sk, grp, NFNLGRP_NONE); + if (err) { + fprintf(stderr, "Failed to join \"events\" multicast group\n"); + return err; + } + + return 0; + } + $ gcc -I/usr/include/libnl3 -lnl-3 -lnl-genl-3 -o dm_repo dm.c + +Fixes: 9a8afc8d3962 ("Network Drop Monitor: Adding drop monitor implementation & Netlink protocol") +Reported-by: "The UK's National Cyber Security Centre (NCSC)" +Signed-off-by: Ido Schimmel +Reviewed-by: Jacob Keller +Reviewed-by: Jiri Pirko +Link: https://lore.kernel.org/r/20231206213102.1824398-3-idosch@nvidia.com +Signed-off-by: Jakub Kicinski +Signed-off-by: Greg Kroah-Hartman +--- + include/net/genetlink.h | 2 ++ + net/core/drop_monitor.c | 4 +++- + net/netlink/genetlink.c | 3 +++ + 3 files changed, 8 insertions(+), 1 deletion(-) + +--- a/include/net/genetlink.h ++++ b/include/net/genetlink.h +@@ -11,10 +11,12 @@ + /** + * struct genl_multicast_group - generic netlink multicast group + * @name: name of the multicast group, names are per-family ++ * @cap_sys_admin: whether %CAP_SYS_ADMIN is required for binding + */ + struct genl_multicast_group { + char name[GENL_NAMSIZ]; + u8 flags; ++ u8 cap_sys_admin:1; + }; + + struct genl_ops; +--- a/net/core/drop_monitor.c ++++ b/net/core/drop_monitor.c +@@ -180,7 +180,7 @@ out: + } + + static const struct genl_multicast_group dropmon_mcgrps[] = { +- { .name = "events", }, ++ { .name = "events", .cap_sys_admin = 1 }, + }; + + static void send_dm_alert(struct work_struct *work) +@@ -1539,11 +1539,13 @@ static const struct genl_ops dropmon_ops + .cmd = NET_DM_CMD_START, + .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, + .doit = net_dm_cmd_trace, ++ .flags = GENL_ADMIN_PERM, + }, + { + .cmd = NET_DM_CMD_STOP, + .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, + .doit = net_dm_cmd_trace, ++ .flags = GENL_ADMIN_PERM, + }, + { + .cmd = NET_DM_CMD_CONFIG_GET, +--- a/net/netlink/genetlink.c ++++ b/net/netlink/genetlink.c +@@ -1012,6 +1012,9 @@ static int genl_bind(struct net *net, in + if ((grp->flags & GENL_UNS_ADMIN_PERM) && + !ns_capable(net->user_ns, CAP_NET_ADMIN)) + ret = -EPERM; ++ if (grp->cap_sys_admin && ++ !ns_capable(net->user_ns, CAP_SYS_ADMIN)) ++ ret = -EPERM; + + break; + } diff --git a/queue-5.4/genetlink-add-cap_net_admin-test-for-multicast-bind.patch b/queue-5.4/genetlink-add-cap_net_admin-test-for-multicast-bind.patch new file mode 100644 index 00000000000..0935e2c6ac7 --- /dev/null +++ b/queue-5.4/genetlink-add-cap_net_admin-test-for-multicast-bind.patch @@ -0,0 +1,94 @@ +From stable+bounces-5300-greg=kroah.com@vger.kernel.org Mon Dec 11 13:42:25 2023 +From: Ido Schimmel +Date: Mon, 11 Dec 2023 14:41:31 +0200 +Subject: genetlink: add CAP_NET_ADMIN test for multicast bind +To: +Cc: , , , , , , , , , +Message-ID: <20231211124133.822891-3-idosch@nvidia.com> + +From: Ido Schimmel + +This is a partial backport of upstream commit 4d54cc32112d ("mptcp: +avoid lock_fast usage in accept path"). It is only a partial backport +because the patch in the link below was erroneously squash-merged into +upstream commit 4d54cc32112d ("mptcp: avoid lock_fast usage in accept +path"). Below is the original patch description from Florian Westphal: + +" +genetlink sets NL_CFG_F_NONROOT_RECV for its netlink socket so anyone can +subscribe to multicast messages. + +rtnetlink doesn't allow this unconditionally, rtnetlink_bind() restricts +bind requests to CAP_NET_ADMIN for a few groups. + +This allows to set GENL_UNS_ADMIN_PERM flag on genl mcast groups to +mandate CAP_NET_ADMIN. + +This will be used by the upcoming mptcp netlink event facility which +exposes the token (mptcp connection identifier) to userspace. +" + +Link: https://lore.kernel.org/mptcp/20210213000001.379332-8-mathew.j.martineau@linux.intel.com/ +Signed-off-by: Ido Schimmel +Signed-off-by: Greg Kroah-Hartman +--- + include/net/genetlink.h | 1 + + net/netlink/genetlink.c | 32 ++++++++++++++++++++++++++++++++ + 2 files changed, 33 insertions(+) + +--- a/include/net/genetlink.h ++++ b/include/net/genetlink.h +@@ -14,6 +14,7 @@ + */ + struct genl_multicast_group { + char name[GENL_NAMSIZ]; ++ u8 flags; + }; + + struct genl_ops; +--- a/net/netlink/genetlink.c ++++ b/net/netlink/genetlink.c +@@ -989,11 +989,43 @@ static struct genl_family genl_ctrl __ro + .netnsok = true, + }; + ++static int genl_bind(struct net *net, int group) ++{ ++ const struct genl_family *family; ++ unsigned int id; ++ int ret = 0; ++ ++ genl_lock_all(); ++ ++ idr_for_each_entry(&genl_fam_idr, family, id) { ++ const struct genl_multicast_group *grp; ++ int i; ++ ++ if (family->n_mcgrps == 0) ++ continue; ++ ++ i = group - family->mcgrp_offset; ++ if (i < 0 || i >= family->n_mcgrps) ++ continue; ++ ++ grp = &family->mcgrps[i]; ++ if ((grp->flags & GENL_UNS_ADMIN_PERM) && ++ !ns_capable(net->user_ns, CAP_NET_ADMIN)) ++ ret = -EPERM; ++ ++ break; ++ } ++ ++ genl_unlock_all(); ++ return ret; ++} ++ + static int __net_init genl_pernet_init(struct net *net) + { + struct netlink_kernel_cfg cfg = { + .input = genl_rcv, + .flags = NL_CFG_F_NONROOT_RECV, ++ .bind = genl_bind, + }; + + /* we'll bump the group number right afterwards */ diff --git a/queue-5.4/netlink-don-t-call-netlink_bind-with-table-lock-held.patch b/queue-5.4/netlink-don-t-call-netlink_bind-with-table-lock-held.patch new file mode 100644 index 00000000000..bbe9b1cee7c --- /dev/null +++ b/queue-5.4/netlink-don-t-call-netlink_bind-with-table-lock-held.patch @@ -0,0 +1,97 @@ +From stable+bounces-5299-greg=kroah.com@vger.kernel.org Mon Dec 11 13:42:24 2023 +From: Ido Schimmel +Date: Mon, 11 Dec 2023 14:41:30 +0200 +Subject: netlink: don't call ->netlink_bind with table lock held +To: +Cc: , , , , , , , , , +Message-ID: <20231211124133.822891-2-idosch@nvidia.com> + +From: Ido Schimmel + +From: Florian Westphal + +commit f2764bd4f6a8dffaec3e220728385d9756b3c2cb upstream. + +When I added support to allow generic netlink multicast groups to be +restricted to subscribers with CAP_NET_ADMIN I was unaware that a +genl_bind implementation already existed in the past. + +It was reverted due to ABBA deadlock: + +1. ->netlink_bind gets called with the table lock held. +2. genetlink bind callback is invoked, it grabs the genl lock. + +But when a new genl subsystem is (un)registered, these two locks are +taken in reverse order. + +One solution would be to revert again and add a comment in genl +referring 1e82a62fec613, "genetlink: remove genl_bind"). + +This would need a second change in mptcp to not expose the raw token +value anymore, e.g. by hashing the token with a secret key so userspace +can still associate subflow events with the correct mptcp connection. + +However, Paolo Abeni reminded me to double-check why the netlink table is +locked in the first place. + +I can't find one. netlink_bind() is already called without this lock +when userspace joins a group via NETLINK_ADD_MEMBERSHIP setsockopt. +Same holds for the netlink_unbind operation. + +Digging through the history, commit f773608026ee1 +("netlink: access nlk groups safely in netlink bind and getname") +expanded the lock scope. + +commit 3a20773beeeeade ("net: netlink: cap max groups which will be considered in netlink_bind()") +... removed the nlk->ngroups access that the lock scope +extension was all about. + +Reduce the lock scope again and always call ->netlink_bind without +the table lock. + +The Fixes tag should be vs. the patch mentioned in the link below, +but that one got squash-merged into the patch that came earlier in the +series. + +Fixes: 4d54cc32112d8d ("mptcp: avoid lock_fast usage in accept path") +Link: https://lore.kernel.org/mptcp/20210213000001.379332-8-mathew.j.martineau@linux.intel.com/T/#u +Cc: Cong Wang +Cc: Xin Long +Cc: Johannes Berg +Cc: Sean Tranchetti +Cc: Paolo Abeni +Cc: Pablo Neira Ayuso +Signed-off-by: Florian Westphal +Signed-off-by: David S. Miller +Signed-off-by: Ido Schimmel +Signed-off-by: Greg Kroah-Hartman +--- + net/netlink/af_netlink.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +--- a/net/netlink/af_netlink.c ++++ b/net/netlink/af_netlink.c +@@ -1020,7 +1020,6 @@ static int netlink_bind(struct socket *s + return -EINVAL; + } + +- netlink_lock_table(); + if (nlk->netlink_bind && groups) { + int group; + +@@ -1032,13 +1031,14 @@ static int netlink_bind(struct socket *s + if (!err) + continue; + netlink_undo_bind(group, groups, sk); +- goto unlock; ++ return err; + } + } + + /* No need for barriers here as we return to user-space without + * using any of the bound attributes. + */ ++ netlink_lock_table(); + if (!bound) { + err = nladdr->nl_pid ? + netlink_insert(sk, nladdr->nl_pid) : diff --git a/queue-5.4/psample-require-cap_net_admin-when-joining-packets-group.patch b/queue-5.4/psample-require-cap_net_admin-when-joining-packets-group.patch new file mode 100644 index 00000000000..1777c492757 --- /dev/null +++ b/queue-5.4/psample-require-cap_net_admin-when-joining-packets-group.patch @@ -0,0 +1,115 @@ +From stable+bounces-5301-greg=kroah.com@vger.kernel.org Mon Dec 11 13:42:26 2023 +From: Ido Schimmel +Date: Mon, 11 Dec 2023 14:41:32 +0200 +Subject: psample: Require 'CAP_NET_ADMIN' when joining "packets" group +To: +Cc: , , , , , , , , , +Message-ID: <20231211124133.822891-4-idosch@nvidia.com> + +From: Ido Schimmel + +commit 44ec98ea5ea9cfecd31a5c4cc124703cb5442832 upstream. + +The "psample" generic netlink family notifies sampled packets over the +"packets" multicast group. This is problematic since by default generic +netlink allows non-root users to listen to these notifications. + +Fix by marking the group with the 'GENL_UNS_ADMIN_PERM' flag. This will +prevent non-root users or root without the 'CAP_NET_ADMIN' capability +(in the user namespace owning the network namespace) from joining the +group. + +Tested using [1]. + +Before: + + # capsh -- -c ./psample_repo + # capsh --drop=cap_net_admin -- -c ./psample_repo + +After: + + # capsh -- -c ./psample_repo + # capsh --drop=cap_net_admin -- -c ./psample_repo + Failed to join "packets" multicast group + +[1] + $ cat psample.c + #include + #include + #include + #include + + int join_grp(struct nl_sock *sk, const char *grp_name) + { + int grp, err; + + grp = genl_ctrl_resolve_grp(sk, "psample", grp_name); + if (grp < 0) { + fprintf(stderr, "Failed to resolve \"%s\" multicast group\n", + grp_name); + return grp; + } + + err = nl_socket_add_memberships(sk, grp, NFNLGRP_NONE); + if (err) { + fprintf(stderr, "Failed to join \"%s\" multicast group\n", + grp_name); + return err; + } + + return 0; + } + + int main(int argc, char **argv) + { + struct nl_sock *sk; + int err; + + sk = nl_socket_alloc(); + if (!sk) { + fprintf(stderr, "Failed to allocate socket\n"); + return -1; + } + + err = genl_connect(sk); + if (err) { + fprintf(stderr, "Failed to connect socket\n"); + return err; + } + + err = join_grp(sk, "config"); + if (err) + return err; + + err = join_grp(sk, "packets"); + if (err) + return err; + + return 0; + } + $ gcc -I/usr/include/libnl3 -lnl-3 -lnl-genl-3 -o psample_repo psample.c + +Fixes: 6ae0a6286171 ("net: Introduce psample, a new genetlink channel for packet sampling") +Reported-by: "The UK's National Cyber Security Centre (NCSC)" +Signed-off-by: Ido Schimmel +Reviewed-by: Jacob Keller +Reviewed-by: Jiri Pirko +Link: https://lore.kernel.org/r/20231206213102.1824398-2-idosch@nvidia.com +Signed-off-by: Jakub Kicinski +Signed-off-by: Greg Kroah-Hartman +--- + net/psample/psample.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +--- a/net/psample/psample.c ++++ b/net/psample/psample.c +@@ -28,7 +28,8 @@ enum psample_nl_multicast_groups { + + static const struct genl_multicast_group psample_nl_mcgrps[] = { + [PSAMPLE_NL_MCGRP_CONFIG] = { .name = PSAMPLE_NL_MCGRP_CONFIG_NAME }, +- [PSAMPLE_NL_MCGRP_SAMPLE] = { .name = PSAMPLE_NL_MCGRP_SAMPLE_NAME }, ++ [PSAMPLE_NL_MCGRP_SAMPLE] = { .name = PSAMPLE_NL_MCGRP_SAMPLE_NAME, ++ .flags = GENL_UNS_ADMIN_PERM }, + }; + + static struct genl_family psample_nl_family __ro_after_init; diff --git a/queue-5.4/series b/queue-5.4/series index af8368f65d7..af2b62d4ab4 100644 --- a/queue-5.4/series +++ b/queue-5.4/series @@ -55,3 +55,7 @@ x86-cpu-amd-check-vendor-in-the-amd-microcode-callback.patch kvm-s390-mm-properly-reset-no-dat.patch nilfs2-fix-missing-error-check-for-sb_set_blocksize-call.patch io_uring-af_unix-disable-sending-io_uring-over-sockets.patch +netlink-don-t-call-netlink_bind-with-table-lock-held.patch +genetlink-add-cap_net_admin-test-for-multicast-bind.patch +psample-require-cap_net_admin-when-joining-packets-group.patch +drop_monitor-require-cap_sys_admin-when-joining-events-group.patch