Hey,
Just a probe to see what people think. I've seen two cases
in about the last month where software was confounded by
an assumption that prctl(PR_CAPBSET_DROP, CAP_SOMETHING)
would succeed if privileged, but not handling the fact
that SECURITY_FILE_CAPABILITIES=n means you can't do that.
Are we at the point yet where we feel we can get rid of
the SECURITY_FILE_CAPABILITIES=n case?
Note that there is a boot arg no_file_caps which prevents
file capabilities from being used if SECURITY_FILE_CAPABILITIES=y.
I think that's the case most users will care about, whereas the
remaining differences between CONFIG_SECURITY_FILE_CAPABILITIES=y
and =n are that with CONFIG_SECURITY_FILE_CAPABILITIES=y :
(1) certain security hooks (task_setscheduler, task_setioprio, and
task_setnice) do capability set comparisions,
(2) it is possible to drop capabilities from the bounding set,
(3) it is possible to set per-task securelevels,
(4) and it is possible to add any capability to your inheritable
set if you have CAP_SETPCAP.
Does anyone know of cases where CONFIG_SECURITY_FILE_CAPABILITIES=n
is still perceived as useful?
thanks,
-serge
On Tuesday 10 November 2009 09:07:39 am Serge E. Hallyn wrote:
> I think that's the case most users will care about, whereas the
> remaining differences between CONFIG_SECURITY_FILE_CAPABILITIES=y
> and =n are that with CONFIG_SECURITY_FILE_CAPABILITIES=y :
>
> (1) certain security hooks (task_setscheduler, task_setioprio, and
> task_setnice) do capability set comparisions,
> (2) it is possible to drop capabilities from the bounding set,
> (3) it is possible to set per-task securelevels,
> (4) and it is possible to add any capability to your inheritable
> set if you have CAP_SETPCAP.
>
> Does anyone know of cases where CONFIG_SECURITY_FILE_CAPABILITIES=n
> is still perceived as useful?
As a library writer, I wished that the kernel behavior was either consistent,
or there is an API that I can use to find out what model we are operating
under. The biggest issue is that for a distribution we know the assumptions
the distribution should be running under. But end users are free to build
their own kernel that has it disabled. This has already lead to dbus not
working at all.
I also take issue with probing the capability version number returning EINVAL
when its the only way to find out what the preferred version is.
-Steve
Quoting Steve Grubb ([email protected]):
> On Tuesday 10 November 2009 09:07:39 am Serge E. Hallyn wrote:
> > I think that's the case most users will care about, whereas the
> > remaining differences between CONFIG_SECURITY_FILE_CAPABILITIES=y
> > and =n are that with CONFIG_SECURITY_FILE_CAPABILITIES=y :
> >
> > (1) certain security hooks (task_setscheduler, task_setioprio, and
> > task_setnice) do capability set comparisions,
> > (2) it is possible to drop capabilities from the bounding set,
> > (3) it is possible to set per-task securelevels,
> > (4) and it is possible to add any capability to your inheritable
> > set if you have CAP_SETPCAP.
> >
> > Does anyone know of cases where CONFIG_SECURITY_FILE_CAPABILITIES=n
> > is still perceived as useful?
>
> As a library writer, I wished that the kernel behavior was either consistent,
> or there is an API that I can use to find out what model we are operating
> under. The biggest issue is that for a distribution we know the assumptions
> the distribution should be running under. But end users are free to build
> their own kernel that has it disabled. This has already lead to dbus not
> working at all.
>
> I also take issue with probing the capability version number returning EINVAL
> when its the only way to find out what the preferred version is.
In 2007/2008, KaiGai had floated patches to export capability info
over securityfs. If it was something library writers and distros
wanted, we could resurrect those patches - and tack on some info
about cap-related kernel config.
-serge
Hi,
On Tue, Nov 10, 2009 at 08:07:39AM -0600, Serge E. Hallyn wrote:
> Just a probe to see what people think. I've seen two cases
> in about the last month where software was confounded by
> an assumption that prctl(PR_CAPBSET_DROP, CAP_SOMETHING)
> would succeed if privileged, but not handling the fact
> that SECURITY_FILE_CAPABILITIES=n means you can't do that.
>
> Are we at the point yet where we feel we can get rid of
> the SECURITY_FILE_CAPABILITIES=n case?
It seems to me that the process caps bounding set (and file caps) are the
way forward and retaining the =n option is nonsense, especially since caps
are an integral part of the kernel.
> Does anyone know of cases where CONFIG_SECURITY_FILE_CAPABILITIES=n
> is still perceived as useful?
Building a kernel that willfully ignores fscaps? I don't see the point.
It saves only a few bytes of code, AFAICT, and if it needs to be disabled
for some reason, the kernel boot option "no_file_caps" can be set.
At the very least it should default to "y" and/or have its help updated to
include the list of things it enables.
-Kees
--
Kees Cook
Ubuntu Security Team
On Tuesday 10 November 2009 10:53:49 am Serge E. Hallyn wrote:
> > > Does anyone know of cases where CONFIG_SECURITY_FILE_CAPABILITIES=n
> > > is still perceived as useful?
> >
> >
> > As a library writer, I wished that the kernel behavior was either
> > consistent, or there is an API that I can use to find out what model we
> > are operating under. The biggest issue is that for a distribution we know
> > the assumptions the distribution should be running under. But end users
> > are free to build their own kernel that has it disabled. This has already
> > lead to dbus not working at all.
> >
> > I also take issue with probing the capability version number returning
> > EINVAL when its the only way to find out what the preferred version is.
>
> In 2007/2008, KaiGai had floated patches to export capability info
> over securityfs. If it was something library writers and distros
> wanted, we could resurrect those patches - and tack on some info
> about cap-related kernel config.
Unfortunately, I would have to support the kernels from 2.6.26->2.6.32 which
presumably don't have this facility. So, I'm kind of stuck. I think in a
previous discussion you mentioned that I could call getcap or
prctl(PR_CAPBSET_READ) and check for CAP_SETPCAP. I think I have to go that
direction for backwards compatibility.
But back to detecting the capability version number...if I pass 0 as the
version in the header, why can't the kernel just say oh you want the preferred
version number, stuff it in the header, and return the syscall with success and
not EINVAL?
Another irritation...if I want to clear the bounding set, I have to make a for
loop and call prctl 34 times (once for each bit). I'd rather see a v4
capability that takes the bounding set as part of the same syscall. Maybe all
3 of these could be fixed in the same OS release so that changing to v4 also
signifies the other behavior changes.
-Steve
Quoting Steve Grubb ([email protected]):
> On Tuesday 10 November 2009 10:53:49 am Serge E. Hallyn wrote:
> > > > Does anyone know of cases where CONFIG_SECURITY_FILE_CAPABILITIES=n
> > > > is still perceived as useful?
> > >
> > >
> > > As a library writer, I wished that the kernel behavior was either
> > > consistent, or there is an API that I can use to find out what model we
> > > are operating under. The biggest issue is that for a distribution we know
> > > the assumptions the distribution should be running under. But end users
> > > are free to build their own kernel that has it disabled. This has already
> > > lead to dbus not working at all.
> > >
> > > I also take issue with probing the capability version number returning
> > > EINVAL when its the only way to find out what the preferred version is.
> >
> > In 2007/2008, KaiGai had floated patches to export capability info
> > over securityfs. If it was something library writers and distros
> > wanted, we could resurrect those patches - and tack on some info
> > about cap-related kernel config.
>
> Unfortunately, I would have to support the kernels from 2.6.26->2.6.32 which
> presumably don't have this facility. So, I'm kind of stuck. I think in a
> previous discussion you mentioned that I could call getcap or
> prctl(PR_CAPBSET_READ) and check for CAP_SETPCAP. I think I have to go that
> direction for backwards compatibility.
Yes, I'm afraid so - unless /proc/config.gz happened to be available.
I suppose looking through /proc/1/status might be more reliable
actually, in case you were running in an already-partially-restricted
process tree.
> But back to detecting the capability version number...if I pass 0 as the
> version in the header, why can't the kernel just say oh you want the preferred
> version number, stuff it in the header, and return the syscall with success and
> not EINVAL?
This is something I believe Andrew has advocated in the past, but I
forget why. Andrew?
> Another irritation...if I want to clear the bounding set, I have to make a for
> loop and call prctl 34 times (once for each bit). I'd rather see a v4
> capability that takes the bounding set as part of the same syscall. Maybe all
> 3 of these could be fixed in the same OS release so that changing to v4 also
> signifies the other behavior changes.
I worry a bit about people confusing the bounding set as something
more flexible than it is, and/or getting lazy and using the bounding
set instead of fI|pI .vs. fP, but am not solidly against this.
Anyway, maybe we should get on thsi sooner rather than later...
Are there any other deficiencies people see in the current
API?
-serge
On Tue, Nov 10, 2009 at 4:19 PM, Serge E. Hallyn <[email protected]> wrote:
> Quoting Steve Grubb ([email protected]):
>> On Tuesday 10 November 2009 10:53:49 am Serge E. Hallyn wrote:
>> > > > Does anyone know of cases where CONFIG_SECURITY_FILE_CAPABILITIES=n
>> > > > is still perceived as useful?
[+1 for removing the option.]
>> > > As a library writer, I wished that the kernel behavior was either
>> > > consistent, ?or there is an API that I can use to find out what model we
>> > > are operating under. The biggest issue is that for a distribution we know
>> > > the assumptions the distribution should be running under. But end users
>> > > are free to build their own kernel that has it disabled. This has already
>> > > lead to dbus not working at all.
>> > >
>> > > I also take issue with probing the capability version number returning
>> > > EINVAL ?when its the only way to find out what the preferred version is.
>> >
>> > In 2007/2008, KaiGai had floated patches to export capability info
>> > over securityfs. ?If it was something library writers and distros
>> > wanted, we could resurrect those patches - and tack on some info
>> > about cap-related kernel config.
>>
>> Unfortunately, I would have to support the kernels from 2.6.26->2.6.32 which
>> presumably don't have this facility. So, I'm kind of stuck. I think in a
>> previous discussion you mentioned that I could call getcap or
>> prctl(PR_CAPBSET_READ) and check for CAP_SETPCAP. I think I have to go that
>> direction for backwards compatibility.
>
> Yes, I'm afraid so - unless /proc/config.gz happened to be available.
> I suppose looking through /proc/1/status might be more reliable
> actually, in case you were running in an already-partially-restricted
> process tree.
>
>> But back to detecting the capability version number...if I pass 0 as the
>> version in the header, why can't the kernel just say oh you want the preferred
>> version number, stuff it in the header, and return the syscall with success and
>> not EINVAL?
>
> This is something I believe Andrew has advocated in the past, but I
> forget why. ?Andrew?
This is so a library can understand that it doesn't understand the
current ABI. For example, consider the case of some kernel of the
future with a different ABI meeting an old library.
The intention is for it to fail safe and not blunder on doing
"security" related operations with an imperfect idea of the current
kernel interface.
This is how libcap figures out it can work with the hosting kernel:
http://git.kernel.org/?p=libs/libcap/libcap.git;a=blob;f=libcap/cap_alloc.c;h=5fa5e9305c146529e297285befa03e7c5521e9ba;hb=HEAD
Does that help explain?
>> Another irritation...if I want to clear the bounding set, I have to make a for
>> loop and call prctl 34 times (once for each bit). I'd rather see a v4
>> capability that takes the bounding set as part of the same syscall. Maybe all
>> 3 of these could be fixed in the same OS release so that changing to v4 also
>> signifies the other behavior changes.
Please don't do this. The semantics of the bounding set are very
different from capabilities, and much more of an overkill hack to cope
with a naive capability inheritance world view and not really anything
to do with "the" privilege model that capabilities represent.
> I worry a bit about people confusing the bounding set as something
> more ?flexible than it is, and/or getting lazy and using the bounding
> set instead of fI|pI .vs. fP, but am not solidly against this.
Completely agree with the sentiment here.
> Anyway, maybe we should get on thsi sooner rather than later...
Or, not at all... :-)
> Are there any other deficiencies people see ?in the current
> API?
I think it is worth, humbly, asking folk to read over our paper again
http://ols.fedoraproject.org/OLS/Reprints-2008/hallyn-reprint.pdf
and trying to grok what the model is before proposing changing it.
Thanks
Andrew
On Wednesday 18 November 2009 11:40:13 am Andrew G. Morgan wrote:
> >> But back to detecting the capability version number...if I pass 0 as the
> >> version in the header, why can't the kernel just say oh you want the
> >> preferred version number, stuff it in the header, and return the syscall
> >> with success and not EINVAL?
>
> This is so a library can understand that it doesn't understand the
> current ABI.
If user space is passing a NULL for the cap_user_data_t argument, user space
has a pretty good idea that its not expecting actual capabilities to be filled
in. My basic point is that there is no way to "correctly" use the capabilities
API to determine what the preferred version is.
> For example, consider the case of some kernel of the
> future with a different ABI meeting an old library.
Right. Either user space checks version numbers to verify it knows what its
doing and errors out, or it blindly tries to set capabilities and gets an
error. Either way, if user space is *really* using the API to do work, it
won't be passing NULL as the user data argument.
Maybe if version is set to 0 and NULL is being passed for arg 2 to getcap,
that would be the secret handshake that lets the kernel know all I want is the
preferred version number and nothing else since there is no data structure to
fill in.
> The intention is for it to fail safe and not blunder on doing
> "security" related operations with an imperfect idea of the current
> kernel interface.
>
> This is how libcap figures out it can work with the hosting kernel:
capget(0x20080522, 0, NULL) = -1 EFAULT (Bad address)
Still an error.
-Steve
On Wed, Nov 18, 2009 at 9:49 AM, Steve Grubb <[email protected]> wrote:
> On Wednesday 18 November 2009 11:40:13 am Andrew G. Morgan wrote:
>> >> But back to detecting the capability version number...if I pass 0 as the
>> >> version in the header, why can't the kernel just say oh you want the
>> >> preferred version number, stuff it in the header, and return the syscall
>> >> with success and not EINVAL?
>>
>> This is so a library can understand that it doesn't understand the
>> current ABI.
>
> If user space is passing a NULL for the cap_user_data_t argument, user space
> has a pretty good idea that its not expecting actual capabilities to be filled
> in. My basic point is that there is no way to "correctly" use the capabilities
> API to determine what the preferred version is.
But older kernels didn't do that.
>> For example, consider the case of some kernel of the
>> future with a different ABI meeting an old library.
>
> Right. Either user space checks version numbers to verify it knows what its
> doing and errors out, or it blindly tries to set capabilities and gets an
> error. Either way, if user space is *really* using the API to do work, it
> won't be passing NULL as the user data argument.
>
> Maybe if version is set to 0 and NULL is being passed for arg 2 to getcap,
> that would be the secret handshake that lets the kernel know all I want is the
> preferred version number and nothing else since there is no data structure to
> fill in.
>
>
>> The intention is for it to fail safe and not blunder on doing
>> "security" related operations with an imperfect idea of the current
>> kernel interface.
>>
>> This is how libcap figures out it can work with the hosting kernel:
>
> capget(0x20080522, 0, NULL) ? ? ? ? ? ? = -1 EFAULT (Bad address)
I'm not sure what this is supposed to do. This system call takes two
arguments and none of them work as your above snippet suggests.
http://git.kernel.org/?p=libs/libcap/libcap.git;a=blob;f=libcap/cap_alloc.c;h=5fa5e9305c146529e297285befa03e7c5521e9ba;hb=HEAD
SYSCALL_DEFINE2(capget, cap_user_header_t, header, cap_user_data_t, dataptr)
165 {
166 int ret = 0;
167 pid_t pid;
168 unsigned tocopy;
169 kernel_cap_t pE, pI, pP;
170
171 ret = cap_validate_magic(header, &tocopy);
172 if (ret != 0)
173 return ret;
ie., two arguments, both of which are pointers. dataptr is not touched
if you supply incorrect magic... The return at line 173 is taken if
header is explored and does not contain the correct magic (ie.
Invalid) - which it overwrites with the kernel-preferred value in the
header, and returns EINVAL...
I don't see an EFAULT problem here.
> Still an error.
Could you clarify? Perhaps with some actual code?
Thanks
Andrew
>
> -Steve
>
On Wednesday 18 November 2009 01:36:20 pm Andrew G. Morgan wrote:
> On Wed, Nov 18, 2009 at 9:49 AM, Steve Grubb <[email protected]> wrote:
> > On Wednesday 18 November 2009 11:40:13 am Andrew G. Morgan wrote:
> >> >> But back to detecting the capability version number...if I pass 0 as
> >> >> the version in the header, why can't the kernel just say oh you want
> >> >> the preferred version number, stuff it in the header, and return the
> >> >> syscall with success and not EINVAL?
> >>
> >> This is so a library can understand that it doesn't understand the
> >> current ABI.
> >
> > If user space is passing a NULL for the cap_user_data_t argument, user
> > space has a pretty good idea that its not expecting actual capabilities
> > to be filled in. My basic point is that there is no way to "correctly"
> > use the capabilities API to determine what the preferred version is.
>
> But older kernels didn't do that.
True, but now we have the problem.
> >> The intention is for it to fail safe and not blunder on doing
> >> "security" related operations with an imperfect idea of the current
> >> kernel interface.
> >>
> >> This is how libcap figures out it can work with the hosting kernel:
> >
> > capget(0x20080522, 0, NULL) = -1 EFAULT (Bad address)
>
> I'm not sure what this is supposed to do. This system call takes two
> arguments and none of them work as your above snippet suggests.
This is from running "strace /usr/sbin/getcap libcap.h". I think strace is
splitting arg 1 into its 2 elements within the structure for display purposes.
You can strace it yourself and see. :)
> SYSCALL_DEFINE2(capget, cap_user_header_t, header, cap_user_data_t,
> dataptr) 165 {
> 166 int ret = 0;
> 167 pid_t pid;
> 168 unsigned tocopy;
> 169 kernel_cap_t pE, pI, pP;
> 170
> 171 ret = cap_validate_magic(header, &tocopy);
> 172 if (ret != 0)
> 173 return ret;
>
> ie., two arguments, both of which are pointers. dataptr is not touched
> if you supply incorrect magic... The return at line 173 is taken if
> header is explored and does not contain the correct magic (ie.
> Invalid) - which it overwrites with the kernel-preferred value in the
> header, and returns EINVAL...
OK, this is the right place to make a fix. Something along the lines of:
@@ -169,8 +169,11 @@ SYSCALL_DEFINE2(capget, cap_user_header_
kernel_cap_t pE, pI, pP;
ret = cap_validate_magic(header, &tocopy);
- if (ret != 0)
+ if (ret != 0) {
+ if (ret == -EINVAL && dataptr == NULL)
+ return 0;
return ret;
+ }
if (get_user(pid, &header->pid))
return -EFAULT;
> I don't see an EFAULT problem here.
It comes when get_user fails above.
-Steve
On Wed, Nov 18, 2009 at 11:33 AM, Steve Grubb <[email protected]> wrote:
> On Wednesday 18 November 2009 01:36:20 pm Andrew G. Morgan wrote:
>> On Wed, Nov 18, 2009 at 9:49 AM, Steve Grubb <[email protected]> wrote:
>> > On Wednesday 18 November 2009 11:40:13 am Andrew G. Morgan wrote:
>> >> >> But back to detecting the capability version number...if I pass 0 as
>> >> >> the version in the header, why can't the kernel just say oh you want
>> >> >> the preferred version number, stuff it in the header, and return the
>> >> >> syscall with success and not EINVAL?
>> >>
>> >> This is so a library can understand that it doesn't understand the
>> >> current ABI.
>> >
>> > If user space is passing a NULL for the cap_user_data_t argument, user
>> > space has a pretty good idea that its not expecting actual capabilities
>> > to be filled in. My basic point is that there is no way to "correctly"
>> > use the capabilities API to determine what the preferred version is.
>>
>> But older kernels didn't do that.
>
> True, but now we have the problem.
>
>
>> >> The intention is for it to fail safe and not blunder on doing
>> >> "security" related operations with an imperfect idea of the current
>> >> kernel interface.
>> >>
>> >> This is how libcap figures out it can work with the hosting kernel:
>> >
>> > capget(0x20080522, 0, NULL) ? ? ? ? ? ? = -1 EFAULT (Bad address)
>>
>> I'm not sure what this is supposed to do. This system call takes two
>> arguments and none of them work as your above snippet suggests.
>
> This is from running "strace /usr/sbin/getcap libcap.h". I think strace is
> splitting arg 1 into its 2 elements within the structure for display purposes.
> You can strace it yourself and see. :)
>
>
>> SYSCALL_DEFINE2(capget, cap_user_header_t, header, cap_user_data_t,
>> ?dataptr) 165 {
>> ?166 ? ? ? ? int ret = 0;
>> ?167 ? ? ? ? pid_t pid;
>> ?168 ? ? ? ? unsigned tocopy;
>> ?169 ? ? ? ? kernel_cap_t pE, pI, pP;
>> ?170
>> ?171 ? ? ? ? ret = cap_validate_magic(header, &tocopy);
>> ?172 ? ? ? ? if (ret != 0)
>> ?173 ? ? ? ? ? ? ? ? return ret;
>>
>> ie., two arguments, both of which are pointers. dataptr is not touched
>> if you supply incorrect magic... The return at line 173 is taken if
>> header is explored and does not contain the correct magic (ie.
>> Invalid) - which it overwrites with the kernel-preferred value in the
>> header, and returns EINVAL...
>
> OK, this is the right place to make a fix. Something along the lines of:
>
> @@ -169,8 +169,11 @@ SYSCALL_DEFINE2(capget, cap_user_header_
> ? ? ? ?kernel_cap_t pE, pI, pP;
>
> ? ? ? ?ret = cap_validate_magic(header, &tocopy);
> - ? ? ? if (ret != 0)
> + ? ? ? if (ret != 0) {
> + ? ? ? ? ? ? ? if (ret == -EINVAL && dataptr == NULL)
> + ? ? ? ? ? ? ? ? ? ? ? return 0;
> ? ? ? ? ? ? ? ?return ret;
> + ? ? ? }
>
> ? ? ? ?if (get_user(pid, &header->pid))
> ? ? ? ? ? ? ? ?return -EFAULT;
>
>
>> I don't see an EFAULT problem here.
>
> It comes when get_user fails above.
So, how are you expecting to get the prevailing kernel's cap-version
(magic) from the kernel if not via the header object? If you supply a
NULL for the header, what is sys_capget supposed to use to communicate
the value back to user space?
Cheers
Andrew
>
> -Steve
>
Steve,
[We chatted offline. I'm still not sure I agree with the "now we have
the problem" bit since what is there today does work... However,
you've persuaded me that the system call error is ugly/noisy and not a
compatibility requirement of pre-existing code, provided that a NULL
is passed for dataptr -- that is, I've gone back and looked at older
libcaps and older kernels and checked. So...]
How about this change?
@@ -169,8 +169,11 @@ SYSCALL_DEFINE2(capget, cap_user_header_
kernel_cap_t pE, pI, pP;
ret = cap_validate_magic(header, &tocopy);
- if (ret != 0)
+ if ((ret != 0) || (dataptr == NULL)) {
+ if ((ret == -EINVAL) && (dataptr == NULL))
+ return 0;
return ret;
+ }
if (get_user(pid, &header->pid))
return -EFAULT;
? This is a slightly modified version of what you posted before.
Specifically, in the case that the user guessed a compatible version
this NULL call will succeed and not EFAULT.
Cheers
Andrew
On Wed, Nov 18, 2009 at 11:33 AM, Steve Grubb <[email protected]> wrote:
> On Wednesday 18 November 2009 01:36:20 pm Andrew G. Morgan wrote:
>> On Wed, Nov 18, 2009 at 9:49 AM, Steve Grubb <[email protected]> wrote:
>> > On Wednesday 18 November 2009 11:40:13 am Andrew G. Morgan wrote:
>> >> >> But back to detecting the capability version number...if I pass 0 as
>> >> >> the version in the header, why can't the kernel just say oh you want
>> >> >> the preferred version number, stuff it in the header, and return the
>> >> >> syscall with success and not EINVAL?
>> >>
>> >> This is so a library can understand that it doesn't understand the
>> >> current ABI.
>> >
>> > If user space is passing a NULL for the cap_user_data_t argument, user
>> > space has a pretty good idea that its not expecting actual capabilities
>> > to be filled in. My basic point is that there is no way to "correctly"
>> > use the capabilities API to determine what the preferred version is.
>>
>> But older kernels didn't do that.
>
> True, but now we have the problem.
>
>
>> >> The intention is for it to fail safe and not blunder on doing
>> >> "security" related operations with an imperfect idea of the current
>> >> kernel interface.
>> >>
>> >> This is how libcap figures out it can work with the hosting kernel:
>> >
>> > capget(0x20080522, 0, NULL) ? ? ? ? ? ? = -1 EFAULT (Bad address)
>>
>> I'm not sure what this is supposed to do. This system call takes two
>> arguments and none of them work as your above snippet suggests.
>
> This is from running "strace /usr/sbin/getcap libcap.h". I think strace is
> splitting arg 1 into its 2 elements within the structure for display purposes.
> You can strace it yourself and see. :)
>
>
>> SYSCALL_DEFINE2(capget, cap_user_header_t, header, cap_user_data_t,
>> ?dataptr) 165 {
>> ?166 ? ? ? ? int ret = 0;
>> ?167 ? ? ? ? pid_t pid;
>> ?168 ? ? ? ? unsigned tocopy;
>> ?169 ? ? ? ? kernel_cap_t pE, pI, pP;
>> ?170
>> ?171 ? ? ? ? ret = cap_validate_magic(header, &tocopy);
>> ?172 ? ? ? ? if (ret != 0)
>> ?173 ? ? ? ? ? ? ? ? return ret;
>>
>> ie., two arguments, both of which are pointers. dataptr is not touched
>> if you supply incorrect magic... The return at line 173 is taken if
>> header is explored and does not contain the correct magic (ie.
>> Invalid) - which it overwrites with the kernel-preferred value in the
>> header, and returns EINVAL...
>
> OK, this is the right place to make a fix. Something along the lines of:
>
> @@ -169,8 +169,11 @@ SYSCALL_DEFINE2(capget, cap_user_header_
> ? ? ? ?kernel_cap_t pE, pI, pP;
>
> ? ? ? ?ret = cap_validate_magic(header, &tocopy);
> - ? ? ? if (ret != 0)
> + ? ? ? if (ret != 0) {
> + ? ? ? ? ? ? ? if (ret == -EINVAL && dataptr == NULL)
> + ? ? ? ? ? ? ? ? ? ? ? return 0;
> ? ? ? ? ? ? ? ?return ret;
> + ? ? ? }
>
> ? ? ? ?if (get_user(pid, &header->pid))
> ? ? ? ? ? ? ? ?return -EFAULT;
>
>
>> I don't see an EFAULT problem here.
>
> It comes when get_user fails above.
>
> -Steve
>
On Thursday 19 November 2009 10:35:12 am Andrew G. Morgan wrote:
> How about this change?
>
> @@ -169,8 +169,11 @@ SYSCALL_DEFINE2(capget, cap_user_header_
> kernel_cap_t pE, pI, pP;
>
> ret = cap_validate_magic(header, &tocopy);
> - if (ret != 0)
> + if ((ret != 0) || (dataptr == NULL)) {
> + if ((ret == -EINVAL) && (dataptr == NULL))
> + return 0;
> return ret;
> + }
>
> if (get_user(pid, &header->pid))
> return -EFAULT;
>
> ? This is a slightly modified version of what you posted before.
> Specifically, in the case that the user guessed a compatible version
> this NULL call will succeed and not EFAULT.
Sure. Looks good to me.
Thanks,
-Steve