mirror of
https://github.com/apache/nuttx.git
synced 2026-10-06 11:03:16 +08:00
fs/aio: fix aioc use-after-free and aio_cancel() issues
aioc_decant() frees the AIO container and detaches the aiocbp. The I/O workers (aio_read_worker, aio_write_worker, aio_fsync_worker) called it before signaling completion, so aio_signal() and any code touching the container afterwards ran on freed memory. Additionally, if the caller closed the file early the detached container could be reused with a stale file reference. Move aioc_decant() to after aio_signal() and use aioc->aioc_aiocbp directly in the workers. aio_cancel() also had two problems: with no aiocbp it looped over g_aio_pending with a do/while that skipped the list re-entry check, so a failed work_cancel() on an already running I/O caused an endless loop; and an invalid fildes only checked 'fildes < 0' instead of validating the descriptor, so a closed fd was not reported as EBADF. Use a for-loop that always advances and validate the descriptor with file_get()/file_put(). Co-developed-by: wushenhui <wushenhui@xiaomi.com> Signed-off-by: wushenhui <wushenhui@xiaomi.com> Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
This commit is contained in:
committed by
Alan C. Assis
parent
2f4d017bb6
commit
b7d6c8ff41
+20
-15
@@ -32,6 +32,7 @@
|
||||
#include <errno.h>
|
||||
|
||||
#include <nuttx/wqueue.h>
|
||||
#include <nuttx/fs/fs.h>
|
||||
|
||||
#include "aio/aio.h"
|
||||
|
||||
@@ -83,18 +84,23 @@
|
||||
|
||||
int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
|
||||
{
|
||||
if (fildes < 0)
|
||||
{
|
||||
set_errno(EBADF);
|
||||
return ERROR;
|
||||
}
|
||||
|
||||
FAR struct aio_container_s *aioc;
|
||||
FAR struct aio_container_s *next;
|
||||
FAR struct file *filep;
|
||||
|
||||
pid_t pid;
|
||||
int status;
|
||||
int ret;
|
||||
|
||||
ret = file_get(fildes, &filep);
|
||||
if (ret < 0)
|
||||
{
|
||||
set_errno(-ret);
|
||||
return ERROR;
|
||||
}
|
||||
|
||||
file_put(filep);
|
||||
|
||||
/* Check if a non-NULL aiocbp was provided */
|
||||
|
||||
/* Lock the scheduler so that no I/O events can complete on the worker
|
||||
@@ -165,14 +171,16 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
|
||||
{
|
||||
/* No aiocbp.. cancel all outstanding I/O for the fildes */
|
||||
|
||||
next = (FAR struct aio_container_s *)g_aio_pending.head;
|
||||
do
|
||||
for (aioc = (FAR struct aio_container_s *)g_aio_pending.head;
|
||||
aioc;
|
||||
aioc = next)
|
||||
{
|
||||
/* Find the next container with this AIO control block */
|
||||
next = (FAR struct aio_container_s *)aioc->aioc_link.flink;
|
||||
|
||||
for (aioc = next;
|
||||
aioc && aioc->aioc_aiocbp->aio_fildes != fildes;
|
||||
aioc = (FAR struct aio_container_s *)aioc->aioc_link.flink);
|
||||
if (aioc->aioc_aiocbp->aio_fildes != fildes)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
/* Did we find the container? We should; the aio_result says
|
||||
* that the transfer is pending. If not we return AIO_ALLDONE.
|
||||
@@ -195,8 +203,6 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
|
||||
* transfers
|
||||
*/
|
||||
|
||||
next =
|
||||
(FAR struct aio_container_s *)aioc->aioc_link.flink;
|
||||
pid = aioc->aioc_pid;
|
||||
aiocbp = aioc_decant(aioc);
|
||||
DEBUGASSERT(aiocbp);
|
||||
@@ -217,7 +223,6 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
|
||||
}
|
||||
}
|
||||
}
|
||||
while (aioc);
|
||||
}
|
||||
|
||||
aio_unlock();
|
||||
|
||||
+2
-1
@@ -79,7 +79,7 @@ static void aio_fsync_worker(FAR void *arg)
|
||||
#ifdef CONFIG_PRIORITY_INHERITANCE
|
||||
prio = aioc->aioc_prio;
|
||||
#endif
|
||||
aiocbp = aioc_decant(aioc);
|
||||
aiocbp = aioc->aioc_aiocbp;
|
||||
|
||||
/* Perform the fsync using aioc_filep */
|
||||
|
||||
@@ -97,6 +97,7 @@ static void aio_fsync_worker(FAR void *arg)
|
||||
/* Signal the client */
|
||||
|
||||
aio_signal(pid, aiocbp);
|
||||
aioc_decant(aioc);
|
||||
|
||||
#ifdef CONFIG_PRIORITY_INHERITANCE
|
||||
/* Restore the low priority worker thread default priority */
|
||||
|
||||
+2
-1
@@ -79,7 +79,7 @@ static void aio_read_worker(FAR void *arg)
|
||||
#ifdef CONFIG_PRIORITY_INHERITANCE
|
||||
prio = aioc->aioc_prio;
|
||||
#endif
|
||||
aiocbp = aioc_decant(aioc);
|
||||
aiocbp = aioc->aioc_aiocbp;
|
||||
|
||||
/* Perform the file read using:
|
||||
*
|
||||
@@ -106,6 +106,7 @@ static void aio_read_worker(FAR void *arg)
|
||||
/* Signal the client */
|
||||
|
||||
aio_signal(pid, aiocbp);
|
||||
aioc_decant(aioc);
|
||||
|
||||
#ifdef CONFIG_PRIORITY_INHERITANCE
|
||||
/* Restore the low priority worker thread default priority */
|
||||
|
||||
+2
-1
@@ -82,7 +82,7 @@ static void aio_write_worker(FAR void *arg)
|
||||
#ifdef CONFIG_PRIORITY_INHERITANCE
|
||||
prio = aioc->aioc_prio;
|
||||
#endif
|
||||
aiocbp = aioc_decant(aioc);
|
||||
aiocbp = aioc->aioc_aiocbp;
|
||||
|
||||
/* Call fcntl(F_GETFL) to get the file open mode. */
|
||||
|
||||
@@ -134,6 +134,7 @@ errout:
|
||||
/* Signal the client */
|
||||
|
||||
aio_signal(pid, aiocbp);
|
||||
aioc_decant(aioc);
|
||||
|
||||
#ifdef CONFIG_PRIORITY_INHERITANCE
|
||||
/* Restore the low priority worker thread default priority */
|
||||
|
||||
Reference in New Issue
Block a user