Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx): > > In 2009 Philip Reiser notied that a few users of netlink connector > interface needed a capability check and added the idiom > cap_raised(nsp->eff_cap, CAP_SYS_ADMIN) to a few of them, on the premise > that netlink was asynchronous. > > In 2011 Patrick McHardy noticed we were being silly because netlink is > synchronous and removed eff_cap from the netlink_skb_params and changed > the idiom to cap_raised(current_cap(), CAP_SYS_ADMIN). > > Looking at those spots with a fresh eye we should be calling > capable(CAP_SYS_ADMIN). The only reason I can see for not calling > capable is that it once appeared we were not in the same task as the > caller which would have made calling capable() impossible. And (just to make sure) that is now absolutely not the case? > In the initial user_namespace the only difference between between > cap_raised(current_cap(), CAP_SYS_ADMIN) and capable(CAP_SYS_ADMIN) > are a few sanity checks and the fact that capable(CAP_SYS_ADMIN) > sets PF_SUPERPRIV if we use the capability. > > Since we are going to be using root privilege setting PF_SUPERPRIV > seems the right thing to do. > > The motivation for this that patch is that in a child user namespace > cap_raised(current_cap(),...) tests your capabilities with respect to > that child user namespace not capabilities in the initial user namespace > and thus will allow processes that should be unprivielged to use the > kernel services that are only protected with > cap_raised(current_cap(),..). > > To fix possible user_namespace issues and to just clean up the code > replace cap_raised(current_cap(), CAP_SYS_ADMIN) with > capable(CAP_SYS_ADMIN). > > Cc: Patrick McHardy <kaber@xxxxxxxxx> > Cc: Philipp Reisner <philipp.reisner@xxxxxxxxxx> > Cc: Serge E. Hallyn <serge.hallyn@xxxxxxxxxxxxx> Acked-by: Serge E. Hallyn <serge.hallyn@xxxxxxxxxxxxx> thanks, -serge > Cc: Andrew Morgan <morgan@xxxxxxxxxx> > Cc: Vasiliy Kulikov <segoon@xxxxxxxxxxxx> > Cc: David Howells <dhowells@xxxxxxxxxx> > Signed-off-by: Eric W. Biederman <ebiederm@xxxxxxxxxxxx> > --- > drivers/block/drbd/drbd_nl.c | 2 +- > drivers/md/dm-log-userspace-transfer.c | 2 +- > drivers/video/uvesafb.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c > index abfaaca..946166e 100644 > --- a/drivers/block/drbd/drbd_nl.c > +++ b/drivers/block/drbd/drbd_nl.c > @@ -2297,7 +2297,7 @@ static void drbd_connector_callback(struct cn_msg *req, struct netlink_skb_parms > return; > } > > - if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) { > + if (!capable(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 1f23e04..08d9a20 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(current_cap(), CAP_SYS_ADMIN)) > + if (!capable(CAP_SYS_ADMIN)) > return; > > spin_lock(&receiving_list_lock); > diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c > index 260cca7..9f7d27a 100644 > --- a/drivers/video/uvesafb.c > +++ b/drivers/video/uvesafb.c > @@ -73,7 +73,7 @@ static void uvesafb_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *ns > struct uvesafb_task *utask; > struct uvesafb_ktask *task; > > - if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) > + if (!capable(CAP_SYS_ADMIN)) > return; > > if (msg->seq >= UVESAFB_TASKS_MAX) > -- > 1.7.2.5 -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html