BusyBox Bug and Patch Tracking
BusyBox
Main | My View | View Issues | Change Log | Docs
  

Viewing Issue Simple Details [ Jump to Notes ] [ View Advanced ] [ Issue History ] [ Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0000716 [uClibc] Other minor always 02-10-06 10:49 03-21-06 16:31
Reporter rholzmann View Status public  
Assigned To uClibc
Priority normal Resolution fixed  
Status closed   Product Version 0.9.28
Summary 0000716: 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.
Additional Information
Attached Files  utmp-thread.patch [^] (1,311 bytes) 02-10-06 10:49
 utent-patch2.patch [^] (2,974 bytes) 02-14-06 07:21

- Relationships

- Notes
(0001083)
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
 
(0001095)
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.
 
(0001103)
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 :/
 
(0001192)
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

Copyright © 2000 - 2006 Mantis Group
Powered by Mantis Bugtracker