Messages in this thread | | | From | Loic Poulain <> | Date | Fri, 31 Mar 2023 17:52:18 +0200 | Subject | Re: [PATCH] mfd: syscon: Use of_io_request_and_map() for IO mapping |
| |
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
| |