Re: [RFC][PATCH] Prevent overriding of Symbols in the Kernel, avoiding Undefined behaviour

[Anand H. Krishnan]

Hi,


> I'm wondering, doesn't this take quite a long time? 
> Too long to hold
> a spinlock for?
> 
> Of course we need locking to prevent other module
> loads to modify the
> symbol table while we are checking this one, but to
> prevent the kernel
> from stalling everything else for a long time,
> wouldn't it be better
> to use a semaphore (we can sleep with those -
> right?) and an explicit
> schedule() call in the loop(s)?   Or am I completely
> out in outere
> space with that thought?
> 

Both at the time of loading a module and while
unloading a 
module module_mutex is acquired. In the first place we
wer
e not planning to take the spinlock at all. But saw
that r
esolve_symbol does that and hence wasn't sure whether 
the
lock should be acquired. 


> Shouldn't this printk() be using KERN_ERROR ?
> 
> 	printk(KERN_ERROR "%s: Duplicate Exported Symbol
> found in %s\n",
> 
> 
> > +			strtab + index, mod->name);
> > +	return -ENOEXEC;
> > +duplicate_gpl_sym:
> > +	spin_unlock_irq(&modlist_lock);
> > +	printk("%s: Duplicate Exported Symbol found in
> %s\n",
> > +			strtab + index, mod->name);
> > +	return -ENOEXEC;
> > +}
> 
> Why 3 different exit paths? and 2 of them are even
> identical. Why not
> something like this instead? :
> 
> {
> ...
>     if (unlikely(value) {
>         ret = -ENOEXEC;
>         goto out;
>     }
> ...
>  out:
>     spin_unlock_irq();
>     if (ret)
>         printk();
>     return ret;
> }
> 

We will send an updated patch with the modifications.

Thanks,
Anand


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/