On 14/03/23, David Miller wrote: In the general case, I agree. In the one existing case (netfilter), it adds a module that is never unloaded. (refcounts are bumped up and down, but I don't see an auto-reap based on cleared multicast group subscriptions.) For that matter, netlink_realloc_groups() isn't reversed on error either. In the proposed case (audit) it is only a permissions check, so there is nothing to undo. So, I was being lazy looking at the existing situation. Is it time to add a ".unbind = netlink_unbind" to struct proto_ops netlink_ops? (I am only half serious here...) Fair enough. I'll go back and look at updating subscriptions and listeners first and undoing those actions if the bind fails. In the case of netlink_setsockopt() it is just one to undo, which is easy. netlink_bind() is a bit more complex, but doable. The whole purpose here was to add a way for each protocol to be able to add its own permissions check and signal a way for netlink to refuse the subscription if the userspace process doesn't have the required permissions, so not returning an error defeats that whole purpose. - RGB -- Richard Guy Briggs <rbriggs(a)redhat.com&gt; Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

Have the netlink per-protocol optional bind function return an int error code rather than void to signal a failure. This will enable netlink protocols to perform extra checks including capabilities and permissions verifications when updating memberships in multicast groups. In netlink_bind() and netlink_setsockopt() the call to the per-protocol bind function was moved above the multicast group update to prevent any access to the multicast socket groups before checking with the per-protocol bind function. This will enable the per-protocol bind function to be used to check permissions which could be denied before making them available, and to avoid the messy job of undoing the addition should the per-protocol bind function fail. The netfilter subsystem seems to be the only one currently using the per-protocol bind function. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- In particular, the audit subsystem (NETLINK_AUDIT protocol) could benefit by being able to check specific capabilities for each multicast group before granting membership to the requesting socket. Currently, all NETLINK_AUDIT sockets must have the capability CAP_NET_ADMIN. No other capabilities are required to join a multicast group. This capability is too broad allowing access to this socket by many applications that must not have access to this information. It is proposed to add capability CAP_AUDIT_READ to allow this access while dropping the exessively broad capability CAP_NET_ADMIN. There has also been some interest expressed by IETF ForCES folk. --- include/linux/netlink.h | 3 ++- net/netfilter/nfnetlink.c | 3 ++- net/netlink/af_netlink.c | 39 ++++++++++++++++++++++++++------------- net/netlink/af_netlink.h | 6 ++++-- 4 files changed, 34 insertions(+), 17 deletions(-) diff --git a/include/linux/netlink.h b/include/linux/netlink.h index 7a6c396..0ba2edd 100644 --- a/include/linux/netlink.h +++ b/include/linux/netlink.h @@ -45,7 +45,8 @@ struct netlink_kernel_cfg { unsigned int flags; void (*input)(struct sk_buff *skb); struct mutex *cb_mutex; - void (*bind)(int group); + int (*bind)(int group); + void (*unbind)(int group); bool (*compare)(struct net *net, struct sock *sk); }; diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c index 1da5ef1..14c8ac8 100644 --- a/net/netfilter/nfnetlink.c +++ b/net/netfilter/nfnetlink.c @@ -392,7 +392,7 @@ static void nfnetlink_rcv(struct sk_buff *skb) } #ifdef CONFIG_MODULES -static void nfnetlink_bind(int group) +static int nfnetlink_bind(int group) { const struct nfnetlink_subsystem *ss; int type = nfnl_group2type[group]; @@ -402,6 +402,7 @@ static void nfnetlink_bind(int group) rcu_read_unlock(); if (!ss) request_module("nfnetlink-subsys-%d", type); + return 0; } #endif diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index bca50b9..301dfe9 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1198,7 +1198,8 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol, struct module *module = NULL; struct mutex *cb_mutex; struct netlink_sock *nlk; - void (*bind)(int group); + int (*bind)(int group); + void (*unbind)(int group); int err = 0; sock->state = SS_UNCONNECTED; @@ -1224,6 +1225,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol, err = -EPROTONOSUPPORT; cb_mutex = nl_table[protocol].cb_mutex; bind = nl_table[protocol].bind; + unbind = nl_table[protocol].unbind; netlink_unlock_table(); if (err < 0) @@ -1240,6 +1242,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol, nlk = nlk_sk(sock->sk); nlk->module = module; nlk->netlink_bind = bind; + nlk->netlink_unbind = unbind; out: return err; @@ -1293,6 +1296,7 @@ static int netlink_release(struct socket *sock) kfree_rcu(old, rcu); nl_table[sk->sk_protocol].module = NULL; nl_table[sk->sk_protocol].bind = NULL; + nl_table[sk->sk_protocol].unbind = NULL; nl_table[sk->sk_protocol].flags = 0; nl_table[sk->sk_protocol].registered = 0; } @@ -1441,6 +1445,22 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr, if (!nladdr->nl_groups && (nlk->groups == NULL || !(u32)nlk->groups[0])) return 0; + if (nlk->netlink_bind && nladdr->nl_groups) { + int i; + + for (i = 0; i < nlk->ngroups; i++) + if (test_bit(i, (long unsigned int *)&nladdr->nl_groups)) { + err = nlk->netlink_bind(i); + if (err) + if (!nlk->portid) + netlink_remove(sk); + for (int undo = 0; undo < i; undo++) + if (nlk->netlink_unbind) + nlk->netlink_unbind(undo); + return err; + } + } + netlink_table_grab(); netlink_update_subscriptions(sk, nlk->subscriptions + hweight32(nladdr->nl_groups) - @@ -1449,15 +1469,6 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr, netlink_update_listeners(sk); netlink_table_ungrab(); - if (nlk->netlink_bind && nlk->groups[0]) { - int i; - - for (i=0; i<nlk->ngroups; i++) { - if (test_bit(i, nlk->groups)) - nlk->netlink_bind(i); - } - } - return 0; } @@ -2095,14 +2106,16 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname, return err; if (!val || val - 1 >= nlk->ngroups) return -EINVAL; + if (nlk->netlink_bind) { + err = nlk->netlink_bind(val); + if (err) + return err; + } netlink_table_grab(); netlink_update_socket_mc(nlk, val, optname == NETLINK_ADD_MEMBERSHIP); netlink_table_ungrab(); - if (nlk->netlink_bind) - nlk->netlink_bind(val); - err = 0; break; } diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h index acbd774..2aeae8a 100644 --- a/net/netlink/af_netlink.h +++ b/net/netlink/af_netlink.h @@ -37,7 +37,8 @@ struct netlink_sock { struct mutex *cb_mutex; struct mutex cb_def_mutex; void (*netlink_rcv)(struct sk_buff *skb); - void (*netlink_bind)(int group); + int (*netlink_bind)(int group); + void (*netlink_unbind)(int group); struct module *module; #ifdef CONFIG_NETLINK_MMAP struct mutex pg_vec_lock; @@ -73,7 +74,8 @@ struct netlink_table { unsigned int groups; struct mutex *cb_mutex; struct module *module; - void (*bind)(int group); + int (*bind)(int group); + void (*unbind)(int group); bool (*compare)(struct net *net, struct sock *sock); int registered; }; -- 1.7.1

Have the netlink per-protocol optional bind function return an int error code rather than void to signal a failure. This will enable netlink protocols to perform extra checks including capabilities and permissions verifications when updating memberships in multicast groups. In netlink_bind() and netlink_setsockopt() the call to the per-protocol bind function was moved above the multicast group update to prevent any access to the multicast socket groups before checking with the per-protocol bind function. This will enable the per-protocol bind function to be used to check permissions which could be denied before making them available, and to avoid the messy job of undoing the addition should the per-protocol bind function fail. The netfilter subsystem seems to be the only one currently using the per-protocol bind function. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- In particular, the audit subsystem (NETLINK_AUDIT protocol) could benefit by being able to check specific capabilities for each multicast group before granting membership to the requesting socket. Currently, all NETLINK_AUDIT sockets must have the capability CAP_NET_ADMIN. No other capabilities are required to join a multicast group. This capability is too broad allowing access to this socket by many applications that must not have access to this information. It is proposed to add capability CAP_AUDIT_READ to allow this access while dropping the exessively broad capability CAP_NET_ADMIN. There has also been some interest expressed by IETF ForCES folk. v5: *really* fixed a logic error (missing braces) and a C99 error. v3: added netlink_unbind() option and unwound netlink_insert(). --- include/linux/netlink.h | 3 ++- net/netfilter/nfnetlink.c | 3 ++- net/netlink/af_netlink.c | 39 ++++++++++++++++++++++++++------------- net/netlink/af_netlink.h | 6 ++++-- 4 files changed, 34 insertions(+), 17 deletions(-) diff --git a/include/linux/netlink.h b/include/linux/netlink.h index 7a6c396..0ba2edd 100644 --- a/include/linux/netlink.h +++ b/include/linux/netlink.h @@ -45,7 +45,8 @@ struct netlink_kernel_cfg { unsigned int flags; void (*input)(struct sk_buff *skb); struct mutex *cb_mutex; - void (*bind)(int group); + int (*bind)(int group); + void (*unbind)(int group); bool (*compare)(struct net *net, struct sock *sk); }; diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c index 1da5ef1..14c8ac8 100644 --- a/net/netfilter/nfnetlink.c +++ b/net/netfilter/nfnetlink.c @@ -392,7 +392,7 @@ static void nfnetlink_rcv(struct sk_buff *skb) } #ifdef CONFIG_MODULES -static void nfnetlink_bind(int group) +static int nfnetlink_bind(int group) { const struct nfnetlink_subsystem *ss; int type = nfnl_group2type[group]; @@ -402,6 +402,7 @@ static void nfnetlink_bind(int group) rcu_read_unlock(); if (!ss) request_module("nfnetlink-subsys-%d", type); + return 0; } #endif diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index bca50b9..301dfe9 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1198,7 +1198,8 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol, struct module *module = NULL; struct mutex *cb_mutex; struct netlink_sock *nlk; - void (*bind)(int group); + int (*bind)(int group); + void (*unbind)(int group); int err = 0; sock->state = SS_UNCONNECTED; @@ -1224,6 +1225,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol, err = -EPROTONOSUPPORT; cb_mutex = nl_table[protocol].cb_mutex; bind = nl_table[protocol].bind; + unbind = nl_table[protocol].unbind; netlink_unlock_table(); if (err < 0) @@ -1240,6 +1242,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol, nlk = nlk_sk(sock->sk); nlk->module = module; nlk->netlink_bind = bind; + nlk->netlink_unbind = unbind; out: return err; @@ -1293,6 +1296,7 @@ static int netlink_release(struct socket *sock) kfree_rcu(old, rcu); nl_table[sk->sk_protocol].module = NULL; nl_table[sk->sk_protocol].bind = NULL; + nl_table[sk->sk_protocol].unbind = NULL; nl_table[sk->sk_protocol].flags = 0; nl_table[sk->sk_protocol].registered = 0; } @@ -1441,6 +1445,22 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr, if (!nladdr->nl_groups && (nlk->groups == NULL || !(u32)nlk->groups[0])) return 0; + if (nlk->netlink_bind && nladdr->nl_groups) { + int i; + + for (i = 0; i < nlk->ngroups; i++) + if (test_bit(i, (long unsigned int *)&nladdr->nl_groups)) { + err = nlk->netlink_bind(i); + if (err) + if (!nlk->portid) + netlink_remove(sk); + for (int undo = 0; undo < i; undo++) + if (nlk->netlink_unbind) + nlk->netlink_unbind(undo); + return err; + } + } + netlink_table_grab(); netlink_update_subscriptions(sk, nlk->subscriptions + hweight32(nladdr->nl_groups) - @@ -1449,15 +1469,6 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr, netlink_update_listeners(sk); netlink_table_ungrab(); - if (nlk->netlink_bind && nlk->groups[0]) { - int i; - - for (i=0; i<nlk->ngroups; i++) { - if (test_bit(i, nlk->groups)) - nlk->netlink_bind(i); - } - } - return 0; } @@ -2095,14 +2106,16 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname, return err; if (!val || val - 1 >= nlk->ngroups) return -EINVAL; + if (nlk->netlink_bind) { + err = nlk->netlink_bind(val); + if (err) + return err; + } netlink_table_grab(); netlink_update_socket_mc(nlk, val, optname == NETLINK_ADD_MEMBERSHIP); netlink_table_ungrab(); - if (nlk->netlink_bind) - nlk->netlink_bind(val); - err = 0; break; } diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h index acbd774..2aeae8a 100644 --- a/net/netlink/af_netlink.h +++ b/net/netlink/af_netlink.h @@ -37,7 +37,8 @@ struct netlink_sock { struct mutex *cb_mutex; struct mutex cb_def_mutex; void (*netlink_rcv)(struct sk_buff *skb); - void (*netlink_bind)(int group); + int (*netlink_bind)(int group); + void (*netlink_unbind)(int group); struct module *module; #ifdef CONFIG_NETLINK_MMAP struct mutex pg_vec_lock; @@ -73,7 +74,8 @@ struct netlink_table { unsigned int groups; struct mutex *cb_mutex; struct module *module; - void (*bind)(int group); + int (*bind)(int group); + void (*unbind)(int group); bool (*compare)(struct net *net, struct sock *sock); int registered; }; -- 1.7.1

