Re: [PATCH] kernel: arg2 is unsigned long which is never < 0
于 2013年02月06日 23:24, Serge Hallyn 写道: > This really seems like splitting hairs to me. > > Acked-by: Serge E. Hallyn > > on the original patch. > > thanks, > -serge thank you. -- Chen Gang Asianux Corporation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never < 0
于 2013年02月07日 03:41, Kees Cook 写道: > Well, it seems we'd need to add an include to gain access to binfmts.h > in once place, but that doesn't seem bad. I'll send to patch to clean > this up. thank you. and after your patch is integrated into main branch (at least in next-* branch), I should send patch v2 for it, too. and excuse me I will 'disappear' during next 7-10 days for Chinese Year (Spring Festival). during these days, welcome any members to send patch v2 instead of me (better to mark Reported-by Cyrill Gorcunov , too) thanks. :-) -- Chen Gang Asianux Corporation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never < 0
On Wed, Feb 6, 2013 at 2:36 AM, Chen Gang wrote:
> 于 2013年02月06日 16:56, Cyrill Gorcunov 写道:
>> On Wed, Feb 06, 2013 at 04:44:35PM +0800, Chen Gang wrote:
>>> >
>>> > diff --git a/kernel/sys.c b/kernel/sys.c
>>> > index 24d1ef5..568b9ca 100644
>>> > --- a/kernel/sys.c
>>> > +++ b/kernel/sys.c
>>> > @@ -2027,7 +2027,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long,
>>> > arg2, unsigned long, arg3,
>>> >error = get_dumpable(me->mm);
>>> >break;
>>> >case PR_SET_DUMPABLE:
>>> > - if (arg2 < 0 || arg2 > 1) {
>>> > + if (arg2 > 1) {
>>> >error = -EINVAL;
>>> >break;
>>> >}
>> I guess
>>
>> if (arg2 != SUID_DUMPABLE_DISABLED &&
>> arg2 != SUID_DUMPABLE_ENABLED) {
>> error = -EINVAL;
>> break;
>> }
>>
>> would be better. Still, current patch looks good to me.
>
> thank you for your suggestion, firstly.
>
> and after read more, it seems a little more complex:
> for me, I think it would be better:
>
> if (arg2 != SUID_DUMP_DISABLE && arg2 != SUID_DUMP_USER) {
> error = -EINVAL;
> break;
> }
>
>
> the reason is below:
>
> it has 2 branches:
>
> branch 1:
>
> #define SUID_DUMP_DISABLE 0 /* No setuid dumping */
> #define SUID_DUMP_USER 1 /* Dump as user of process */
> #define SUID_DUMP_ROOT 2 /* Dump as root */
>
> in patch d6e711448137ca3301512cec41a2c2ce852b3d0a
> Signed-of-by Alan Cox in 2005.
> define these constant for using.
> change 2 choices to 3 choices (add a new choice).
Ah, good find. These weren't used anywhere in the kernel, so I didn't see them.
> in patch abf75a5033d4da7b8a7e92321d74021d1fcfb502
> Signed-of-by Marcel Holtmann in 2006.
> find and fix a security issue for it.
>
>
> branch 2:
>
> #define SUID_DUMPABLE_DISABLED 0
> #define SUID_DUMPABLE_ENABLED 1
> #define SUID_DUMPABLE_SAFE 2
>
> in patch 54b501992dd2a839e94e76aa392c392b55080ce8
> Signed-of-by Kees Cook in Jul 30 2012
> define the constants for using
> print warning when detect unsafe core_pattern settings
>
> in patch 0f4cfb2e4e7a7e4e97a3e90e2ba1062f07fb2cb1
> Signed-of-by Oleg Nesterov in Oct 4 2012
> use SUID_DUMPABLE_ENABLED rather than hardcoded 1
>
> analysing:
> branch 1 and branch 2 have the same values with different macro names.
> branch 1 is much older than branch 2.
> for features:
> branch 1 is for functional feature and bug fix,
> branch 2 is for printing warning and beautifying code.
>
> it seems:
> branch 2 did not notice the branch 1, before it performs.
> if it noticed, it is meanless to define the new macros.
Well, it seems we'd need to add an include to gain access to binfmts.h
in once place, but that doesn't seem bad. I'll send to patch to clean
this up.
>
> result:
> still use the macros of branch 1
> and use branch 1 macros instead of branch 2 macros (need an additional
> patch).
>
> :-)
>
> Regards.
>
> --
> Chen Gang
>
> Asianux Corporation
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never < 0
On Wed, Feb 06, 2013 at 06:36:38PM +0800, Chen Gang wrote: ... > > result: > still use the macros of branch 1 > and use branch 1 macros instead of branch 2 macros (need an additional > patch). Oh, what a mess ;) The reason I used SUID_DUMPABLE_DISABLED -- because it's used in set_dumpable which is called directly from this prctl. I'm fine with either way, plain [0;1] is acceptable as well still I don;t like it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never < 0
Quoting Chen Gang (gang.c...@asianux.com):
> 于 2013年02月06日 16:56, Cyrill Gorcunov 写道:
> > On Wed, Feb 06, 2013 at 04:44:35PM +0800, Chen Gang wrote:
> >> >
> >> > diff --git a/kernel/sys.c b/kernel/sys.c
> >> > index 24d1ef5..568b9ca 100644
> >> > --- a/kernel/sys.c
> >> > +++ b/kernel/sys.c
> >> > @@ -2027,7 +2027,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long,
> >> > arg2, unsigned long, arg3,
> >> > error = get_dumpable(me->mm);
> >> > break;
> >> > case PR_SET_DUMPABLE:
> >> > -if (arg2 < 0 || arg2 > 1) {
> >> > +if (arg2 > 1) {
> >> > error = -EINVAL;
> >> > break;
> >> > }
> > I guess
> >
> > if (arg2 != SUID_DUMPABLE_DISABLED &&
> > arg2 != SUID_DUMPABLE_ENABLED) {
> > error = -EINVAL;
> > break;
> > }
> >
> > would be better. Still, current patch looks good to me.
>
> thank you for your suggestion, firstly.
>
> and after read more, it seems a little more complex:
> for me, I think it would be better:
>
> if (arg2 != SUID_DUMP_DISABLE && arg2 != SUID_DUMP_USER) {
> error = -EINVAL;
> break;
> }
>
>
> the reason is below:
>
> it has 2 branches:
>
> branch 1:
>
> #define SUID_DUMP_DISABLE 0 /* No setuid dumping */
> #define SUID_DUMP_USER 1 /* Dump as user of process */
> #define SUID_DUMP_ROOT 2 /* Dump as root */
>
> in patch d6e711448137ca3301512cec41a2c2ce852b3d0a
> Signed-of-by Alan Cox in 2005.
> define these constant for using.
> change 2 choices to 3 choices (add a new choice).
>
> in patch abf75a5033d4da7b8a7e92321d74021d1fcfb502
> Signed-of-by Marcel Holtmann in 2006.
> find and fix a security issue for it.
>
>
> branch 2:
>
> #define SUID_DUMPABLE_DISABLED 0
> #define SUID_DUMPABLE_ENABLED 1
> #define SUID_DUMPABLE_SAFE 2
>
> in patch 54b501992dd2a839e94e76aa392c392b55080ce8
> Signed-of-by Kees Cook in Jul 30 2012
> define the constants for using
> print warning when detect unsafe core_pattern settings
>
> in patch 0f4cfb2e4e7a7e4e97a3e90e2ba1062f07fb2cb1
> Signed-of-by Oleg Nesterov in Oct 4 2012
> use SUID_DUMPABLE_ENABLED rather than hardcoded 1
>
> analysing:
> branch 1 and branch 2 have the same values with different macro names.
> branch 1 is much older than branch 2.
> for features:
> branch 1 is for functional feature and bug fix,
> branch 2 is for printing warning and beautifying code.
>
> it seems:
> branch 2 did not notice the branch 1, before it performs.
> if it noticed, it is meanless to define the new macros.
>
> result:
> still use the macros of branch 1
> and use branch 1 macros instead of branch 2 macros (need an additional
> patch).
>
> :-)
>
> Regards.
This really seems like splitting hairs to me.
Acked-by: Serge E. Hallyn
on the original patch.
thanks,
-serge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never < 0
于 2013年02月06日 16:56, Cyrill Gorcunov 写道:
> On Wed, Feb 06, 2013 at 04:44:35PM +0800, Chen Gang wrote:
>> >
>> > diff --git a/kernel/sys.c b/kernel/sys.c
>> > index 24d1ef5..568b9ca 100644
>> > --- a/kernel/sys.c
>> > +++ b/kernel/sys.c
>> > @@ -2027,7 +2027,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long,
>> > arg2, unsigned long, arg3,
>> >error = get_dumpable(me->mm);
>> >break;
>> >case PR_SET_DUMPABLE:
>> > - if (arg2 < 0 || arg2 > 1) {
>> > + if (arg2 > 1) {
>> >error = -EINVAL;
>> >break;
>> >}
> I guess
>
> if (arg2 != SUID_DUMPABLE_DISABLED &&
> arg2 != SUID_DUMPABLE_ENABLED) {
> error = -EINVAL;
> break;
> }
>
> would be better. Still, current patch looks good to me.
thank you for your suggestion, firstly.
and after read more, it seems a little more complex:
for me, I think it would be better:
if (arg2 != SUID_DUMP_DISABLE && arg2 != SUID_DUMP_USER) {
error = -EINVAL;
break;
}
the reason is below:
it has 2 branches:
branch 1:
#define SUID_DUMP_DISABLE 0 /* No setuid dumping */
#define SUID_DUMP_USER 1 /* Dump as user of process */
#define SUID_DUMP_ROOT 2 /* Dump as root */
in patch d6e711448137ca3301512cec41a2c2ce852b3d0a
Signed-of-by Alan Cox in 2005.
define these constant for using.
change 2 choices to 3 choices (add a new choice).
in patch abf75a5033d4da7b8a7e92321d74021d1fcfb502
Signed-of-by Marcel Holtmann in 2006.
find and fix a security issue for it.
branch 2:
#define SUID_DUMPABLE_DISABLED 0
#define SUID_DUMPABLE_ENABLED 1
#define SUID_DUMPABLE_SAFE 2
in patch 54b501992dd2a839e94e76aa392c392b55080ce8
Signed-of-by Kees Cook in Jul 30 2012
define the constants for using
print warning when detect unsafe core_pattern settings
in patch 0f4cfb2e4e7a7e4e97a3e90e2ba1062f07fb2cb1
Signed-of-by Oleg Nesterov in Oct 4 2012
use SUID_DUMPABLE_ENABLED rather than hardcoded 1
analysing:
branch 1 and branch 2 have the same values with different macro names.
branch 1 is much older than branch 2.
for features:
branch 1 is for functional feature and bug fix,
branch 2 is for printing warning and beautifying code.
it seems:
branch 2 did not notice the branch 1, before it performs.
if it noticed, it is meanless to define the new macros.
result:
still use the macros of branch 1
and use branch 1 macros instead of branch 2 macros (need an additional patch).
:-)
Regards.
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never < 0
On Wed, Feb 06, 2013 at 04:44:35PM +0800, Chen Gang wrote:
>
> arg2 will never < 0, for its type is 'unsigned long'
>
> so delete the waste code.
>
>
> Signed-off-by: Chen Gang
> ---
> kernel/sys.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 24d1ef5..568b9ca 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2027,7 +2027,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long,
> arg2, unsigned long, arg3,
> error = get_dumpable(me->mm);
> break;
> case PR_SET_DUMPABLE:
> - if (arg2 < 0 || arg2 > 1) {
> + if (arg2 > 1) {
> error = -EINVAL;
> break;
> }
I guess
if (arg2 != SUID_DUMPABLE_DISABLED &&
arg2 != SUID_DUMPABLE_ENABLED) {
error = -EINVAL;
break;
}
would be better. Still, current patch looks good to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] kernel: arg2 is unsigned long which is never < 0
arg2 will never < 0, for its type is 'unsigned long'
so delete the waste code.
Signed-off-by: Chen Gang
---
kernel/sys.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 24d1ef5..568b9ca 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2027,7 +2027,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2,
unsigned long, arg3,
error = get_dumpable(me->mm);
break;
case PR_SET_DUMPABLE:
- if (arg2 < 0 || arg2 > 1) {
+ if (arg2 > 1) {
error = -EINVAL;
break;
}
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] kernel: arg2 is unsigned long which is never 0
arg2 will never 0, for its type is 'unsigned long'
so delete the waste code.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
kernel/sys.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 24d1ef5..568b9ca 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2027,7 +2027,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2,
unsigned long, arg3,
error = get_dumpable(me-mm);
break;
case PR_SET_DUMPABLE:
- if (arg2 0 || arg2 1) {
+ if (arg2 1) {
error = -EINVAL;
break;
}
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never 0
On Wed, Feb 06, 2013 at 04:44:35PM +0800, Chen Gang wrote:
arg2 will never 0, for its type is 'unsigned long'
so delete the waste code.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
kernel/sys.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index 24d1ef5..568b9ca 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2027,7 +2027,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long,
arg2, unsigned long, arg3,
error = get_dumpable(me-mm);
break;
case PR_SET_DUMPABLE:
- if (arg2 0 || arg2 1) {
+ if (arg2 1) {
error = -EINVAL;
break;
}
I guess
if (arg2 != SUID_DUMPABLE_DISABLED
arg2 != SUID_DUMPABLE_ENABLED) {
error = -EINVAL;
break;
}
would be better. Still, current patch looks good to me.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never 0
于 2013年02月06日 16:56, Cyrill Gorcunov 写道:
On Wed, Feb 06, 2013 at 04:44:35PM +0800, Chen Gang wrote:
diff --git a/kernel/sys.c b/kernel/sys.c
index 24d1ef5..568b9ca 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2027,7 +2027,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long,
arg2, unsigned long, arg3,
error = get_dumpable(me-mm);
break;
case PR_SET_DUMPABLE:
- if (arg2 0 || arg2 1) {
+ if (arg2 1) {
error = -EINVAL;
break;
}
I guess
if (arg2 != SUID_DUMPABLE_DISABLED
arg2 != SUID_DUMPABLE_ENABLED) {
error = -EINVAL;
break;
}
would be better. Still, current patch looks good to me.
thank you for your suggestion, firstly.
and after read more, it seems a little more complex:
for me, I think it would be better:
if (arg2 != SUID_DUMP_DISABLE arg2 != SUID_DUMP_USER) {
error = -EINVAL;
break;
}
the reason is below:
it has 2 branches:
branch 1:
#define SUID_DUMP_DISABLE 0 /* No setuid dumping */
#define SUID_DUMP_USER 1 /* Dump as user of process */
#define SUID_DUMP_ROOT 2 /* Dump as root */
in patch d6e711448137ca3301512cec41a2c2ce852b3d0a
Signed-of-by Alan Cox in 2005.
define these constant for using.
change 2 choices to 3 choices (add a new choice).
in patch abf75a5033d4da7b8a7e92321d74021d1fcfb502
Signed-of-by Marcel Holtmann in 2006.
find and fix a security issue for it.
branch 2:
#define SUID_DUMPABLE_DISABLED 0
#define SUID_DUMPABLE_ENABLED 1
#define SUID_DUMPABLE_SAFE 2
in patch 54b501992dd2a839e94e76aa392c392b55080ce8
Signed-of-by Kees Cook in Jul 30 2012
define the constants for using
print warning when detect unsafe core_pattern settings
in patch 0f4cfb2e4e7a7e4e97a3e90e2ba1062f07fb2cb1
Signed-of-by Oleg Nesterov in Oct 4 2012
use SUID_DUMPABLE_ENABLED rather than hardcoded 1
analysing:
branch 1 and branch 2 have the same values with different macro names.
branch 1 is much older than branch 2.
for features:
branch 1 is for functional feature and bug fix,
branch 2 is for printing warning and beautifying code.
it seems:
branch 2 did not notice the branch 1, before it performs.
if it noticed, it is meanless to define the new macros.
result:
still use the macros of branch 1
and use branch 1 macros instead of branch 2 macros (need an additional patch).
:-)
Regards.
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never 0
Quoting Chen Gang (gang.c...@asianux.com):
于 2013年02月06日 16:56, Cyrill Gorcunov 写道:
On Wed, Feb 06, 2013 at 04:44:35PM +0800, Chen Gang wrote:
diff --git a/kernel/sys.c b/kernel/sys.c
index 24d1ef5..568b9ca 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2027,7 +2027,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long,
arg2, unsigned long, arg3,
error = get_dumpable(me-mm);
break;
case PR_SET_DUMPABLE:
-if (arg2 0 || arg2 1) {
+if (arg2 1) {
error = -EINVAL;
break;
}
I guess
if (arg2 != SUID_DUMPABLE_DISABLED
arg2 != SUID_DUMPABLE_ENABLED) {
error = -EINVAL;
break;
}
would be better. Still, current patch looks good to me.
thank you for your suggestion, firstly.
and after read more, it seems a little more complex:
for me, I think it would be better:
if (arg2 != SUID_DUMP_DISABLE arg2 != SUID_DUMP_USER) {
error = -EINVAL;
break;
}
the reason is below:
it has 2 branches:
branch 1:
#define SUID_DUMP_DISABLE 0 /* No setuid dumping */
#define SUID_DUMP_USER 1 /* Dump as user of process */
#define SUID_DUMP_ROOT 2 /* Dump as root */
in patch d6e711448137ca3301512cec41a2c2ce852b3d0a
Signed-of-by Alan Cox in 2005.
define these constant for using.
change 2 choices to 3 choices (add a new choice).
in patch abf75a5033d4da7b8a7e92321d74021d1fcfb502
Signed-of-by Marcel Holtmann in 2006.
find and fix a security issue for it.
branch 2:
#define SUID_DUMPABLE_DISABLED 0
#define SUID_DUMPABLE_ENABLED 1
#define SUID_DUMPABLE_SAFE 2
in patch 54b501992dd2a839e94e76aa392c392b55080ce8
Signed-of-by Kees Cook in Jul 30 2012
define the constants for using
print warning when detect unsafe core_pattern settings
in patch 0f4cfb2e4e7a7e4e97a3e90e2ba1062f07fb2cb1
Signed-of-by Oleg Nesterov in Oct 4 2012
use SUID_DUMPABLE_ENABLED rather than hardcoded 1
analysing:
branch 1 and branch 2 have the same values with different macro names.
branch 1 is much older than branch 2.
for features:
branch 1 is for functional feature and bug fix,
branch 2 is for printing warning and beautifying code.
it seems:
branch 2 did not notice the branch 1, before it performs.
if it noticed, it is meanless to define the new macros.
result:
still use the macros of branch 1
and use branch 1 macros instead of branch 2 macros (need an additional
patch).
:-)
Regards.
This really seems like splitting hairs to me.
Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com
on the original patch.
thanks,
-serge
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never 0
On Wed, Feb 06, 2013 at 06:36:38PM +0800, Chen Gang wrote: ... result: still use the macros of branch 1 and use branch 1 macros instead of branch 2 macros (need an additional patch). Oh, what a mess ;) The reason I used SUID_DUMPABLE_DISABLED -- because it's used in set_dumpable which is called directly from this prctl. I'm fine with either way, plain [0;1] is acceptable as well still I don;t like it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never 0
On Wed, Feb 6, 2013 at 2:36 AM, Chen Gang gang.c...@asianux.com wrote:
于 2013年02月06日 16:56, Cyrill Gorcunov 写道:
On Wed, Feb 06, 2013 at 04:44:35PM +0800, Chen Gang wrote:
diff --git a/kernel/sys.c b/kernel/sys.c
index 24d1ef5..568b9ca 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2027,7 +2027,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long,
arg2, unsigned long, arg3,
error = get_dumpable(me-mm);
break;
case PR_SET_DUMPABLE:
- if (arg2 0 || arg2 1) {
+ if (arg2 1) {
error = -EINVAL;
break;
}
I guess
if (arg2 != SUID_DUMPABLE_DISABLED
arg2 != SUID_DUMPABLE_ENABLED) {
error = -EINVAL;
break;
}
would be better. Still, current patch looks good to me.
thank you for your suggestion, firstly.
and after read more, it seems a little more complex:
for me, I think it would be better:
if (arg2 != SUID_DUMP_DISABLE arg2 != SUID_DUMP_USER) {
error = -EINVAL;
break;
}
the reason is below:
it has 2 branches:
branch 1:
#define SUID_DUMP_DISABLE 0 /* No setuid dumping */
#define SUID_DUMP_USER 1 /* Dump as user of process */
#define SUID_DUMP_ROOT 2 /* Dump as root */
in patch d6e711448137ca3301512cec41a2c2ce852b3d0a
Signed-of-by Alan Cox in 2005.
define these constant for using.
change 2 choices to 3 choices (add a new choice).
Ah, good find. These weren't used anywhere in the kernel, so I didn't see them.
in patch abf75a5033d4da7b8a7e92321d74021d1fcfb502
Signed-of-by Marcel Holtmann in 2006.
find and fix a security issue for it.
branch 2:
#define SUID_DUMPABLE_DISABLED 0
#define SUID_DUMPABLE_ENABLED 1
#define SUID_DUMPABLE_SAFE 2
in patch 54b501992dd2a839e94e76aa392c392b55080ce8
Signed-of-by Kees Cook in Jul 30 2012
define the constants for using
print warning when detect unsafe core_pattern settings
in patch 0f4cfb2e4e7a7e4e97a3e90e2ba1062f07fb2cb1
Signed-of-by Oleg Nesterov in Oct 4 2012
use SUID_DUMPABLE_ENABLED rather than hardcoded 1
analysing:
branch 1 and branch 2 have the same values with different macro names.
branch 1 is much older than branch 2.
for features:
branch 1 is for functional feature and bug fix,
branch 2 is for printing warning and beautifying code.
it seems:
branch 2 did not notice the branch 1, before it performs.
if it noticed, it is meanless to define the new macros.
Well, it seems we'd need to add an include to gain access to binfmts.h
in once place, but that doesn't seem bad. I'll send to patch to clean
this up.
result:
still use the macros of branch 1
and use branch 1 macros instead of branch 2 macros (need an additional
patch).
:-)
Regards.
--
Chen Gang
Asianux Corporation
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never 0
于 2013年02月07日 03:41, Kees Cook 写道: Well, it seems we'd need to add an include to gain access to binfmts.h in once place, but that doesn't seem bad. I'll send to patch to clean this up. thank you. and after your patch is integrated into main branch (at least in next-* branch), I should send patch v2 for it, too. and excuse me I will 'disappear' during next 7-10 days for Chinese Year (Spring Festival). during these days, welcome any members to send patch v2 instead of me (better to mark Reported-by Cyrill Gorcunov gorcu...@openvz.org, too) thanks. :-) -- Chen Gang Asianux Corporation -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel: arg2 is unsigned long which is never 0
于 2013年02月06日 23:24, Serge Hallyn 写道: This really seems like splitting hairs to me. Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com on the original patch. thanks, -serge thank you. -- Chen Gang Asianux Corporation -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
