Hi Andrew, On 01/10/2013 07:19 AM, Andrew Morton wrote:
... + entry = firmware_map_find_entry(start, end - 1, type); + if (!entry) + return -EINVAL; + + firmware_map_remove_entry(entry); ...The above code looks racy. After firmware_map_find_entry() does the spin_unlock() there is nothing to prevent a concurrent firmware_map_remove_entry() from removing the entry, so the kernel ends up calling firmware_map_remove_entry() twice against the same entry. An easy fix for this is to hold the spinlock across the entire lookup/remove operation. This problem is inherent to firmware_map_find_entry() as you have implemented it, so this function simply should not exist in the current form - no caller can use it without being buggy! A simple fix for this is to remove the spin_lock()/spin_unlock() from firmware_map_find_entry() and add locking documentation to firmware_map_find_entry(), explaining that the caller must hold map_entries_lock and must not release that lock until processing of firmware_map_find_entry()'s return value has completed.
Thank you for your advice, I'll fix it soon. Since you have merged the patch-set, do I need to resend all these patches again, or just send a patch to fix it based on the current one ? Thanks. :)
-- To unsubscribe from this list: send the line "unsubscribe linux-ia64" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
|