* [PATCH] mfd: syscon: Use of_io_request_and_map() for IO mapping @ 2023-03-20 15:23 Loic Poulain 2023-03-30 11:42 ` Lee Jones 0 siblings, 1 reply; 6+ messages in thread From: Loic Poulain @ 2023-03-20 15:23 UTC (permalink / raw) To: lee, arnd; +Cc: linux-kernel, Loic Poulain Use of_io_request_and_map() instead of of_iomap() so that the region is reserved and protected, i.e reported in /proc/iomem and not accessible from user side (CONFIG_IO_STRICT_DEVMEM). Signed-off-by: Loic Poulain <loic.poulain@linaro.org> --- drivers/mfd/syscon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c index bdb2ce7ff03b..7e6d4edda118 100644 --- a/drivers/mfd/syscon.c +++ b/drivers/mfd/syscon.c @@ -60,7 +60,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) goto err_map; } - base = of_iomap(np, 0); + base = of_io_request_and_map(np, 0, NULL); if (!base) { ret = -ENOMEM; goto err_map; -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: syscon: Use of_io_request_and_map() for IO mapping 2023-03-20 15:23 [PATCH] mfd: syscon: Use of_io_request_and_map() for IO mapping Loic Poulain @ 2023-03-30 11:42 ` Lee Jones 2023-03-30 12:45 ` Arnd Bergmann 0 siblings, 1 reply; 6+ messages in thread From: Lee Jones @ 2023-03-30 11:42 UTC (permalink / raw) To: Loic Poulain; +Cc: arnd, linux-kernel On Mon, 20 Mar 2023, Loic Poulain wrote: > Use of_io_request_and_map() instead of of_iomap() so that the > region is reserved and protected, i.e reported in /proc/iomem > and not accessible from user side (CONFIG_IO_STRICT_DEVMEM). > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > --- > drivers/mfd/syscon.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c > index bdb2ce7ff03b..7e6d4edda118 100644 > --- a/drivers/mfd/syscon.c > +++ b/drivers/mfd/syscon.c > @@ -60,7 +60,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) > goto err_map; > } > > - base = of_iomap(np, 0); > + base = of_io_request_and_map(np, 0, NULL); > if (!base) { > ret = -ENOMEM; > goto err_map; Looks sane though. Arnd, do you have an opinion? -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: syscon: Use of_io_request_and_map() for IO mapping 2023-03-30 11:42 ` Lee Jones @ 2023-03-30 12:45 ` Arnd Bergmann 2023-03-30 13:22 ` Loic Poulain 0 siblings, 1 reply; 6+ messages in thread From: Arnd Bergmann @ 2023-03-30 12:45 UTC (permalink / raw) To: Lee Jones, Loic Poulain; +Cc: linux-kernel On Thu, Mar 30, 2023, at 13:42, Lee Jones wrote: > On Mon, 20 Mar 2023, Loic Poulain wrote: > >> Use of_io_request_and_map() instead of of_iomap() so that the >> region is reserved and protected, i.e reported in /proc/iomem >> and not accessible from user side (CONFIG_IO_STRICT_DEVMEM). >> >> Signed-off-by: Loic Poulain <loic.poulain@linaro.org> >> --- >> drivers/mfd/syscon.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c >> index bdb2ce7ff03b..7e6d4edda118 100644 >> --- a/drivers/mfd/syscon.c >> +++ b/drivers/mfd/syscon.c >> @@ -60,7 +60,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) >> goto err_map; >> } >> >> - base = of_iomap(np, 0); >> + base = of_io_request_and_map(np, 0, NULL); >> if (!base) { >> ret = -ENOMEM; >> goto err_map; > > Looks sane though. > > Arnd, do you have an opinion? Thanks for pinging me. I would indeed expect this to cause problems, as syscon mappings are likely to be used in a way that is not entirely clean, with other devices defining overlapping ranges. For any other driver, the change makes a lot of sense after checking the DT file, but for syscon in particular I don't think it's even realistic to check all users. Arnd ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: syscon: Use of_io_request_and_map() for IO mapping 2023-03-30 12:45 ` Arnd Bergmann @ 2023-03-30 13:22 ` Loic Poulain 2023-03-31 13:06 ` Arnd Bergmann 0 siblings, 1 reply; 6+ messages in thread From: Loic Poulain @ 2023-03-30 13:22 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Lee Jones, linux-kernel Hi Arnd, On Thu, 30 Mar 2023 at 14:45, Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Mar 30, 2023, at 13:42, Lee Jones wrote: > > On Mon, 20 Mar 2023, Loic Poulain wrote: > > > >> Use of_io_request_and_map() instead of of_iomap() so that the > >> region is reserved and protected, i.e reported in /proc/iomem > >> and not accessible from user side (CONFIG_IO_STRICT_DEVMEM). > >> > >> Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > >> --- > >> drivers/mfd/syscon.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c > >> index bdb2ce7ff03b..7e6d4edda118 100644 > >> --- a/drivers/mfd/syscon.c > >> +++ b/drivers/mfd/syscon.c > >> @@ -60,7 +60,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk) > >> goto err_map; > >> } > >> > >> - base = of_iomap(np, 0); > >> + base = of_io_request_and_map(np, 0, NULL); > >> if (!base) { > >> ret = -ENOMEM; > >> goto err_map; > > > > Looks sane though. > > > > Arnd, do you have an opinion? > > Thanks for pinging me. I would indeed expect this to cause > problems, as syscon mappings are likely to be used in a way > that is not entirely clean, with other devices defining > overlapping ranges. Isn't syscon exactly here to address that collision/overlapping issue? From a syscon perspective, it seems to be handled correctly at least since the mapping is only setup once, with the first user device (in syscon_node_to_regmap). Or are you thinking about non syscon devices overlapping the syscon area? > > For any other driver, the change makes a lot of sense after > checking the DT file, but for syscon in particular I don't > think it's even realistic to check all users. > > Arnd ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: syscon: Use of_io_request_and_map() for IO mapping 2023-03-30 13:22 ` Loic Poulain @ 2023-03-31 13:06 ` Arnd Bergmann 2023-03-31 15:52 ` Loic Poulain 0 siblings, 1 reply; 6+ messages in thread From: Arnd Bergmann @ 2023-03-31 13:06 UTC (permalink / raw) To: Loic Poulain; +Cc: Lee Jones, linux-kernel On Thu, Mar 30, 2023, at 15:22, Loic Poulain wrote: > On Thu, 30 Mar 2023 at 14:45, Arnd Bergmann <arnd@arndb.de> wrote: >> On Thu, Mar 30, 2023, at 13:42, Lee Jones wrote: >> > On Mon, 20 Mar 2023, Loic Poulain wrote: >> >> Thanks for pinging me. I would indeed expect this to cause >> problems, as syscon mappings are likely to be used in a way >> that is not entirely clean, with other devices defining >> overlapping ranges. > > Isn't syscon exactly here to address that collision/overlapping issue? > From a syscon perspective, it seems to be handled correctly at least > since the mapping is only setup once, with the first user device (in > syscon_node_to_regmap). Or are you thinking about non syscon devices > overlapping the syscon area? I meant them overlapping with other devices. Ideally this should not exist, but most likely we have to deal with dtb files where some other device does overlap with a syscon. Arnd ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: syscon: Use of_io_request_and_map() for IO mapping 2023-03-31 13:06 ` Arnd Bergmann @ 2023-03-31 15:52 ` Loic Poulain 0 siblings, 0 replies; 6+ messages in thread From: Loic Poulain @ 2023-03-31 15:52 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Lee Jones, linux-kernel On Fri, 31 Mar 2023 at 15:06, Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Mar 30, 2023, at 15:22, Loic Poulain wrote: > > On Thu, 30 Mar 2023 at 14:45, Arnd Bergmann <arnd@arndb.de> wrote: > >> On Thu, Mar 30, 2023, at 13:42, Lee Jones wrote: > >> > On Mon, 20 Mar 2023, Loic Poulain wrote: > >> > >> Thanks for pinging me. I would indeed expect this to cause > >> problems, as syscon mappings are likely to be used in a way > >> that is not entirely clean, with other devices defining > >> overlapping ranges. > > > > Isn't syscon exactly here to address that collision/overlapping issue? > > From a syscon perspective, it seems to be handled correctly at least > > since the mapping is only setup once, with the first user device (in > > syscon_node_to_regmap). Or are you thinking about non syscon devices > > overlapping the syscon area? > > I meant them overlapping with other devices. Ideally this should > not exist, but most likely we have to deal with dtb files where > some other device does overlap with a syscon. Ok in that case, this is indeed a no-go, but my main concern is about CONFIG_IO_STRICT_DEVMEM which is supposed to restrict userspace access to *idle* io-memory only, i.e. not in-use by a driver. In our case the range is obviously in-use by one or even several drivers, but because the range is not requested and marked busy, we fail to prevent such access. It's kind of a security concern since syscon is widely adopted in devicetree, sometimes for sensitive controllers such as OTP, platform-reset, etc. There are some alternatives, that may not be entirely satisfying: - Introduce a new syscon 'exclusive' devicetree property, for enforcing range request while keeping compatibility with 'bad' overlapping dtb. - Use range requests only when CONFIG_IO_STRICT_DEVMEM is enabled, ensuring security, but possibly breaking some devices. - Introduce a new 'IORESOURCE_SHARED' flag for syscon resources, allowing the resource framework to allocate overlapping IORESOURCE_BUSY regions while preventing userspace access/mapping. Regards, Loic ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-31 15:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-20 15:23 [PATCH] mfd: syscon: Use of_io_request_and_map() for IO mapping Loic Poulain 2023-03-30 11:42 ` Lee Jones 2023-03-30 12:45 ` Arnd Bergmann 2023-03-30 13:22 ` Loic Poulain 2023-03-31 13:06 ` Arnd Bergmann 2023-03-31 15:52 ` Loic Poulain
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox