* [PATCH] proc: don't show nonexistent capabilities @ 2012-10-02 20:30 Andrew Vagin 2012-10-03 16:24 ` Serge E. Hallyn 2012-10-05 14:05 ` Serge E. Hallyn 0 siblings, 2 replies; 6+ messages in thread From: Andrew Vagin @ 2012-10-02 20:30 UTC (permalink / raw) To: linux-kernel Cc: criu, Pavel Emelyanov, Cyrill Gorcunov, Serge Hallyn, linux-security-module Without this patch it is really hard to interpret a bounding set, if CAP_LAST_CAP is unknown for a current kernel. Non-existant capabilities can not be deleted from a bounding set with help of prctl. E.g.: Here are two examples without/with this patch. CapBnd: ffffffe0fdecffff CapBnd: 00000000fdecffff I suggest to hide non-existent capabilities. Here is two reasons. * It's logically and easier for using. * It helps to checkpoint-restore capabilities of tasks, because tasks can be restored on another kernel, where CAP_LAST_CAP is bigger. Cc: Serge Hallyn <serge.hallyn@canonical.com> Cc: Pavel Emelyanov <xemul@parallels.com> Signed-off-by: Andrew Vagin <avagin@openvz.org> --- include/linux/capability.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/include/linux/capability.h b/include/linux/capability.h index d10b7ed..1642778 100644 --- a/include/linux/capability.h +++ b/include/linux/capability.h @@ -420,7 +420,8 @@ extern const kernel_cap_t __cap_init_eff_set; #else /* HAND-CODED capability initializers */ # define CAP_EMPTY_SET ((kernel_cap_t){{ 0, 0 }}) -# define CAP_FULL_SET ((kernel_cap_t){{ ~0, ~0 }}) +# define CAP_FULL_SET ((kernel_cap_t){{ ~0, \ + CAP_TO_MASK(CAP_LAST_CAP + 1) - 1 } }) # define CAP_FS_SET ((kernel_cap_t){{ CAP_FS_MASK_B0 \ | CAP_TO_MASK(CAP_LINUX_IMMUTABLE), \ CAP_FS_MASK_B1 } }) -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] proc: don't show nonexistent capabilities 2012-10-02 20:30 [PATCH] proc: don't show nonexistent capabilities Andrew Vagin @ 2012-10-03 16:24 ` Serge E. Hallyn 2012-10-04 21:42 ` Andrey Wagin 2012-10-05 14:05 ` Serge E. Hallyn 1 sibling, 1 reply; 6+ messages in thread From: Serge E. Hallyn @ 2012-10-03 16:24 UTC (permalink / raw) To: Andrew Vagin Cc: linux-kernel, criu, Pavel Emelyanov, Cyrill Gorcunov, Serge Hallyn, linux-security-module Quoting Andrew Vagin (avagin@openvz.org): > Without this patch it is really hard to interpret a bounding set, > if CAP_LAST_CAP is unknown for a current kernel. > > Non-existant capabilities can not be deleted from a bounding set > with help of prctl. > > E.g.: Here are two examples without/with this patch. > CapBnd: ffffffe0fdecffff > CapBnd: 00000000fdecffff > > I suggest to hide non-existent capabilities. Here is two reasons. > * It's logically and easier for using. > * It helps to checkpoint-restore capabilities of tasks, because tasks > can be restored on another kernel, where CAP_LAST_CAP is bigger. > > Cc: Serge Hallyn <serge.hallyn@canonical.com> Hm, I don't object to this patch. Sounds useful indeed. I can't help shake the feeling though that something somewhere will get confused by this (though it shouldn't), so I'd like to do some testing. Have you run ltp against this? Are you running this daily with your distro? > Cc: Pavel Emelyanov <xemul@parallels.com> > Signed-off-by: Andrew Vagin <avagin@openvz.org> > --- > include/linux/capability.h | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/include/linux/capability.h b/include/linux/capability.h > index d10b7ed..1642778 100644 > --- a/include/linux/capability.h > +++ b/include/linux/capability.h > @@ -420,7 +420,8 @@ extern const kernel_cap_t __cap_init_eff_set; > #else /* HAND-CODED capability initializers */ > > # define CAP_EMPTY_SET ((kernel_cap_t){{ 0, 0 }}) > -# define CAP_FULL_SET ((kernel_cap_t){{ ~0, ~0 }}) > +# define CAP_FULL_SET ((kernel_cap_t){{ ~0, \ > + CAP_TO_MASK(CAP_LAST_CAP + 1) - 1 } }) > # define CAP_FS_SET ((kernel_cap_t){{ CAP_FS_MASK_B0 \ > | CAP_TO_MASK(CAP_LINUX_IMMUTABLE), \ > CAP_FS_MASK_B1 } }) > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] proc: don't show nonexistent capabilities 2012-10-03 16:24 ` Serge E. Hallyn @ 2012-10-04 21:42 ` Andrey Wagin 0 siblings, 0 replies; 6+ messages in thread From: Andrey Wagin @ 2012-10-04 21:42 UTC (permalink / raw) To: Serge E. Hallyn Cc: linux-kernel, criu, Pavel Emelyanov, Cyrill Gorcunov, Serge Hallyn, linux-security-module [-- Attachment #1: Type: text/plain, Size: 2537 bytes --] 2012/10/3 Serge E. Hallyn <serge@hallyn.com>: > Quoting Andrew Vagin (avagin@openvz.org): >> Without this patch it is really hard to interpret a bounding set, >> if CAP_LAST_CAP is unknown for a current kernel. >> >> Non-existant capabilities can not be deleted from a bounding set >> with help of prctl. >> >> E.g.: Here are two examples without/with this patch. >> CapBnd: ffffffe0fdecffff >> CapBnd: 00000000fdecffff >> >> I suggest to hide non-existent capabilities. Here is two reasons. >> * It's logically and easier for using. >> * It helps to checkpoint-restore capabilities of tasks, because tasks >> can be restored on another kernel, where CAP_LAST_CAP is bigger. >> >> Cc: Serge Hallyn <serge.hallyn@canonical.com> > > Hm, I don't object to this patch. Sounds useful indeed. I can't > help shake the feeling though that something somewhere will get > confused by this (though it shouldn't), so I'd like to do some > testing. Have you run ltp against this? Are you running this > daily with your distro? I've been using a kernel with this patch on my workstation (FC17) for two days. The same kernel works on a test server (RHEL6). I've executed LTP on the kernel with this patch and without it and the results are not differ. The results for both kernels are attached. Thanks. > >> Cc: Pavel Emelyanov <xemul@parallels.com> >> Signed-off-by: Andrew Vagin <avagin@openvz.org> >> --- >> include/linux/capability.h | 3 ++- >> 1 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/include/linux/capability.h b/include/linux/capability.h >> index d10b7ed..1642778 100644 >> --- a/include/linux/capability.h >> +++ b/include/linux/capability.h >> @@ -420,7 +420,8 @@ extern const kernel_cap_t __cap_init_eff_set; >> #else /* HAND-CODED capability initializers */ >> >> # define CAP_EMPTY_SET ((kernel_cap_t){{ 0, 0 }}) >> -# define CAP_FULL_SET ((kernel_cap_t){{ ~0, ~0 }}) >> +# define CAP_FULL_SET ((kernel_cap_t){{ ~0, \ >> + CAP_TO_MASK(CAP_LAST_CAP + 1) - 1 } }) >> # define CAP_FS_SET ((kernel_cap_t){{ CAP_FS_MASK_B0 \ >> | CAP_TO_MASK(CAP_LINUX_IMMUTABLE), \ >> CAP_FS_MASK_B1 } }) >> -- >> 1.7.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ [-- Attachment #2: results-3.5.4-1.fc17.x86_64.gz --] [-- Type: application/x-gzip, Size: 46964 bytes --] [-- Attachment #3: results-3.6.0+-with-patch.gz --] [-- Type: application/x-gzip, Size: 46982 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] proc: don't show nonexistent capabilities 2012-10-02 20:30 [PATCH] proc: don't show nonexistent capabilities Andrew Vagin 2012-10-03 16:24 ` Serge E. Hallyn @ 2012-10-05 14:05 ` Serge E. Hallyn 2012-10-05 15:54 ` Andrew G. Morgan 1 sibling, 1 reply; 6+ messages in thread From: Serge E. Hallyn @ 2012-10-05 14:05 UTC (permalink / raw) To: Andrew Vagin Cc: linux-kernel, criu, Pavel Emelyanov, Cyrill Gorcunov, Serge Hallyn, linux-security-module, Andrew Morgan Quoting Andrew Vagin (avagin@openvz.org): > Without this patch it is really hard to interpret a bounding set, > if CAP_LAST_CAP is unknown for a current kernel. To be clear, note that you *can* figure it out using prctl(PR_CAPBSET_DROP). But this is a nice improvement. > Non-existant capabilities can not be deleted from a bounding set > with help of prctl. > > E.g.: Here are two examples without/with this patch. > CapBnd: ffffffe0fdecffff > CapBnd: 00000000fdecffff > > I suggest to hide non-existent capabilities. Here is two reasons. > * It's logically and easier for using. > * It helps to checkpoint-restore capabilities of tasks, because tasks > can be restored on another kernel, where CAP_LAST_CAP is bigger. > > Cc: Serge Hallyn <serge.hallyn@canonical.com> Thanks, Andrew. I saw your other email about having run LTP, I didn't see any problems from userspace either. Great idea! Reviewed-by: Serge Hallyn <serge.hallyn@canonical.com> Still it's been like that for so long that, just to be safe, I'm cc:ing Andrew Morgan - can you see any problems with this? > Cc: Pavel Emelyanov <xemul@parallels.com> > Signed-off-by: Andrew Vagin <avagin@openvz.org> > --- > include/linux/capability.h | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/include/linux/capability.h b/include/linux/capability.h > index d10b7ed..1642778 100644 > --- a/include/linux/capability.h > +++ b/include/linux/capability.h > @@ -420,7 +420,8 @@ extern const kernel_cap_t __cap_init_eff_set; > #else /* HAND-CODED capability initializers */ > > # define CAP_EMPTY_SET ((kernel_cap_t){{ 0, 0 }}) > -# define CAP_FULL_SET ((kernel_cap_t){{ ~0, ~0 }}) > +# define CAP_FULL_SET ((kernel_cap_t){{ ~0, \ > + CAP_TO_MASK(CAP_LAST_CAP + 1) - 1 } }) > # define CAP_FS_SET ((kernel_cap_t){{ CAP_FS_MASK_B0 \ > | CAP_TO_MASK(CAP_LINUX_IMMUTABLE), \ > CAP_FS_MASK_B1 } }) > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] proc: don't show nonexistent capabilities 2012-10-05 14:05 ` Serge E. Hallyn @ 2012-10-05 15:54 ` Andrew G. Morgan 2012-10-05 16:46 ` Serge Hallyn 0 siblings, 1 reply; 6+ messages in thread From: Andrew G. Morgan @ 2012-10-05 15:54 UTC (permalink / raw) To: Serge E. Hallyn Cc: Cyrill Gorcunov, criu, linux-kernel, Serge Hallyn, linux-security-module, Pavel Emelyanov, Andrew Vagin I like the sentiment but have you considered the implications for a default system root user trying to set all=eip ? Existing code can do this because all bits are accessible by default. If you set the bounding set to something less than ~0, then any code that currently does this will start to fail in the common case. Cheers Andrew On Oct 5, 2012 7:13 AM, "Serge E. Hallyn" <serge@hallyn.com> wrote: > > Quoting Andrew Vagin (avagin@openvz.org): > > Without this patch it is really hard to interpret a bounding set, > > if CAP_LAST_CAP is unknown for a current kernel. > > To be clear, note that you *can* figure it out using > prctl(PR_CAPBSET_DROP). But this is a nice improvement. > > > Non-existant capabilities can not be deleted from a bounding set > > with help of prctl. > > > > E.g.: Here are two examples without/with this patch. > > CapBnd: ffffffe0fdecffff > > CapBnd: 00000000fdecffff > > > > I suggest to hide non-existent capabilities. Here is two reasons. > > * It's logically and easier for using. > > * It helps to checkpoint-restore capabilities of tasks, because tasks > > can be restored on another kernel, where CAP_LAST_CAP is bigger. > > > > Cc: Serge Hallyn <serge.hallyn@canonical.com> > > Thanks, Andrew. I saw your other email about having run LTP, I didn't > see any problems from userspace either. Great idea! > > Reviewed-by: Serge Hallyn <serge.hallyn@canonical.com> > > Still it's been like that for so long that, just to be safe, I'm cc:ing > Andrew Morgan - can you see any problems with this? > > > Cc: Pavel Emelyanov <xemul@parallels.com> > > Signed-off-by: Andrew Vagin <avagin@openvz.org> > > --- > > include/linux/capability.h | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/include/linux/capability.h b/include/linux/capability.h > > index d10b7ed..1642778 100644 > > --- a/include/linux/capability.h > > +++ b/include/linux/capability.h > > @@ -420,7 +420,8 @@ extern const kernel_cap_t __cap_init_eff_set; > > #else /* HAND-CODED capability initializers */ > > > > # define CAP_EMPTY_SET ((kernel_cap_t){{ 0, 0 }}) > > -# define CAP_FULL_SET ((kernel_cap_t){{ ~0, ~0 }}) > > +# define CAP_FULL_SET ((kernel_cap_t){{ ~0, \ > > + CAP_TO_MASK(CAP_LAST_CAP + 1) - 1 } }) > > # define CAP_FS_SET ((kernel_cap_t){{ CAP_FS_MASK_B0 \ > > | CAP_TO_MASK(CAP_LINUX_IMMUTABLE), \ > > CAP_FS_MASK_B1 } }) > > -- > > 1.7.1 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] proc: don't show nonexistent capabilities 2012-10-05 15:54 ` Andrew G. Morgan @ 2012-10-05 16:46 ` Serge Hallyn 0 siblings, 0 replies; 6+ messages in thread From: Serge Hallyn @ 2012-10-05 16:46 UTC (permalink / raw) To: Andrew G. Morgan Cc: Serge E. Hallyn, Cyrill Gorcunov, criu, linux-kernel, linux-security-module, Pavel Emelyanov, Andrew Vagin Drat, thanks Andrew, I thought I had a testcase for that in LTP, but apparently not. capsh --caps="all=eip" -- -c /bin/bash indeed fails with this patch (and succeeds without). So Nacked-by: Serge Hallyn <serge.hallyn@canonical.com> since this is a much more common idiom, enough so that I'm not willing to say userspace should work around it (which indeed it could). Note that /proc/self/status is a slow path anyway, so updating that file to output only valid capabilities might be a workable alternative. -serge Quoting Andrew G. Morgan (morgan@kernel.org): > I like the sentiment but have you considered the implications for a > default system root user trying to set all=eip ? Existing code can do > this because all bits are accessible by default. If you set the > bounding set to something less than ~0, then any code that currently > does this will start to fail in the common case. > > Cheers > > Andrew > > On Oct 5, 2012 7:13 AM, "Serge E. Hallyn" <serge@hallyn.com> wrote: > > > > Quoting Andrew Vagin (avagin@openvz.org): > > > Without this patch it is really hard to interpret a bounding set, > > > if CAP_LAST_CAP is unknown for a current kernel. > > > > To be clear, note that you *can* figure it out using > > prctl(PR_CAPBSET_DROP). But this is a nice improvement. > > > > > Non-existant capabilities can not be deleted from a bounding set > > > with help of prctl. > > > > > > E.g.: Here are two examples without/with this patch. > > > CapBnd: ffffffe0fdecffff > > > CapBnd: 00000000fdecffff > > > > > > I suggest to hide non-existent capabilities. Here is two reasons. > > > * It's logically and easier for using. > > > * It helps to checkpoint-restore capabilities of tasks, because tasks > > > can be restored on another kernel, where CAP_LAST_CAP is bigger. > > > > > > Cc: Serge Hallyn <serge.hallyn@canonical.com> > > > > Thanks, Andrew. I saw your other email about having run LTP, I didn't > > see any problems from userspace either. Great idea! > > > > Reviewed-by: Serge Hallyn <serge.hallyn@canonical.com> > > > > Still it's been like that for so long that, just to be safe, I'm cc:ing > > Andrew Morgan - can you see any problems with this? > > > > > Cc: Pavel Emelyanov <xemul@parallels.com> > > > Signed-off-by: Andrew Vagin <avagin@openvz.org> > > > --- > > > include/linux/capability.h | 3 ++- > > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > > > diff --git a/include/linux/capability.h b/include/linux/capability.h > > > index d10b7ed..1642778 100644 > > > --- a/include/linux/capability.h > > > +++ b/include/linux/capability.h > > > @@ -420,7 +420,8 @@ extern const kernel_cap_t __cap_init_eff_set; > > > #else /* HAND-CODED capability initializers */ > > > > > > # define CAP_EMPTY_SET ((kernel_cap_t){{ 0, 0 }}) > > > -# define CAP_FULL_SET ((kernel_cap_t){{ ~0, ~0 }}) > > > +# define CAP_FULL_SET ((kernel_cap_t){{ ~0, \ > > > + CAP_TO_MASK(CAP_LAST_CAP + 1) - 1 } }) > > > # define CAP_FS_SET ((kernel_cap_t){{ CAP_FS_MASK_B0 \ > > > | CAP_TO_MASK(CAP_LINUX_IMMUTABLE), \ > > > CAP_FS_MASK_B1 } }) > > > -- > > > 1.7.1 > > > > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > > the body of a message to majordomo@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-10-05 16:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-02 20:30 [PATCH] proc: don't show nonexistent capabilities Andrew Vagin 2012-10-03 16:24 ` Serge E. Hallyn 2012-10-04 21:42 ` Andrey Wagin 2012-10-05 14:05 ` Serge E. Hallyn 2012-10-05 15:54 ` Andrew G. Morgan 2012-10-05 16:46 ` Serge Hallyn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox