From acd3b47def53db18b3cf425e0931b781a14221cd Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Wed, 2 Dec 2015 05:20:02 -0600 Subject: [PATCH] Fix problem in last change to the procfs: Forgot to save the reallocated table pointer! Also added warnings: There are some concurrency issues in the current implementation if you try to modify the procfs data structures will the procfs is mounted and possibly busy. --- configs | 2 +- fs/procfs/fs_procfs.c | 24 +++++++++++++++++++++++- include/nuttx/fs/procfs.h | 4 ++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/configs b/configs index 78bded3f7c..7e02a47bfd 160000 --- a/configs +++ b/configs @@ -1 +1 @@ -Subproject commit 78bded3f7c49fd74338741d797066e7592c633ae +Subproject commit 7e02a47bfd113b95136dafe75ce2a6551534b7e4 diff --git a/fs/procfs/fs_procfs.c b/fs/procfs/fs_procfs.c index 753e48ce8b..c9032560e6 100644 --- a/fs/procfs/fs_procfs.c +++ b/fs/procfs/fs_procfs.c @@ -1038,6 +1038,10 @@ int procfs_initialize(void) * Description: * Add a new entry to the procfs file system. * + * NOTE: This function should be called *prior* to mounting the procfs + * file system to prevent concurrency problems with the modification of + * the procfs data set while it is in use. + * * Input Parameters: * entry - Describes the entry to be registered. * @@ -1058,7 +1062,17 @@ int procfs_register(FAR const struct procfs_entry_s *entry) procfs_initialize(); - /* realloc the procfs entries */ + /* realloc the table of procfs entries. + * + * REVISIT: This reallocation may free memory previously used for the + * procfs entry table. If that table were actively in use, then that + * could cause procfs logic to use a stale memory pointer! We avoid that + * problem by requiring that the procfs file be unmounted when the new + * entry is added. That requirment, however, is not enforced explicitly. + * + * Locking the scheduler as done below is insufficient. As would be just + * marking the entries as volatile. + */ newcount = g_procfs_entrycount + 1; newsize = newcount * sizeof(struct procfs_entry_s); @@ -1067,11 +1081,19 @@ int procfs_register(FAR const struct procfs_entry_s *entry) newtable = (FAR struct procfs_entry_s *)kmm_realloc(g_procfs_entries, newsize); if (newtable == NULL) { + /* Reallocation failed! */ + ret = -ENOMEM; } else { + /* Copy the new entry at the end of the reallocated table */ + memcpy(&newtable[g_procfs_entrycount], entry, sizeof(struct procfs_entry_s)); + + /* Instantiate the reallocated table */ + + g_procfs_entries = newtable; g_procfs_entrycount = newcount; ret = OK; } diff --git a/include/nuttx/fs/procfs.h b/include/nuttx/fs/procfs.h index c48be86d2f..f11046735e 100644 --- a/include/nuttx/fs/procfs.h +++ b/include/nuttx/fs/procfs.h @@ -186,6 +186,10 @@ size_t procfs_memcpy(FAR const char *src, size_t srclen, * Description: * Add a new entry to the procfs file system. * + * NOTE: This function should be called *prior* to mounting the procfs + * file system to prevent concurrency problems with the modification of + * the procfs data set while it is in use. + * * Input Parameters: * entry - Describes the entry to be registered. *