| Message ID | 4D6F6180.5030903@trash.net |
|---|---|
| State | Deferred, archived |
| Delegated to: | David Miller |
| Headers | show |
Reviewed-by: James Morris <jmorris@namei.org>
* Patrick McHardy (kaber@trash.net) wrote: > commit 8ff259625f0ab295fa085b0718eed13093813fbc > Author: Patrick McHardy <kaber@trash.net> > Date: Thu Mar 3 10:17:31 2011 +0100 > > netlink: kill eff_cap from struct netlink_skb_parms > > Netlink message processing in the kernel is synchronous these days, > capabilities can be checked directly in security_netlink_recv() from > the current process. > > Signed-off-by: Patrick McHardy <kaber@trash.net> Thanks for doing that Patrick. I looked at this earlier and thought there was still an async path, but I guess that's just to another userspace process. BTW, I think you missed a couple connector based callers: drivers/staging/pohmelfs/config.c: if (!cap_raised(nsp->eff_cap, CAP_SYS_AD drivers/video/uvesafb.c: if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN)) Fix those and: Acked-by: Chris Wright <chrisw@sous-sol.org> Ideally, we'd consolidate those into a variant of security_netlink_recv(). However the issue is with types. Inside connector callback we only have netlink_skb_params (seems inapproriate to cast back out to skb). We could change the lsm hook to only pass nsp, but SELinux actually cares about the netlink type. Any ideas? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Chris Wright <chrisw@sous-sol.org> Date: Thu, 3 Mar 2011 09:32:30 -0800 > * Patrick McHardy (kaber@trash.net) wrote: > >> commit 8ff259625f0ab295fa085b0718eed13093813fbc >> Author: Patrick McHardy <kaber@trash.net> >> Date: Thu Mar 3 10:17:31 2011 +0100 >> >> netlink: kill eff_cap from struct netlink_skb_parms >> >> Netlink message processing in the kernel is synchronous these days, >> capabilities can be checked directly in security_netlink_recv() from >> the current process. >> >> Signed-off-by: Patrick McHardy <kaber@trash.net> > > Thanks for doing that Patrick. I looked at this earlier and thought > there was still an async path, but I guess that's just to another > userspace process. > > BTW, I think you missed a couple connector based callers: > > drivers/staging/pohmelfs/config.c: if (!cap_raised(nsp->eff_cap, CAP_SYS_AD > drivers/video/uvesafb.c: if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN)) > > Fix those and: > > Acked-by: Chris Wright <chrisw@sous-sol.org> Patrick, I'll apply your first patch, please respin this second patch with the changes mentioned here. Thanks! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Chris Wright (chrisw@sous-sol.org) wrote: > Ideally, we'd consolidate those into a variant of security_netlink_recv(). > However the issue is with types. Inside connector callback we only have > netlink_skb_params (seems inapproriate to cast back out to skb). > > We could change the lsm hook to only pass nsp, but SELinux actually > cares about the netlink type. Any ideas? Actually I misremembered, it only cares on the send path. We could completely drop skb from recv lsm hook, will send an RFC momentarily with example. thanks, -chris -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c index 8cbfaa6..fe81c85 100644 --- a/drivers/block/drbd/drbd_nl.c +++ b/drivers/block/drbd/drbd_nl.c @@ -2177,7 +2177,7 @@ static void drbd_connector_callback(struct cn_msg *req, struct netlink_skb_parms return; } - if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN)) { + if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) { retcode = ERR_PERM; goto fail; } diff --git a/drivers/md/dm-log-userspace-transfer.c b/drivers/md/dm-log-userspace-transfer.c index 049eaf1..1f23e04 100644 --- a/drivers/md/dm-log-userspace-transfer.c +++ b/drivers/md/dm-log-userspace-transfer.c @@ -134,7 +134,7 @@ static void cn_ulog_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp) { struct dm_ulog_request *tfr = (struct dm_ulog_request *)(msg + 1); - if (!cap_raised(nsp->eff_cap, CAP_SYS_ADMIN)) + if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) return; spin_lock(&receiving_list_lock); diff --git a/include/linux/netlink.h b/include/linux/netlink.h index 66823b8..4c4ac3f 100644 --- a/include/linux/netlink.h +++ b/include/linux/netlink.h @@ -160,7 +160,6 @@ struct netlink_skb_parms { struct ucred creds; /* Skb credentials */ __u32 pid; __u32 dst_group; - kernel_cap_t eff_cap; }; #define NETLINK_CB(skb) (*(struct netlink_skb_parms*)&((skb)->cb)) diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 97ecd92..a808fb1 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1364,12 +1364,6 @@ static int netlink_sendmsg(struct kiocb *kiocb, struct socket *sock, NETLINK_CB(skb).dst_group = dst_group; memcpy(NETLINK_CREDS(skb), &siocb->scm->creds, sizeof(struct ucred)); - /* What can I do? Netlink is asynchronous, so that - we will have to save current capabilities to - check them, when this message will be delivered - to corresponding kernel module. --ANK (980802) - */ - err = -EFAULT; if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) { kfree_skb(skb); diff --git a/security/commoncap.c b/security/commoncap.c index 64c2ed9..a83e607 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -52,13 +52,12 @@ static void warn_setuid_and_fcaps_mixed(const char *fname) int cap_netlink_send(struct sock *sk, struct sk_buff *skb) { - NETLINK_CB(skb).eff_cap = current_cap(); return 0; } int cap_netlink_recv(struct sk_buff *skb, int cap) { - if (!cap_raised(NETLINK_CB(skb).eff_cap, cap)) + if (!cap_raised(current_cap(), cap)) return -EPERM; return 0; }
commit 8ff259625f0ab295fa085b0718eed13093813fbc Author: Patrick McHardy <kaber@trash.net> Date: Thu Mar 3 10:17:31 2011 +0100 netlink: kill eff_cap from struct netlink_skb_parms Netlink message processing in the kernel is synchronous these days, capabilities can be checked directly in security_netlink_recv() from the current process. Signed-off-by: Patrick McHardy <kaber@trash.net>