some possible bugs around (race conditions etc.)

some possible bugs around (race conditions etc.)

[Martin Mačok]

Hi,
we (Kamil Toman <ktoman@email.cz> and me) were studying linux source
and trying to make some "audit". We went over 2.4.7 source and in the
time of this writing I'm looking at 2.4.9-ac10 to compare if it was
changed. This is a list of possible things we have found:

[ definitely - we're kernel newbies so take us easy ;-) ]

lines according to 2.4.9-ac10:

kernel/capability.c:
59-63, 91-93, 203-206: SMP race, possible fix: rwlock

kernel/exit.c:
485: sys_exit doesn't return anything (nor long type)
        why it isn't void ?
442-447: is this signal handling correct?
501: task INTERRUPTIBLE - possible ineffectivity, couldn't this task
        be woken up too often (early)?

kernel/fork.c:
586: isn't memcpy() more effective?

kernel/acct.c:
SMP race ?:
----------------------------------------------------
CPU1                            CPU2

sys_acct(file)
{
    ....
    if (old_acct)

                                sys_acct(NULL)
                                sys_acct(nextfile)
                                {

                                    ....
        do_acct_process() -- BUG!
        filp_close() -- BUG!
----------------------------------------------------

kernel/sys.c:
1217: mixed signed/unsigned - doesn't it return EINVAL even when it
        shouldn't?
1042: what if strlen < len? can we get rid of chars after null?
428: why wmb() ?

kernel/sched.c:
1303-1309: isn't there a same race cond. as in kmod.c:65 ?
1323: is this needed on UP?
603:  is this correct on SMP? shouldn't there be some penalty
        accounted for being "randomly" woken/run?

kernel/kmod.c
211: shouldn't module_name be tested a bit?

Comments are welcomed.

Have a nice day

-- 
   Martin Mačok
  underground.cz
    openbsd.cz

Re: some possible bugs around (race conditions etc.)

[Alan Cox]

> kernel/capability.c:
> 59-63, 91-93, 203-206: SMP race, possible fix: rwlock

Looks ok to me 

> kernel/exit.c:
> 485: sys_exit doesn't return anything (nor long type)
>         why it isn't void ?

Syscall return - its to keep syscall wrappers happy no more

> kernel/fork.c:
> 586: isn't memcpy() more effective?

gcc will do that magic itself

>     if (old_acct)
> 

Nope. old_acct is saved carefulyl so it seems ok

> kernel/sys.c:
> 1217: mixed signed/unsigned - doesn't it return EINVAL even when it
>         shouldn't?

It returns ok when it shouldnt - fixed

> 428: why wmb() ?

So the other CPUs cant see the priviledge change before dumpable clear

> 1323: is this needed on UP?

Not really

> 603:  is this correct on SMP? shouldn't there be some penalty
>         accounted for being "randomly" woken/run?

No

> kernel/kmod.c
> 211: shouldn't module_name be tested a bit?

modprobe checks