2002-06-04 21:15:20

by Robert Love

[permalink] [raw]
Subject: [PATCH] remove suser()

Linus,

Attached patch replaces the lone remaining suser() call with capable()
and then removes suser() itself in a triumphant celebration of the glory
of capable(). Or something. ;-)

Small cleanup of capable() and some comments, too.

Patch is against 2.5.20, please apply.

Robert Love

diff -urN linux-2.5.20/drivers/net/wan/pc300_drv.c linux/drivers/net/wan/pc300_drv.c
--- linux-2.5.20/drivers/net/wan/pc300_drv.c Sun Jun 2 18:44:53 2002
+++ linux/drivers/net/wan/pc300_drv.c Tue Jun 4 13:56:54 2002
@@ -2564,7 +2564,7 @@
return -EINVAL;
return 0;
case SIOCSPC300CONF:
- if (!suser())
+ if (!capable(CAP_NET_ADMIN))
return -EPERM;
if (!arg ||
copy_from_user(&conf_aux.conf, arg, sizeof(pc300chconf_t)))
diff -urN linux-2.5.20/include/linux/compatmac.h linux/include/linux/compatmac.h
--- linux-2.5.20/include/linux/compatmac.h Sun Jun 2 18:44:41 2002
+++ linux/include/linux/compatmac.h Tue Jun 4 13:57:33 2002
@@ -102,8 +102,6 @@

#define my_iounmap(x, b) (((long)x<0x100000)?0:vfree ((void*)x))

-#define capable(x) suser()
-
#define tty_flip_buffer_push(tty) queue_task(&tty->flip.tqueue, &tq_timer)
#define signal_pending(current) (current->signal & ~current->blocked)
#define schedule_timeout(to) do {current->timeout = jiffies + (to);schedule ();} while (0)
diff -urN linux-2.5.20/include/linux/sched.h linux/include/linux/sched.h
--- linux-2.5.20/include/linux/sched.h Sun Jun 2 18:44:41 2002
+++ linux/include/linux/sched.h Tue Jun 4 14:03:35 2002
@@ -588,24 +588,10 @@
* This has now become a routine instead of a macro, it sets a flag if
* it returns true (to do BSD-style accounting where the process is flagged
* if it uses root privs). The implication of this is that you should do
- * normal permissions checks first, and check suser() last.
+ * normal permissions checks first, and check fsuser() last.
*
- * [Dec 1997 -- Chris Evans]
- * For correctness, the above considerations need to be extended to
- * fsuser(). This is done, along with moving fsuser() checks to be
- * last.
- *
- * These will be removed, but in the mean time, when the SECURE_NOROOT
- * flag is set, uids don't grant privilege.
+ * suser() is gone, fsuser() should go soon too...
*/
-static inline int suser(void)
-{
- if (!issecure(SECURE_NOROOT) && current->euid == 0) {
- current->flags |= PF_SUPERPRIV;
- return 1;
- }
- return 0;
-}

static inline int fsuser(void)
{
@@ -617,19 +603,12 @@
}

/*
- * capable() checks for a particular capability.
- * New privilege checks should use this interface, rather than suser() or
- * fsuser(). See include/linux/capability.h for defined capabilities.
+ * capable() checks for a particular capability.
+ * See include/linux/capability.h for defined capabilities.
*/
-
static inline int capable(int cap)
{
-#if 1 /* ok now */
- if (cap_raised(current->cap_effective, cap))
-#else
- if (cap_is_fs_cap(cap) ? current->fsuid == 0 : current->euid == 0)
-#endif
- {
+ if (cap_raised(current->cap_effective, cap)) {
current->flags |= PF_SUPERPRIV;
return 1;
}




2002-06-06 22:07:06

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] remove suser()

Hi!

> Attached patch replaces the lone remaining suser() call with capable()
> and then removes suser() itself in a triumphant celebration of the glory
> of capable(). Or something. ;-)
>
> Small cleanup of capable() and some comments, too.
>
> Patch is against 2.5.20, please apply.
>
> Robert Love
>
> diff -urN linux-2.5.20/drivers/net/wan/pc300_drv.c linux/drivers/net/wan/pc300_drv.c
> --- linux-2.5.20/drivers/net/wan/pc300_drv.c Sun Jun 2 18:44:53 2002
> +++ linux/drivers/net/wan/pc300_drv.c Tue Jun 4 13:56:54 2002
> @@ -2564,7 +2564,7 @@
> return -EINVAL;
> return 0;
> case SIOCSPC300CONF:
> - if (!suser())
> + if (!capable(CAP_NET_ADMIN))
> return -EPERM;
> if (!arg ||
> copy_from_user(&conf_aux.conf, arg, sizeof(pc300chconf_t)))
> diff -urN linux-2.5.20/include/linux/compatmac.h linux/include/linux/compatmac.h
> --- linux-2.5.20/include/linux/compatmac.h Sun Jun 2 18:44:41 2002
> +++ linux/include/linux/compatmac.h Tue Jun 4 13:57:33 2002
> @@ -102,8 +102,6 @@
>
> #define my_iounmap(x, b) (((long)x<0x100000)?0:vfree ((void*)x))
>
> -#define capable(x) suser()
> -
> #define tty_flip_buffer_push(tty) queue_task(&tty->flip.tqueue, &tq_timer)

