Re: Patch to lock down modules

Apparently, On Thu, Mar 14, 2002 at 01:25:24PM -0500,
	Andrew R. Reiter said words to the effect of;

> On Thu, 14 Mar 2002, Jake Burkholder wrote:
> 
> :
> :Couple problems:
> :
> :I think that you missed a MOD_SLOCK before the
> :	newmod = module_lookupbyname(data->name);
> :in module_register.
> 
> Actually, iirc, module_register only got called from one spot, and in that
> spot, keeping SLOCK held seemed better than dropping it and reaquiring a
> SLOCK in module_register.  If we'd like to keep this more consisten with
> some of the other funcitons, then I can change this no problem.

Hum.  Well, I don't like its side effect of releasing the lock.  I think
that the place where its called in kern_linker.c should not do a lookup
by name itself, but leave it to module_register, it will return an error
anyway.  That way it doesn't need to acquire the lock before hand, and
can leave it up to module_register.

> 
> :
> :Also in that function the MOD_XLOCK isn't needed until just before the
> :TAILQ_INSERT_TAIL.  The increment of nextid needs something, I would
> :probably move it to just before the TAILQ_INSERT, under the XLOCK.  An
> :atomic_add_and_return_old_value would be useful here.  Also, I think
> :that you should do another module_lookupbyname after acquiring the
> :XLOCK and before the TAILQ_INSERT, because conceivably another module
> :with the same name could show up when you release the SLOCK.
> 
> Seems reasonable...
> 
> :Can you explain the wierd logic that was added to linker_file_unload?
> 
> Ugh, yes, this is kinda ugly.  Essentially this is a result of hacking
> this back and forth between where we should be holding SLOCK over.  This
> is exactly what I wanted to discuss this b/c prior to making this change,
> I had a much more simple strategy here (but was dropped due to changes to
> a better strategy for kern_module).  We discussed before about possibly
> having two sets of lookup functions -- internal and external -- perhaps
> this is a better solution?  

Hmm, I'll have to look at it more closely.

> 
> :Thanks,
> :Jake
> :
> 
> --
> Andrew R. Reiter
> [e-mail]
> [e-mail]

To Unsubscribe: send mail to [e-mail]
with "unsubscribe freebsd-smp" in the body of the message