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

fs/tmpfs: fix use after free issue

Browse Source 
mmap establish a mapping address space that refer to a tmpfs file,
there are two issues:

1. if the tmpfs file is removed and a subsequent close(), tmpfs_close
will release the backend memory object, use after free errors occur
when operating the mapping memory. We add an extra reference to memory object,
memory will be released when there are no more mappings.

2. if unmap only a portion of the memory, fix the bug that adds another map.
Use realloc and shrink the mapping memory instead.

The fix pass LTP posix case mmap/10-1.c and mmap/12-1.c

Signed-off-by: fangxinyong <fangxinyong@xiaomi.com>
This commit is contained in:
fangxinyong 2023-07-19 19:27:03 +08:00 committed by Xiang Xiao
parent 43b0421b2a
commit bfeb73e850
1 changed files with 45 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
fs/tmpfs/fs_tmpfs.c
View File

@ -93,6 +93,7 @@ static int tmpfs_realloc_file(FAR struct tmpfs_file_s *tfo,
size_t newsize);
static void tmpfs_release_lockedobject(FAR struct tmpfs_object_s *to);
static void tmpfs_release_lockedfile(FAR struct tmpfs_file_s *tfo);
static int tmpfs_release_file(FAR struct tmpfs_file_s *tfo);
static int tmpfs_find_dirent(FAR struct tmpfs_directory_s *tdo,
FAR const char *name, size_t len);
static int tmpfs_remove_dirent(FAR struct tmpfs_directory_s *tdo,
@ -349,6 +350,7 @@ static void tmpfs_release_lockedfile(FAR struct tmpfs_file_s *tfo)
if (tfo->tfo_refs == 1 && (tfo->tfo_flags & TFO_FLAG_UNLINKED) != 0)
{
tmpfs_unlock_file(tfo);
nxrmutex_destroy(&tfo->tfo_lock);
kmm_free(tfo->tfo_data);
kmm_free(tfo);
@ -363,6 +365,28 @@ static void tmpfs_release_lockedfile(FAR struct tmpfs_file_s *tfo)
}
}
/****************************************************************************
* Name: tmpfs_release_file
****************************************************************************/
static int tmpfs_release_file(FAR struct tmpfs_file_s *tfo)
{
int ret;
DEBUGASSERT(tfo);
/* Get exclusive access to the file */
ret = tmpfs_lock_file(tfo);
if (ret < 0)
{
return ret;
}
tmpfs_release_lockedfile(tfo);
return OK;
}
/****************************************************************************
* Name: tmpfs_find_dirent
****************************************************************************/
@ -1411,49 +1435,15 @@ static int tmpfs_close(FAR struct file *filep)
finfo("filep: %p\n", filep);
DEBUGASSERT(filep->f_priv != NULL && filep->f_inode != NULL);
/* Recover our private data from the struct file instance */
tfo = filep->f_priv;
/* Get exclusive access to the file */
ret = tmpfs_lock_file(tfo);
if (ret < 0)
ret = tmpfs_release_file(tfo);
if (ret >= 0)
{
return ret;
filep->f_priv = NULL;
}
/* Decrement the reference count on the file */
DEBUGASSERT(tfo->tfo_refs > 0);
if (tfo->tfo_refs > 0)
{
tfo->tfo_refs--;
}
filep->f_priv = NULL;
/* If the reference count decremented to zero and the file has been
* unlinked, then free the file allocation now.
*/
if (tfo->tfo_refs == 0 && (tfo->tfo_flags & TFO_FLAG_UNLINKED) != 0)
{
/* Free the file object while we hold the lock? Weird but this
* should be safe because the object is unlinked and could not
* have any other references.
*/
nxrmutex_destroy(&tfo->tfo_lock);
kmm_free(tfo->tfo_data);
kmm_free(tfo);
return OK;
}
/* Release the lock on the file */
tmpfs_unlock_file(tfo);
return OK;
return ret;
}
/****************************************************************************
@ -1647,7 +1637,7 @@ static int tmpfs_unmap(FAR struct task_group_s *group,
FAR struct mm_map_entry_s *entry,
FAR void *start, size_t length)
{
FAR struct file *filep = entry->priv.p;
FAR struct tmpfs_file_s *tfo = entry->priv.p;
off_t offset;
int ret;
@ -1671,6 +1661,10 @@ static int tmpfs_unmap(FAR struct task_group_s *group,
/* Then remove the mapping from the list */
ret = mm_map_remove(get_group_mm(group), entry);
if (ret >= 0)
{
ret = tmpfs_release_file(tfo);
}
}
/* No.. We have been asked to "unmap' only a portion of the memory
@ -1679,8 +1673,10 @@ static int tmpfs_unmap(FAR struct task_group_s *group,
else
{
entry->length = length;
ret = tmpfs_mmap(filep, entry);
entry->length = offset;
tmpfs_lock_file(tfo);
ret = tmpfs_realloc_file(tfo, offset);
tmpfs_unlock_file(tfo);
}
return ret;
@ -1703,9 +1699,16 @@ static int tmpfs_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
map->length && map->offset + map->length <= tfo->tfo_size)
{
map->vaddr = tfo->tfo_data + map->offset;
map->priv.p = filep;
map->priv.p = tfo;
map->munmap = tmpfs_unmap;
ret = mm_map_add(get_current_mm(), map);
if (ret >= 0)
{
tmpfs_lock_file(tfo);
tfo->tfo_refs++;
tmpfs_unlock_file(tfo);
}
}
return ret;