sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Inode semaphore must be entrant or a deadlock can occur in certain scenarios

Browse Source
This commit is contained in:
Gregory Nutt 2013-11-28 13:12:14 -06:00
parent 1ea447867a
commit 2aa0809e8b
2 changed files with 82 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
ChangeLog
View File

@ -6123,3 +6123,12 @@
src/stm32f429i-disco-internal.h, up_nsh.c, and up_spi.h: Add a
configuration and board support for an external SST25 FLASH. From Ken
Pettit (2013-11-28).
* fs/fs_inode.c: The inode semaphore must be re-entrant. Here is the
re-entering path that I found: (1) USB host connects to FLASH drive
and creates /dev/sda, (2) /dev/sda is mounted, (3) FLASH drive is
removed but /dev/sda is not destroyed because there is still a\
reference on the device because of the mount, (4) umount() is called,
taking the inode semaphore, now the driver tries to destroy the block
driver by calling unregister_blockdriver(). But (5)
unregister_blockdriver() also takes the inode semaphore causing a
deadlock if the inode semaphore is not re-entry. (2013-11-28).

85
fs/fs_inode.c
View File

@ -52,11 +52,31 @@
* Pre-processor Definitions
****************************************************************************/
#define NO_HOLDER (pid_t)-1;
/****************************************************************************
* Private Types
****************************************************************************/
/* Implements a re-entrant mutex for inode access. This must be re-entrant
* because there can be cycles. For example, it may be necessary to destroy
* a block driver inode on umount() after a removable block device has been
* removed. In that case umount() hold the inode semaphore, but the block
* driver may callback to unregister_blockdriver() after the un-mount,
* requiring the seamphore again.
*/
struct inode_sem_s
{
sem_t sem; /* The semaphore */
pid_t holder; /* The current holder of the semaphore */
int16_t count; /* Number of counts held */
};
/****************************************************************************
* Private Variables
****************************************************************************/
static sem_t tree_sem;
static struct inode_sem_s g_inode_sem;
/****************************************************************************
* Public Variables
@ -164,8 +184,10 @@ void fs_initialize(void)
* a-time access to the inode tree).
*/
(void)sem_init(&tree_sem, 0, 1);
(void)sem_init(&g_inode_sem.sem, 0, 1);
g_inode_sem.holder = NO_HOLDER;
g_inode_sem.count = 0;
/* Initialize files array (if it is used) */
#ifdef CONFIG_HAVE_WEAKFUNCTIONS
@ -180,21 +202,42 @@ void fs_initialize(void)
* Name: inode_semtake
*
* Description:
* Get exclusive access to the in-memory inode tree (tree_sem).
* Get exclusive access to the in-memory inode tree (g_inode_sem).
*
****************************************************************************/
void inode_semtake(void)
{
pid_t me;
/* Do we already hold the semaphore? */
me = getpid();
if (me == g_inode_sem.holder)
{
/* Yes... just increment the count */
g_inode_sem.count++;
DEBUGASSERT(g_inode_sem.count > 0);
}
/* Take the semaphore (perhaps waiting) */
while (sem_wait(&tree_sem) != 0)
else
{
/* The only case that an error should occr here is if
* the wait was awakened by a signal.
*/
while (sem_wait(&g_inode_sem.sem) != 0)
{
/* The only case that an error should occr here is if
* the wait was awakened by a signal.
*/
ASSERT(get_errno() == EINTR);
ASSERT(get_errno() == EINTR);
}
/* No we hold the semaphore */
g_inode_sem.holder = me;
g_inode_sem.count = 1;
}
}
@ -202,13 +245,31 @@ void inode_semtake(void)
* Name: inode_semgive
*
* Description:
* Relinquish exclusive access to the in-memory inode tree (tree_sem).
* Relinquish exclusive access to the in-memory inode tree (g_inode_sem).
*
****************************************************************************/
void inode_semgive(void)
{
sem_post(&tree_sem);
DEBUGASSERT(g_inode_sem.holder == getpid());
/* Is this our last count on the semaphore? */
if (g_inode_sem.count > 1)
{
/* No.. just decrement the count */
g_inode_sem.count--;
}
/* Yes.. then we can really release the semaphore */
else
{
g_inode_sem.holder = NO_HOLDER;
g_inode_sem.count = 0;
sem_post(&g_inode_sem.sem);
}
}
/****************************************************************************
@ -219,7 +280,7 @@ void inode_semgive(void)
* and references to its companion nodes.
*
* Assumptions:
* The caller holds the tree_sem
* The caller holds the g_inode_sem semaphore
*
****************************************************************************/