Bug fix: Hold spinlock across find|remove /sys/firmware/memmap/X operation.

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>
Cc: Wen Congyang <wency@cn.fujitsu.com>
Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Cc: Tang Chen <tangchen@cn.fujitsu.com>
Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/drivers/firmware/memmap.c b/drivers/firmware/memmap.c
index 4211da59cc10c1d99a79956b3ce268ac9693c1f3..940c4e946d0b8142f6bae29581632cea6d026205 100644 (file)
--- 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);