cvs commit: src/sys/i386/i386 sys_machdep.c

[John Baldwin]

On Friday 12 September 2008 11:36:19 am Kostik Belousov wrote:
> On Fri, Sep 12, 2008 at 10:22:35AM -0400, John Baldwin wrote:
> > On Friday 12 September 2008 05:51:11 am Konstantin Belousov wrote:
> > > kib         2008-09-12 09:51:11 UTC
> > > 
> > >   FreeBSD src repository
> > > 
> > >   Modified files:
> > >     sys/i386/i386        sys_machdep.c 
> > >   Log:
> > >   SVN rev 182960 on 2008-09-12 09:51:11Z by kib
> > >   
> > >   The user_ldt_alloc() function shall return with dt_lock locked.
> > >   The user_ldt_free() function shall return with dt_lock unlocked.
> > >   Error handling code in both functions do not handle this, fix it by
> > >   doing necessary lock/unlock.
> > >   
> > >   While there, fix minor style nits.
> > 
> > Hmm, I had actually thought it was intentional for user_ldt_alloc() to 
only 
> > return with the lock held on success and depend on a later call to another 
> > method to drop the lock in the success case (so the locking isn't visible 
to 
> > consumers of the API in theory).  For example, i386_ldt_grow() depended on 
> > this feature and is now broken (it leaks a lock on failure).  I missed 
this 
> > when looking at this yesterday.
> 
> I probably miss something there.
> 
> On failure of user_ldt_alloc(), i386_ldt_grow() does return (ENOMEM),
> without changing lock state for dt_lock.
> 
> There are three call locations for the i386_ldt_grow(), all of
> them in i386_set_ldt(). On failure, each call location does
> mtx_unlock_spin(&dt_lock) immediately after call. So I assumed that
> protocol for i386_ldt_grow() is to always return with dt_lock locked.
> 
> Two other callers of the user_ldt_alloc() in cpu_fork() do panic()
> immediately after the failed call to user_ldt_alloc().
> 
> Could you, please, point me to exact place where the lock would leak ?

Gah, sorry, I had missed the callers of i386_ldt_grow().  I had recalled it 
being purposeful at one point that user_ldt_alloc() had this behavior.

-- 
John Baldwin