This is not right I believe. You want to keep compatibility for older
kernels.

Pavel
--
(about SSSCA) "I don't say this lightly. However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa

2002-06-06 22:20:26

by Robert Love

[permalink] [raw]
Subject: Re: [PATCH] remove suser()

On Thu, 2002-06-06 at 14:48, Pavel Machek wrote:

> > diff -urN linux-2.5.20/include/linux/compatmac.h linux/include/linux/compatmac.h
> > --- linux-2.5.20/include/linux/compatmac.h Sun Jun 2 18:44:41 2002
> > +++ linux/include/linux/compatmac.h Tue Jun 4 13:57:33 2002
> > @@ -102,8 +102,6 @@
> >
> > #define my_iounmap(x, b) (((long)x<0x100000)?0:vfree ((void*)x))
> >
> > -#define capable(x) suser()
> > -
> > #define tty_flip_buffer_push(tty) queue_task(&tty->flip.tqueue, &tq_timer)
>
> This is not right I believe. You want to keep compatibility for older
> kernels.

Someone else said this, too. I must misunderstand the point of this
file... it is used for compatibility with NEW code in OLD kernels?
Confusing that we keep it in the NEW kernel, then. I figured it was
just to keep compatibility in general, or allow OLD code to work in the
new kernels.

If the intention is to use this header with other, older, kernels... and
we really want to keep that in a new kernel... then yes - this bit
should be added back.

Robert Love

2002-06-06 22:32:45

by Ruth Ivimey-Cook

[permalink] [raw]
Subject: Re: [PATCH] remove suser()

On Thu, 6 Jun 2002, Pavel Machek wrote:

>> diff -urN linux-2.5.20/include/linux/compatmac.h linux/include/linux/compatmac.h
>> --- linux-2.5.20/include/linux/compatmac.h Sun Jun 2 18:44:41 2002
>> +++ linux/include/linux/compatmac.h Tue Jun 4 13:57:33 2002
>> @@ -102,8 +102,6 @@
>>
>> #define my_iounmap(x, b) (((long)x<0x100000)?0:vfree ((void*)x))
>>
>> -#define capable(x) suser()
>> -
>> #define tty_flip_buffer_push(tty) queue_task(&tty->flip.tqueue, &tq_timer)
>
>This is not right I believe. You want to keep compatibility for older
>kernels.
>
> Pavel

Why? The file being changed is in a particular kernel, not in, say, glibc; to
require compatibilty btw. a file in kernel X and another in kernel Y seems
stupid... but perhaps I misunderstand.

Ruth


--
Ruth Ivimey-Cook
Software engineer and technical writer.

2002-06-07 05:58:32

by Bernd Eckenfels

[permalink] [raw]
Subject: Re: [PATCH] remove suser()

In article <[email protected]> you wrote:
>>> +++ linux/include/linux/compatmac.h Tue Jun 4 13:57:33 2002
>>> -#define capable(x) suser()

> Why? The file being changed is in a particular kernel, not in, say, glibc; to
> require compatibilty btw. a file in kernel X and another in kernel Y seems
> stupid... but perhaps I misunderstand.


I must admit, I do not understand why this is needed in new kernels, too.
But the suser() call which actually should be removed is in
/usr/src/linux/include/linux/sched.h. Same is true for fsuser().

It is a good idea to remove it from the code, so everybody is forced to use
a sane api everywhere.

Greetings
Bernd