From: Bjorn Helgaas <helgaas@kernel.org>
To: John Garry <john.garry@huawei.com>
Cc: xuwei5@huawei.com, linuxarm@huawei.com, arm@kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
joe@perches.com
Subject: Re: [PATCH 1/5] lib: logic_pio: Fix RCU usage
Date: Fri, 21 Jun 2019 08:43:32 -0500 [thread overview]
Message-ID: <20190621134332.GC82584@google.com> (raw)
In-Reply-To: <1561026716-140537-2-git-send-email-john.garry@huawei.com>
On Thu, Jun 20, 2019 at 06:31:52PM +0800, John Garry wrote:
> The traversing of io_range_list with list_for_each_entry_rcu()
> is not properly protected by rcu_read_lock(), so add it.
>
> In addition, the list traversing used in logic_pio_register_range()
> does not need to use the rcu variant.
Not being an RCU expert myself, a few words here about why one path
needs protection but the other doesn't would be helpful. This
basically restates what the patch *does*, which is obvious from the
diff, but not *why*.
> Fixes: 031e3601869c ("lib: Add generic PIO mapping method")
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
> lib/logic_pio.c | 49 +++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/lib/logic_pio.c b/lib/logic_pio.c
> index feea48fd1a0d..761296376fbc 100644
> --- a/lib/logic_pio.c
> +++ b/lib/logic_pio.c
> @@ -46,7 +46,7 @@ int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
> end = new_range->hw_start + new_range->size;
>
> mutex_lock(&io_range_mutex);
> - list_for_each_entry_rcu(range, &io_range_list, list) {
> + list_for_each_entry(range, &io_range_list, list) {
> if (range->fwnode == new_range->fwnode) {
> /* range already there */
> goto end_register;
> @@ -108,26 +108,38 @@ int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
> */
> struct logic_pio_hwaddr *find_io_range_by_fwnode(struct fwnode_handle *fwnode)
> {
> - struct logic_pio_hwaddr *range;
> + struct logic_pio_hwaddr *range, *found_range = NULL;
>
> + rcu_read_lock();
> list_for_each_entry_rcu(range, &io_range_list, list) {
> - if (range->fwnode == fwnode)
> - return range;
> + if (range->fwnode == fwnode) {
> + found_range = range;
> + break;
> + }
> }
> - return NULL;
> + rcu_read_unlock();
> +
> + return found_range;
> }
>
> /* Return a registered range given an input PIO token */
> static struct logic_pio_hwaddr *find_io_range(unsigned long pio)
> {
> - struct logic_pio_hwaddr *range;
> + struct logic_pio_hwaddr *range, *found_range = NULL;
>
> + rcu_read_lock();
> list_for_each_entry_rcu(range, &io_range_list, list) {
> - if (in_range(pio, range->io_start, range->size))
> - return range;
> + if (in_range(pio, range->io_start, range->size)) {
> + found_range = range;
> + break;
> + }
> }
> - pr_err("PIO entry token %lx invalid\n", pio);
> - return NULL;
> + rcu_read_unlock();
> +
> + if (!found_range)
> + pr_err("PIO entry token 0x%lx invalid\n", pio);
> +
> + return found_range;
> }
>
> /**
> @@ -180,14 +192,23 @@ unsigned long logic_pio_trans_cpuaddr(resource_size_t addr)
> {
> struct logic_pio_hwaddr *range;
>
> + rcu_read_lock();
> list_for_each_entry_rcu(range, &io_range_list, list) {
> if (range->flags != LOGIC_PIO_CPU_MMIO)
> continue;
> - if (in_range(addr, range->hw_start, range->size))
> - return addr - range->hw_start + range->io_start;
> + if (in_range(addr, range->hw_start, range->size)) {
> + unsigned long cpuaddr;
> +
> + cpuaddr = addr - range->hw_start + range->io_start;
> +
> + rcu_read_unlock();
> + return cpuaddr;
> + }
> }
> - pr_err("addr %llx not registered in io_range_list\n",
> - (unsigned long long) addr);
> + rcu_read_unlock();
> +
> + pr_err("addr %pa not registered in io_range_list\n", &addr);
> +
> return ~0UL;
> }
>
> --
> 2.17.1
>
next prev parent reply other threads:[~2019-06-21 13:43 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-20 10:31 [PATCH 0/5] Fixes for HiSilicon LPC driver and logical PIO code John Garry
2019-06-20 10:31 ` [PATCH 1/5] lib: logic_pio: Fix RCU usage John Garry
2019-06-21 13:43 ` Bjorn Helgaas [this message]
2019-06-21 14:12 ` John Garry
2019-06-20 10:31 ` [PATCH 2/5] lib: logic_pio: Add logic_pio_unregister_range() John Garry
2019-06-21 13:49 ` Bjorn Helgaas
2019-06-21 14:19 ` John Garry
2019-06-20 10:31 ` [PATCH 3/5] bus: hisi_lpc: Unregister logical PIO range to avoid potential use-after-free John Garry
2019-06-20 10:31 ` [PATCH 4/5] bus: hisi_lpc: Add .remove method to avoid driver unbind crash John Garry
2019-06-21 13:56 ` Bjorn Helgaas
2019-06-21 14:33 ` John Garry
2019-06-20 10:31 ` [PATCH 5/5] lib: logic_pio: Enforce LOGIC_PIO_INDIRECT region ops are set at registration John Garry
2019-06-20 12:42 ` [PATCH 0/5] Fixes for HiSilicon LPC driver and logical PIO code Olof Johansson
2019-06-20 12:56 ` John Garry
2019-06-20 13:42 ` Olof Johansson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190621134332.GC82584@google.com \
--to=helgaas@kernel.org \
--cc=arm@kernel.org \
--cc=joe@perches.com \
--cc=john.garry@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=xuwei5@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).