From cc7130b836c65d3149666bfe4201d90cf6069940 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Wed, 23 Sep 2015 10:34:08 -0600 Subject: [PATCH] Correct a reference counting error in mq_open() --- ChangeLog | 3 +++ arch | 2 +- fs/driver/fs_registerblockdriver.c | 4 +++- fs/driver/fs_registerdriver.c | 4 +++- fs/inode/fs_inodereserve.c | 8 +++++--- fs/mount/fs_mount.c | 2 ++ fs/mqueue/mq_close.c | 2 +- fs/mqueue/mq_open.c | 12 +++++++++--- fs/semaphore/sem_open.c | 9 ++++++--- fs/unionfs/fs_unionfs.c | 1 + fs/vfs/fs_mkdir.c | 5 ++++- fs/vfs/fs_rename.c | 5 ++++- 12 files changed, 42 insertions(+), 15 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1102051c4b..d091e4399f 100755 --- a/ChangeLog +++ b/ChangeLog @@ -10981,4 +10981,7 @@ Carvalho de Assis (2015-09-19). * drivers/power/bq2425x.c and .h: Battery Charger: Add BQ24250 driver. From Alan Carvalho de Assis (2015-09-20). + * fs/mqueue/mq_open.c: When message queue is opened, inode_reserve() + leaves the reference count at zero. mq_open() logic must assure + that the reference count of the newly created inode is one (2015-09-23). diff --git a/arch b/arch index 4dc3ba30bd..eaeadc582a 160000 --- a/arch +++ b/arch @@ -1 +1 @@ -Subproject commit 4dc3ba30bd14059b76c578694842bda0eaaf9763 +Subproject commit eaeadc582ae79c5abe1ea8d6515585d2b0f5e34b diff --git a/fs/driver/fs_registerblockdriver.c b/fs/driver/fs_registerblockdriver.c index ea611022d3..e0032b581a 100644 --- a/fs/driver/fs_registerblockdriver.c +++ b/fs/driver/fs_registerblockdriver.c @@ -108,7 +108,9 @@ int register_blockdriver(FAR const char *path, ret = inode_reserve(path, &node); if (ret >= 0) { - /* We have it, now populate it with block driver specific information. */ + /* We have it, now populate it with block driver specific information. + * NOTE that the initial reference count on the new inode is zero. + */ INODE_SET_BLOCK(node); diff --git a/fs/driver/fs_registerdriver.c b/fs/driver/fs_registerdriver.c index db635cdfd1..5d8269c25b 100644 --- a/fs/driver/fs_registerdriver.c +++ b/fs/driver/fs_registerdriver.c @@ -103,7 +103,9 @@ int register_driver(FAR const char *path, FAR const struct file_operations *fops ret = inode_reserve(path, &node); if (ret >= 0) { - /* We have it, now populate it with driver specific information. */ + /* We have it, now populate it with driver specific information. + * NOTE that the initial reference count on the new inode is zero. + */ INODE_SET_DRIVER(node); diff --git a/fs/inode/fs_inodereserve.c b/fs/inode/fs_inodereserve.c index 104a42e0e7..faa6c01237 100644 --- a/fs/inode/fs_inodereserve.c +++ b/fs/inode/fs_inodereserve.c @@ -145,9 +145,8 @@ static void inode_insert(FAR struct inode *node, * Name: inode_reserve * * Description: - * Reserve an (initialized) inode the pseudo file system. - * - * NOTE: Caller must hold the inode semaphore + * Reserve an (initialized) inode the pseudo file system. The initial + * reference count on the new inode is zero. * * Input parameters: * path - The path to the inode to create @@ -161,6 +160,9 @@ static void inode_insert(FAR struct inode *node, * EEXIST - An inode already exists at 'path' * ENOMEM - Failed to allocate in-memory resources for the operation * + * Assumptions: + * Caller must hold the inode semaphore + * ****************************************************************************/ int inode_reserve(FAR const char *path, FAR struct inode **inode) diff --git a/fs/mount/fs_mount.c b/fs/mount/fs_mount.c index 3a5602968c..401ff747ba 100644 --- a/fs/mount/fs_mount.c +++ b/fs/mount/fs_mount.c @@ -296,6 +296,8 @@ int mount(FAR const char *source, FAR const char *target, /* Insert a dummy node -- we need to hold the inode semaphore * to do this because we will have a momentarily bad structure. + * NOTE that the new inode will be created with an initial reference + * count of zero. */ { diff --git a/fs/mqueue/mq_close.c b/fs/mqueue/mq_close.c index 00a29bbe6b..eb0d313e8f 100644 --- a/fs/mqueue/mq_close.c +++ b/fs/mqueue/mq_close.c @@ -136,7 +136,7 @@ int mq_close(mqd_t mqdes) } /**************************************************************************** - * Name: mq_close + * Name: mq_inode_release * * Description: * Release a reference count on a message queue inode. diff --git a/fs/mqueue/mq_open.c b/fs/mqueue/mq_open.c index f2b95b9b5a..f49f5a34c5 100644 --- a/fs/mqueue/mq_open.c +++ b/fs/mqueue/mq_open.c @@ -1,7 +1,7 @@ /**************************************************************************** * fs/mqueue/mq_open.c * - * Copyright (C) 2007-2009, 2011, 2014 Gregory Nutt. All rights reserved. + * Copyright (C) 2007-2009, 2011, 2014-2015 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -139,7 +139,8 @@ mqd_t mq_open(FAR const char *mq_name, int oflags, ...) sched_lock(); /* Get the inode for this mqueue. This should succeed if the message - * queue has already been created. + * queue has already been created. In this case, inode_finde() will + * have incremented the reference count on the inode. */ inode = inode_find(fullpath, &relpath); @@ -206,7 +207,9 @@ mqd_t mq_open(FAR const char *mq_name, int oflags, ...) goto errout_with_lock; } - /* Allocate memory for the new message queue. */ + /* Allocate memory for the new message queue. The new inode will + * be created with a reference count of zero. + */ msgq = (FAR struct mqueue_inode_s*)mq_msgqalloc(mode, attr); if (!msgq) @@ -230,6 +233,9 @@ mqd_t mq_open(FAR const char *mq_name, int oflags, ...) inode->u.i_mqueue = msgq; msgq->inode = inode; + /* Set the initial reference count on this inode to one */ + + inode->i_crefs = 1; } sched_unlock(); diff --git a/fs/semaphore/sem_open.c b/fs/semaphore/sem_open.c index 292eddc82c..4d422c8503 100644 --- a/fs/semaphore/sem_open.c +++ b/fs/semaphore/sem_open.c @@ -148,8 +148,9 @@ FAR sem_t *sem_open (FAR const char *name, int oflags, ...) snprintf(fullpath, MAX_SEMPATH, CONFIG_FS_NAMED_SEMPATH "/%s", name); - /* Get the inode for this semaphore. This should succeed if the semaphore - * has already been created. + /* Get the inode for this semaphore. This should succeed if the + * semaphore has already been created. In this case, inode_finde() + * will have incremented the reference count on the inode. */ inode = inode_find(fullpath, &relpath); @@ -214,7 +215,9 @@ FAR sem_t *sem_open (FAR const char *name, int oflags, ...) goto errout_with_lock; } - /* Create an inode in the pseudo-filesystem at this path */ + /* Create an inode in the pseudo-filesystem at this path. The new + * inode will be created with a reference count of zero. + */ inode_semtake(); ret = inode_reserve(fullpath, &inode); diff --git a/fs/unionfs/fs_unionfs.c b/fs/unionfs/fs_unionfs.c index 9a3c8560b5..994c30b663 100644 --- a/fs/unionfs/fs_unionfs.c +++ b/fs/unionfs/fs_unionfs.c @@ -2568,6 +2568,7 @@ int unionfs_mount(FAR const char *fspath1, FAR const char *prefix1, /* Insert a dummy node -- we need to hold the inode semaphore * to do this because we will have a momentarily bad structure. + * NOTE that the inode will be created with a refernce count of zero. */ inode_semtake(); diff --git a/fs/vfs/fs_mkdir.c b/fs/vfs/fs_mkdir.c index 6d037b37c1..832c5f816a 100644 --- a/fs/vfs/fs_mkdir.c +++ b/fs/vfs/fs_mkdir.c @@ -154,7 +154,10 @@ int mkdir(const char *pathname, mode_t mode) else { - /* Create an inode in the pseudo-filesystem at this path */ + /* Create an inode in the pseudo-filesystem at this path. + * NOTE that the new inode will be created with a reference + * count of zero. + */ inode_semtake(); ret = inode_reserve(pathname, &inode); diff --git a/fs/vfs/fs_rename.c b/fs/vfs/fs_rename.c index cb9173dab9..76f11b0abf 100644 --- a/fs/vfs/fs_rename.c +++ b/fs/vfs/fs_rename.c @@ -175,7 +175,10 @@ int rename(FAR const char *oldpath, FAR const char *newpath) #endif #ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS { - /* Create a new, empty inode at the destination location */ + /* Create a new, empty inode at the destination location. + * NOTE that the new inode will be created with a reference count + * of zero. + */ inode_semtake(); ret = inode_reserve(newpath, &newinode);