Messages in this thread |  | | Date | Wed, 6 Feb 2013 09:24:34 -0600 | From | Serge Hallyn <> | Subject | Re: [PATCH] kernel: arg2 is unsigned long which is never < 0 |
| |
Quoting Chen Gang (gang.chen@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.hallyn@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 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/
|  |