From: Richard Guy Briggs <rgb(a)redhat.com&gt; Date: Tue, 25 Mar 2014 08:50:56 -0400 ... Please put an empty line between local variable declarations and code, just as it is a few lines up.

Remove duplicity and simplify code flow by moving the rcu_read_unlock() above the condition and let the flow control exit naturally at the end of the function. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- net/netfilter/nfnetlink.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c index 046aa13..1da5ef1 100644 --- a/net/netfilter/nfnetlink.c +++ b/net/netfilter/nfnetlink.c @@ -399,12 +399,9 @@ static void nfnetlink_bind(int group) rcu_read_lock(); ss = nfnetlink_get_subsys(type); - if (!ss) { - rcu_read_unlock(); - request_module("nfnetlink-subsys-%d", type); - return; - } rcu_read_unlock(); + if (!ss) + request_module("nfnetlink-subsys-%d", type); } #endif -- 1.7.1

Call the per-protocol unbind function rather than bind function on NETLINK_DROP_MEMBERSHIP in netlink_setsockopt(). Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- net/netlink/af_netlink.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 9ae6e0f..3938138 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -2109,7 +2109,7 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname, return err; if (!val || val - 1 >= nlk->ngroups) return -EINVAL; - if (nlk->netlink_bind) { + if (optname == NETLINK_ADD_MEMBERSHIP && nlk->netlink_bind) { err = nlk->netlink_bind(val); if (err) return err; @@ -2118,6 +2118,8 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname, netlink_update_socket_mc(nlk, val, optname == NETLINK_ADD_MEMBERSHIP); netlink_table_ungrab(); + if (optname == NETLINK_DROP_MEMBERSHIP && nlk->netlink_unbind) + nlk->netlink_unbind(val); err = 0; break; -- 1.7.1

Remove duplicity and simplify code flow by moving the rcu_read_unlock() above the condition and let the flow control exit naturally at the end of the function. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- net/netfilter/nfnetlink.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c index e8138da..0df800a 100644 --- a/net/netfilter/nfnetlink.c +++ b/net/netfilter/nfnetlink.c @@ -407,12 +407,9 @@ static void nfnetlink_bind(int group) rcu_read_lock(); ss = nfnetlink_get_subsys(type); - if (!ss) { - rcu_read_unlock(); - request_module("nfnetlink-subsys-%d", type); - return; - } rcu_read_unlock(); + if (!ss) + request_module("nfnetlink-subsys-%d", type); } #endif -- 1.7.1

From: Richard Guy Briggs <rgb(a)redhat.com&gt; Date: Fri, 18 Apr 2014 13:34:06 -0400 It took me a while to figure out why you need to do the netlink_remove() in the error path. I think it's really asking for trouble to allow the socket to have temporary visibility if we end up signalling an error. It seems safest if we only do the autobind/insert once we are absolutely certain that the bind() will fully succeed. This means that you have to do this bind validation loop before autobind/insert. Please make this change and resubmit this series, thanks.

Have the netlink per-protocol optional bind function return an int error code rather than void to signal a failure. This will enable netlink protocols to perform extra checks including capabilities and permissions verifications when updating memberships in multicast groups. In netlink_bind() and netlink_setsockopt() the call to the per-protocol bind function was moved above the multicast group update to prevent any access to the multicast socket groups before checking with the per-protocol bind function. This will enable the per-protocol bind function to be used to check permissions which could be denied before making them available, and to avoid the messy job of undoing the addition should the per-protocol bind function fail. The netfilter subsystem seems to be the only one currently using the per-protocol bind function. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- V1: This rev fixes a bug introduced by flattening the code in the last posting. *V2: This rev moves the per-protocol bind call above the socket exposure call and refactors out the unbind procedure. In particular, the audit subsystem (NETLINK_AUDIT protocol) could benefit by being able to check specific capabilities for each multicast group before granting membership to the requesting socket. Currently, all NETLINK_AUDIT sockets must have the capability CAP_NET_ADMIN. No other capabilities are required to join a multicast group. This capability is too broad allowing access to this socket by many applications that must not have access to this information. It is proposed to add capability CAP_AUDIT_READ to allow this access while dropping the exessively broad capability CAP_NET_ADMIN. There has also been some interest expressed by IETF ForCES folk. --- include/linux/netlink.h | 3 +- net/netfilter/nfnetlink.c | 3 +- net/netlink/af_netlink.c | 68 +++++++++++++++++++++++++++++++------------- net/netlink/af_netlink.h | 6 ++- 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/include/linux/netlink.h b/include/linux/netlink.h index aad8eea..5146ce0 100644 --- a/include/linux/netlink.h +++ b/include/linux/netlink.h @@ -45,7 +45,8 @@ struct netlink_kernel_cfg { unsigned int flags; void (*input)(struct sk_buff *skb); struct mutex *cb_mutex; - void (*bind)(int group); + int (*bind)(int group); + void (*unbind)(int group); bool (*compare)(struct net *net, struct sock *sk); }; diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c index 0df800a..6e42dcf 100644 --- a/net/netfilter/nfnetlink.c +++ b/net/netfilter/nfnetlink.c @@ -400,7 +400,7 @@ static void nfnetlink_rcv(struct sk_buff *skb) } #ifdef CONFIG_MODULES -static void nfnetlink_bind(int group) +static int nfnetlink_bind(int group) { const struct nfnetlink_subsystem *ss; int type = nfnl_group2type[group]; @@ -410,6 +410,7 @@ static void nfnetlink_bind(int group) rcu_read_unlock(); if (!ss) request_module("nfnetlink-subsys-%d", type); + return 0; } #endif diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 894cda0..7e8d229 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1206,7 +1206,8 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol, struct module *module = NULL; struct mutex *cb_mutex; struct netlink_sock *nlk; - void (*bind)(int group); + int (*bind)(int group); + void (*unbind)(int group); int err = 0; sock->state = SS_UNCONNECTED; @@ -1232,6 +1233,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol, err = -EPROTONOSUPPORT; cb_mutex = nl_table[protocol].cb_mutex; bind = nl_table[protocol].bind; + unbind = nl_table[protocol].unbind; netlink_unlock_table(); if (err < 0) @@ -1248,6 +1250,7 @@ static int netlink_create(struct net *net, struct socket *sock, int protocol, nlk = nlk_sk(sock->sk); nlk->module = module; nlk->netlink_bind = bind; + nlk->netlink_unbind = unbind; out: return err; @@ -1301,6 +1304,7 @@ static int netlink_release(struct socket *sock) kfree_rcu(old, rcu); nl_table[sk->sk_protocol].module = NULL; nl_table[sk->sk_protocol].bind = NULL; + nl_table[sk->sk_protocol].unbind = NULL; nl_table[sk->sk_protocol].flags = 0; nl_table[sk->sk_protocol].registered = 0; } @@ -1411,6 +1415,19 @@ static int netlink_realloc_groups(struct sock *sk) return err; } +static void netlink_unbind(int group, long unsigned int groups, + struct netlink_sock *nlk) +{ + int undo; + + if (!nlk->netlink_unbind) + return; + + for (undo = 0; undo < group; undo++) + if (test_bit(group, &groups)) + nlk->netlink_unbind(undo); +} + static int netlink_bind(struct socket *sock, struct sockaddr *addr, int addr_len) { @@ -1419,6 +1436,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr, struct netlink_sock *nlk = nlk_sk(sk); struct sockaddr_nl *nladdr = (struct sockaddr_nl *)addr; int err; + long unsigned int groups = nladdr->nl_groups; if (addr_len < sizeof(struct sockaddr_nl)) return -EINVAL; @@ -1427,7 +1445,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr, return -EINVAL; /* Only superuser is allowed to listen multicasts */ - if (nladdr->nl_groups) { + if (groups) { if (!netlink_capable(sock, NL_CFG_F_NONROOT_RECV)) return -EPERM; err = netlink_realloc_groups(sk); @@ -1435,37 +1453,45 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr, return err; } - if (nlk->portid) { + if (nlk->portid) if (nladdr->nl_pid != nlk->portid) return -EINVAL; - } else { + + if (nlk->netlink_bind && groups) { + int group; + + for (group = 0; group < nlk->ngroups; group++) { + if (!test_bit(group, &groups)) + continue; + err = nlk->netlink_bind(group); + if (!err) + continue; + netlink_unbind(group, groups, nlk); + return err; + } + } + + if (!nlk->portid) { err = nladdr->nl_pid ? netlink_insert(sk, net, nladdr->nl_pid) : netlink_autobind(sock); - if (err) + if (err) { + netlink_unbind(nlk->ngroups - 1, groups, nlk); return err; + } } - if (!nladdr->nl_groups && (nlk->groups == NULL || !(u32)nlk->groups[0])) + if (!groups && (nlk->groups == NULL || !(u32)nlk->groups[0])) return 0; netlink_table_grab(); netlink_update_subscriptions(sk, nlk->subscriptions + - hweight32(nladdr->nl_groups) - + hweight32(groups) - hweight32(nlk->groups[0])); - nlk->groups[0] = (nlk->groups[0] & ~0xffffffffUL) | nladdr->nl_groups; + nlk->groups[0] = (nlk->groups[0] & ~0xffffffffUL) | groups; netlink_update_listeners(sk); netlink_table_ungrab(); - if (nlk->netlink_bind && nlk->groups[0]) { - int i; - - for (i = 0; i < nlk->ngroups; i++) { - if (test_bit(i, nlk->groups)) - nlk->netlink_bind(i); - } - } - return 0; } @@ -2103,14 +2129,16 @@ static int netlink_setsockopt(struct socket *sock, int level, int optname, return err; if (!val || val - 1 >= nlk->ngroups) return -EINVAL; + if (nlk->netlink_bind) { + err = nlk->netlink_bind(val); + if (err) + return err; + } netlink_table_grab(); netlink_update_socket_mc(nlk, val, optname == NETLINK_ADD_MEMBERSHIP); netlink_table_ungrab(); - if (nlk->netlink_bind) - nlk->netlink_bind(val); - err = 0; break; } diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h index ed13a79..0b59d44 100644 --- a/net/netlink/af_netlink.h +++ b/net/netlink/af_netlink.h @@ -38,7 +38,8 @@ struct netlink_sock { struct mutex *cb_mutex; struct mutex cb_def_mutex; void (*netlink_rcv)(struct sk_buff *skb); - void (*netlink_bind)(int group); + int (*netlink_bind)(int group); + void (*netlink_unbind)(int group); struct module *module; #ifdef CONFIG_NETLINK_MMAP struct mutex pg_vec_lock; @@ -74,7 +75,8 @@ struct netlink_table { unsigned int groups; struct mutex *cb_mutex; struct module *module; - void (*bind)(int group); + int (*bind)(int group); + void (*unbind)(int group); bool (*compare)(struct net *net, struct sock *sock); int registered; }; -- 1.7.1

