Andrew,
I'm trying to remove the race in sys_access code.
AFAICS, fsuid is checked in "permission" level so I pushed real fsuid
capture forward.At that state, I can task_lock (which was impossible
before user_walk).Could you tell me if I can improve this one ?
Regards,
Fabian
* Fabian Frederick ([email protected]) wrote:
> Andrew,
>
> I'm trying to remove the race in sys_access code.
> AFAICS, fsuid is checked in "permission" level so I pushed real fsuid
> capture forward.At that state, I can task_lock (which was impossible
> before user_walk).Could you tell me if I can improve this one ?
This changes the semantics of the directory checks implicit
during the pathname resolution.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
On Tue, 2004-04-13 at 20:50, Chris Wright wrote:
> * Fabian Frederick ([email protected]) wrote:
> > Andrew,
> >
> > I'm trying to remove the race in sys_access code.
> > AFAICS, fsuid is checked in "permission" level so I pushed real
fsuid
> > capture forward.At that state, I can task_lock (which was impossible
> > before user_walk).Could you tell me if I can improve this one ?
>
> This changes the semantics of the directory checks implicit
> during the pathname resolution.
Well, the only major function behind user_walk is path_lookup.
This one has some calls with the nameidata.Other process seems
current->fs->xxx relevant read-only.Maybe you mean the
read_lock(¤t->fs->lock) which could bring a deadlock as we
task->lock before ?
If user_walk had to run in ruid, why would we have permission() then ?
Regards,
Fabian
> thanks,
> -chris
* Fabian Frederick ([email protected]) wrote:
> Well, the only major function behind user_walk is path_lookup.
> This one has some calls with the nameidata.Other process seems
> current->fs->xxx relevant read-only.Maybe you mean the
> read_lock(¤t->fs->lock) which could bring a deadlock as we
> task->lock before ?
No, point is simply that there's implicit permission check in
__user_walk().
> If user_walk had to run in ruid, why would we have permission() then ?
It's how the standards require the call to be implemented. The
access(2) check verifies access to the pathname using the ruid not euid.
Part of valid access includes search access on the directory elements of
the full pathname. Those tests are done during __user_walk.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
Fabian Frederick <[email protected]> wrote:
>
>
> I'm trying to remove the race in sys_access code.
> AFAICS, fsuid is checked in "permission" level so I pushed real fsuid
> capture forward.
Do races in access() actually matter? I mean, some other process could
change things a nanosecond after access() has completed and the value which
the access() caller received is wrong anyway.
Or is there some deeper problem which you are addressing here?
* Andrew Morton ([email protected]) wrote:
> Do races in access() actually matter? I mean, some other process could
> change things a nanosecond after access() has completed and the value which
> the access() caller received is wrong anyway.
>
> Or is there some deeper problem which you are addressing here?
There is a race where, the saved off capabilities could blow away recently
updated capabilites when they are restored. But, it's only raceable
against tasks that have SETPCAP and are setting another task's caps.
Otherwise it's serialised by the fact that we're dealing with a single
task that can only be in one syscall at a time. Fixing it would require
something like passing creds into the permission function, instead of
them being deduced from current, a rather invasive change.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
Andrew Morton wrote:
> Do races in access() actually matter? I mean, some other process could
> change things a nanosecond after access() has completed and the value which
> the access() caller received is wrong anyway.
What about the effect access() has on other callers? It temporarily
changes current->fsuid, so fs operations in other completely
independent threads are affected.
-- Jamie
On Wed, Apr 14, 2004 at 08:11:02AM +0100, Jamie Lokier wrote:
> Andrew Morton wrote:
> > Do races in access() actually matter? I mean, some other process could
> > change things a nanosecond after access() has completed and the value which
> > the access() caller received is wrong anyway.
>
> What about the effect access() has on other callers? It temporarily
> changes current->fsuid, so fs operations in other completely
> independent threads are affected.
Huh? What does changing a field of tast_struct have to do with behaviour
of some other threads?
[email protected] wrote:
> Huh? What does changing a field of tast_struct have to do with behaviour
> of some other threads?
It has a lot to do with a brain malfunction at my end, and nothing to
do with anything else. Enjoy the embarrassment,
-- Jamie
Hi,
It seems this thread has lost its initial goal.We can't do uid changes without lock as reported in current open.c doc.Maybe there's some expert here able to tell me exactly what's wrong in this ?
http://fabian.unixtech.be/kernel/open1.diff
And most of all patch this patch as well ;)
Regards,
Fabian
___________________________________