Messages in this thread | | | Date | Tue, 13 Dec 2005 00:23:50 -0800 (PST) | From | "Anand H. Krishnan" <> | Subject | Re: [RFC][PATCH] Prevent overriding of Symbols in the Kernel, avoiding Undefined behaviour |
| |
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |