| Message ID | 1311706717-7398-6-git-send-email-serge@hallyn.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Jul 26, 2011 at 18:58 +0000, Serge Hallyn wrote: > From: Serge E. Hallyn <serge.hallyn@canonical.com> > > A few modules are using cap_raised(current_cap(), cap) to authorize > actions, but the privilege should be applicable against the initial > user namespace. Refuse privilege if the caller is not in init_user_ns. > > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > Cc: Eric W. Biederman <ebiederm@xmission.com> > --- > drivers/block/drbd/drbd_nl.c | 5 +++++ > drivers/md/dm-log-userspace-transfer.c | 3 +++ > drivers/staging/pohmelfs/config.c | 3 +++ > drivers/video/uvesafb.c | 3 +++ > 4 files changed, 14 insertions(+), 0 deletions(-) > > diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c > index 515bcd9..7717f8a 100644 > --- a/drivers/block/drbd/drbd_nl.c > +++ b/drivers/block/drbd/drbd_nl.c > @@ -2297,6 +2297,11 @@ static void drbd_connector_callback(struct cn_msg *req, struct netlink_skb_parms > return; > } > > + if (current_user_ns() != &init_user_ns) { [...] > if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) { [...] Looks like it is an often pattern. Maybe move both checks to a function? Thanks,
Quoting Vasiliy Kulikov (segooon@gmail.com): > On Tue, Jul 26, 2011 at 18:58 +0000, Serge Hallyn wrote: > > From: Serge E. Hallyn <serge.hallyn@canonical.com> > > > > A few modules are using cap_raised(current_cap(), cap) to authorize > > actions, but the privilege should be applicable against the initial > > user namespace. Refuse privilege if the caller is not in init_user_ns. > > > > Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com> > > Cc: Eric W. Biederman <ebiederm@xmission.com> > > --- > > drivers/block/drbd/drbd_nl.c | 5 +++++ > > drivers/md/dm-log-userspace-transfer.c | 3 +++ > > drivers/staging/pohmelfs/config.c | 3 +++ > > drivers/video/uvesafb.c | 3 +++ > > 4 files changed, 14 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c > > index 515bcd9..7717f8a 100644 > > --- a/drivers/block/drbd/drbd_nl.c > > +++ b/drivers/block/drbd/drbd_nl.c > > @@ -2297,6 +2297,11 @@ static void drbd_connector_callback(struct cn_msg *req, struct netlink_skb_parms > > return; > > } > > > > + if (current_user_ns() != &init_user_ns) { > [...] > > if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) { > [...] > > Looks like it is an often pattern. Maybe move both checks to a > function? This pattern is used 4 times (IIRC). The reason I didn't break it out is that it's very close to just 'capable(CAP_SYS_ADMIN)', which also checks for CAP_SYS_ADMIN to the init_user_ns. But the above, rightly or wrongly, does not set the PF_SUPERPRIV task flag. I don't want to advocate usage of the above, and creating a helper for the above would both further pollute the capability-related function namespace, and make the above look more legitimate than I think it is. Imo 'cap-raised(current_cap(), X)' should not be used at all. But I didn't want to deal with that here, just make it user-ns safe. -serge -- 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 515bcd9..7717f8a 100644 --- a/drivers/block/drbd/drbd_nl.c +++ b/drivers/block/drbd/drbd_nl.c @@ -2297,6 +2297,11 @@ static void drbd_connector_callback(struct cn_msg *req, struct netlink_skb_parms return; } + if (current_user_ns() != &init_user_ns) { + retcode = ERR_PERM; + goto fail; + } + 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 1f23e04..140ca81 100644 --- a/drivers/md/dm-log-userspace-transfer.c +++ b/drivers/md/dm-log-userspace-transfer.c @@ -134,6 +134,9 @@ 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 (current_user_ns() != &init_user_ns) + return; + if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) return; diff --git a/drivers/staging/pohmelfs/config.c b/drivers/staging/pohmelfs/config.c index b6c42cb..cd259d0 100644 --- a/drivers/staging/pohmelfs/config.c +++ b/drivers/staging/pohmelfs/config.c @@ -525,6 +525,9 @@ static void pohmelfs_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *n { int err; + if (current_user_ns() != &init_user_ns) + return; + if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) return; diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c index 7f8472c..71dab8e 100644 --- a/drivers/video/uvesafb.c +++ b/drivers/video/uvesafb.c @@ -73,6 +73,9 @@ static void uvesafb_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *ns struct uvesafb_task *utask; struct uvesafb_ktask *task; + if (current_user_ns() != &init_user_ns) + return; + if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) return;