* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.