[uClibc 0000716]: libc/misc/utmp/utent.c deadlocks when used with libpthread

[bugs at busybox.net]

The following issue has been CLOSED 
====================================================================== 
http://busybox.net/bugs/view.php?id=716 
====================================================================== 
Reported By:                rholzmann
Assigned To:                uClibc
====================================================================== 
Project:                    uClibc
Issue ID:                   716
Category:                   Other
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     closed
Resolution:                 fixed
Fixed in Version:           
====================================================================== 
Date Submitted:             02-10-2006 10:49 PST
Last Modified:              03-21-2006 16:31 PST
====================================================================== 
Summary:                    libc/misc/utmp/utent.c deadlocks when used with
libpthread
Description: 
utent.c has a few problems with mutex locking when used in a binary that
was linked with pthreads.  The are a few deadlock conditions where
functions may be called which lock the utmplock but never release it and
where a function that hold the lock calls other functions which try to
relock the same lock.  Here is one example in the function __getutent:

static struct utmp *__getutent(int utmp_fd)
{
    if (utmp_fd == -1) {
        setutent();
    }
    if (utmp_fd == -1) {
        return NULL;
    }

    LOCK;
    if (read(utmp_fd, (char *) &static_utmp, sizeof(struct utmp)) != 
                sizeof(struct utmp))
    {
        return NULL;
    }

    UNLOCK;
    return &static_utmp;
}

Notice the error condition does not unlock the semaphore.  The problem is
not visible when pthreads isn't used since the lock/unlock functions are
NOOP functions.

I have attached a patch file that tries to correct these problems.  I
think the patch fixes all of the possible deadlock conditions.  Let me
know what you think.

====================================================================== 

---------------------------------------------------------------------- 
 vapier - 02-10-06 21:37  
---------------------------------------------------------------------- 
thanks, the patch looks good ... ive also shrunk the code in a few other
places as well and added to svn 

---------------------------------------------------------------------- 
 rholzmann - 02-14-06 07:20  
---------------------------------------------------------------------- 
I think there is still a problem if you call pututline.

1. pututline LOCKs
2. pututline calls getutid
3. getutid calls __getutent
4. a. __getutent possibly calls setutent
   b. setutent LOCKs (deadlock)
 -OR-
5. __getutent LOCKs (deadlock)

I think a good solution would be to change __getutent to not lock and have
calling functions obtain the lock.  I have attached my proposed change. 
The basic design behind the change is to make the utent API layer of
functions handle obtaining/releasing the lock which relieves the
__api_functions from worrying about the locking.  I think this makes the
code a bit easier to follow as well. 

---------------------------------------------------------------------- 
 vapier - 02-15-06 19:54  
---------------------------------------------------------------------- 
your patch does look ok, the only thing i'm unhappy with is how much bigger
the object gets ... but i dont think there is a way around that :/ 

---------------------------------------------------------------------- 
 vapier - 03-21-06 16:31  
---------------------------------------------------------------------- 
ok, ive committed the remaining issues 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
02-10-06 10:49  rholzmann      New Issue                                    
02-10-06 10:49  rholzmann      Status                   new => assigned     
02-10-06 10:49  rholzmann      Assigned To               => uClibc          
02-10-06 10:49  rholzmann      File Added: utmp-thread.patch                    
02-10-06 21:37  vapier         Note Added: 0001083                          
02-14-06 07:20  rholzmann      Note Added: 0001095                          
02-14-06 07:21  rholzmann      File Added: utent-patch2.patch                   

02-15-06 19:54  vapier         Note Added: 0001103                          
03-21-06 16:31  vapier         Note Added: 0001192                          
03-21-06 16:31  vapier         Status                   assigned => closed  
03-21-06 16:31  vapier         Resolution               open => fixed       
======================================================================