lkml.org 
[lkml]   [2013]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[BUG Fix Patch 1/6] Bug fix: Hold spinlock across find|remove /sys/firmware/memmap/X operation.
Date
It is unsafe to return an entry pointer and release the map_entries_lock. So we should
not hold the map_entries_lock separately in firmware_map_find_entry() and
firmware_map_remove_entry(). Hold the map_entries_lock across find and remove
/sys/firmware/memmap/X operation.

And also, users of these two functions need to be careful to hold the lock when using
these two functions.

The suggestion is from Andrew Morton <akpm@linux-foundation.org>

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
drivers/firmware/memmap.c | 25 +++++++++++++++++--------
1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/memmap.c b/drivers/firmware/memmap.c
index 4211da5..940c4e9 100644
--- a/drivers/firmware/memmap.c
+++ b/drivers/firmware/memmap.c
@@ -150,12 +150,12 @@ static int firmware_map_add_entry(u64 start, u64 end,
* firmware_map_remove_entry() - Does the real work to remove a firmware
* memmap entry.
* @entry: removed entry.
+ *
+ * The caller must hold map_entries_lock, and release it properly.
**/
static inline void firmware_map_remove_entry(struct firmware_map_entry *entry)
{
- spin_lock(&map_entries_lock);
list_del(&entry->list);
- spin_unlock(&map_entries_lock);
}

/*
@@ -188,23 +188,28 @@ static inline void remove_sysfs_fw_map_entry(struct firmware_map_entry *entry)
}

/*
- * Search memmap entry
+ * firmware_map_find_entry: Search memmap entry.
+ * @start: Start of the memory range.
+ * @end: End of the memory range (exclusive).
+ * @type: Type of the memory range.
+ *
+ * This function is to find the memmap entey of a given memory range.
+ * The caller must hold map_entries_lock, and must not release the lock
+ * until the processing of the returned entry has completed.
+ *
+ * Return pointer to the entry to be found on success, or NULL on failure.
*/
-
static struct firmware_map_entry * __meminit
firmware_map_find_entry(u64 start, u64 end, const char *type)
{
struct firmware_map_entry *entry;

- spin_lock(&map_entries_lock);
list_for_each_entry(entry, &map_entries, list)
if ((entry->start == start) && (entry->end == end) &&
(!strcmp(entry->type, type))) {
- spin_unlock(&map_entries_lock);
return entry;
}

- spin_unlock(&map_entries_lock);
return NULL;
}

@@ -274,11 +279,15 @@ int __meminit firmware_map_remove(u64 start, u64 end, const char *type)
{
struct firmware_map_entry *entry;

+ spin_lock(&map_entries_lock);
entry = firmware_map_find_entry(start, end - 1, type);
- if (!entry)
+ if (!entry) {
+ spin_unlock(&map_entries_lock);
return -EINVAL;
+ }

firmware_map_remove_entry(entry);
+ spin_unlock(&map_entries_lock);

/* remove the memmap entry */
remove_sysfs_fw_map_entry(entry);
--
1.7.1


\
 
 \ /
  Last update: 2013-01-15 12:42    [W:1.623 / U:0.744 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site