Since resources can be removed, locking should ensure that the resource is not removed while accessing it. However, find_next_iomem_res() does not hold the lock while copying the data of the resource.
Keep holding the lock while the data is copied. While at it, change the return value to a more informative value. It is disregarded by the callers.
Fixes: ff3cc952d3f00 ("resource: Add remove_resource interface") Cc: stable@vger.kernel.org Cc: Borislav Petkov bp@suse.de Cc: Toshi Kani toshi.kani@hpe.com Cc: Peter Zijlstra peterz@infradead.org Cc: Dave Hansen dave.hansen@linux.intel.com Cc: Dan Williams dan.j.williams@intel.com Cc: Bjorn Helgaas bhelgaas@google.com Cc: Ingo Molnar mingo@kernel.org Signed-off-by: Nadav Amit namit@vmware.com --- kernel/resource.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/kernel/resource.c b/kernel/resource.c index 158f04ec1d4f..c0f7ba0ece52 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -365,16 +365,16 @@ static int find_next_iomem_res(resource_size_t start, resource_size_t end, break; }
+ if (p) { + /* copy data */ + res->start = max(start, p->start); + res->end = min(end, p->end); + res->flags = p->flags; + res->desc = p->desc; + } + read_unlock(&resource_lock); - if (!p) - return -1; - - /* copy data */ - res->start = max(start, p->start); - res->end = min(end, p->end); - res->flags = p->flags; - res->desc = p->desc; - return 0; + return p ? 0 : -ENODEV; }
static int __walk_iomem_res_desc(resource_size_t start, resource_size_t end,
On Wed, 12 Jun 2019 21:59:01 -0700 Nadav Amit namit@vmware.com wrote:
Since resources can be removed, locking should ensure that the resource is not removed while accessing it. However, find_next_iomem_res() does not hold the lock while copying the data of the resource.
Looks right to me.
Keep holding the lock while the data is copied. While at it, change the return value to a more informative value. It is disregarded by the callers.
The kerneldoc needs a resync:
--- a/kernel/resource.c~resource-fix-locking-in-find_next_iomem_res-fix +++ a/kernel/resource.c @@ -326,7 +326,7 @@ EXPORT_SYMBOL(release_resource); * * If a resource is found, returns 0 and @*res is overwritten with the part * of the resource that's within [@start..@end]; if none is found, returns - * -1 or -EINVAL for other invalid parameters. + * -ENODEV. Returns -EINVAL for invalid parameters. * * This function walks the whole tree and not just first level children * unless @first_lvl is true. _
linux-stable-mirror@lists.linaro.org