Register a netlink per-protocol bind fuction for audit to check userspace process capabilities before allowing a multicast group connection. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- include/uapi/linux/capability.h | 7 ++++++- kernel/audit.c | 10 ++++++++++ security/selinux/include/classmap.h | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h index 154dd6d..12c37a1 100644 --- a/include/uapi/linux/capability.h +++ b/include/uapi/linux/capability.h @@ -347,7 +347,12 @@ struct vfs_cap_data { #define CAP_BLOCK_SUSPEND 36 -#define CAP_LAST_CAP CAP_BLOCK_SUSPEND +/* Allow reading the audit log via multicast netlink socket */ + +#define CAP_AUDIT_READ 37 + + +#define CAP_LAST_CAP CAP_AUDIT_READ #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP) diff --git a/kernel/audit.c b/kernel/audit.c index 7c28936..223cb74 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1076,10 +1076,20 @@ static void audit_receive(struct sk_buff *skb) mutex_unlock(&audit_cmd_mutex); } +/* Run custom bind function on netlink socket group connect or bind requests. */ +static int audit_bind(int group) +{ + if (!capable(CAP_AUDIT_READ)) + return -EPERM; + + return 0; +} + static int __net_init audit_net_init(struct net *net) { struct netlink_kernel_cfg cfg = { .input = audit_receive, + .bind = audit_bind, }; struct audit_net *aunet = net_generic(net, audit_net_id); diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 14d04e6..be491a7 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -147,7 +147,7 @@ struct security_class_mapping secclass_map[] = { { "peer", { "recv", NULL } }, { "capability2", { "mac_override", "mac_admin", "syslog", "wake_alarm", "block_suspend", - NULL } }, + "audit_read", NULL } }, { "kernel_service", { "use_as_override", "create_files_as", NULL } }, { "tun_socket", { COMMON_SOCK_PERMS, "attach_queue", NULL } }, -- 1.7.1

Test first to see if there are any userspace multicast listeners bound to the socket before starting the multicast send work. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- kernel/audit.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index d272cc1..33531d7 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -435,6 +435,9 @@ static void kauditd_send_multicast_skb(struct sk_buff *skb) struct audit_net *aunet = net_generic(&init_net, audit_net_id); struct sock *sock = aunet->nlsk; + if (!netlink_has_listeners(sock, AUDIT_NLGRP_READLOG)) + return; + /* * The seemingly wasteful skb_copy() rather than bumping the refcount * using skb_get() is necessary because non-standard mods are made to -- 1.7.1

Register a netlink per-protocol bind fuction for audit to check userspace process capabilities before allowing a multicast group connection. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- include/uapi/linux/capability.h | 7 ++++++- kernel/audit.c | 10 ++++++++++ security/selinux/include/classmap.h | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h index 154dd6d..12c37a1 100644 --- a/include/uapi/linux/capability.h +++ b/include/uapi/linux/capability.h @@ -347,7 +347,12 @@ struct vfs_cap_data { #define CAP_BLOCK_SUSPEND 36 -#define CAP_LAST_CAP CAP_BLOCK_SUSPEND +/* Allow reading the audit log via multicast netlink socket */ + +#define CAP_AUDIT_READ 37 + + +#define CAP_LAST_CAP CAP_AUDIT_READ #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP) diff --git a/kernel/audit.c b/kernel/audit.c index 7c28936..223cb74 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1076,10 +1076,20 @@ static void audit_receive(struct sk_buff *skb) mutex_unlock(&audit_cmd_mutex); } +/* Run custom bind function on netlink socket group connect or bind requests. */ +static int audit_bind(int group) +{ + if (!capable(CAP_AUDIT_READ)) + return -EPERM; + + return 0; +} + static int __net_init audit_net_init(struct net *net) { struct netlink_kernel_cfg cfg = { .input = audit_receive, + .bind = audit_bind, }; struct audit_net *aunet = net_generic(net, audit_net_id); diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 14d04e6..be491a7 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -147,7 +147,7 @@ struct security_class_mapping secclass_map[] = { { "peer", { "recv", NULL } }, { "capability2", { "mac_override", "mac_admin", "syslog", "wake_alarm", "block_suspend", - NULL } }, + "audit_read", NULL } }, { "kernel_service", { "use_as_override", "create_files_as", NULL } }, { "tun_socket", { COMMON_SOCK_PERMS, "attach_queue", NULL } }, -- 1.7.1

Test first to see if there are any userspace multicast listeners bound to the socket before starting the multicast send work. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- kernel/audit.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index d272cc1..33531d7 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -435,6 +435,9 @@ static void kauditd_send_multicast_skb(struct sk_buff *skb) struct audit_net *aunet = net_generic(&init_net, audit_net_id); struct sock *sock = aunet->nlsk; + if (!netlink_has_listeners(sock, AUDIT_NLGRP_READLOG)) + return; + /* * The seemingly wasteful skb_copy() rather than bumping the refcount * using skb_get() is necessary because non-standard mods are made to -- 1.7.1