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/
