Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1460017 > unrolled thread

Re: [PATCH 2/2] arm: apply more __ro_after_init

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] arm: apply more __ro_after_init Arnd Bergmann <arnd@arndb.de> - 2016-08-10 23:50 +0200
    Re: [PATCH 2/2] arm: apply more __ro_after_init Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-08-11 01:10 +0200
      Re: [PATCH 2/2] arm: apply more __ro_after_init Arnd Bergmann <arnd@arndb.de> - 2016-08-11 18:10 +0200
        Re: [PATCH 2/2] arm: apply more __ro_after_init Daniel Thompson <daniel.thompson@linaro.org> - 2016-08-12 13:50 +0200

#1460017 — Re: [PATCH 2/2] arm: apply more __ro_after_init

On Wednesday, August 10, 2016 10:43:39 AM CEST Russell King - ARM Linux wrote:
> On Fri, Jun 03, 2016 at 11:40:24AM -0700, Kees Cook wrote:
> > @@ -1309,16 +1309,11 @@ void __init arm_mm_memblock_reserve(void)
> >   * Any other function or debugging method which may touch any device _will_
> >   * crash the kernel.
> >   */
> > +static char vectors[PAGE_SIZE * 2] __ro_after_init __aligned(PAGE_SIZE);
> >  static void __init devicemaps_init(const struct machine_desc *mdesc)
> >  {
> >       struct map_desc map;
> >       unsigned long addr;
> > -     void *vectors;
> > -
> > -     /*
> > -      * Allocate the vector page early.
> > -      */
> > -     vectors = early_alloc(PAGE_SIZE * 2);
> 
> This one is not appropriate.  We _do_ write to these pages after init
> for FIQ handler updates.  See set_fiq_handler().

Is that the only thing that modifies the page? If we think this is a
valuable change, we could make it depend on the absence of FIQ
support, as very few platforms (rpc, omap1, s3c24xx and possibly
imx) seem to even use it.

	Arnd

[toc] | [next] | [standalone]

#1460085

On Wed, Aug 10, 2016 at 09:31:05PM +0200, Arnd Bergmann wrote:
> On Wednesday, August 10, 2016 11:12:53 AM CEST Russell King - ARM Linux wrote:
> > There's the TLS emulation too, but that writes via the vectors mapping
> > at 0xffff0ff0.
> 
> Ok, so that should be safe. Can we change the fiq code to also use the
> high mapping and then take the __ro_after_init patch on top?

We can't - if the kernel is configured without the kuser helpers in
the vectors page, it's mapped read-only.  I'm not sure what the
intersection is between platforms that can have FIQs and platforms
that can disable the kuser helpers.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1460644

On Thursday, August 11, 2016 12:02:42 AM CEST Russell King - ARM Linux wrote:
> On Wed, Aug 10, 2016 at 09:31:05PM +0200, Arnd Bergmann wrote:
> > On Wednesday, August 10, 2016 11:12:53 AM CEST Russell King - ARM Linux wrote:
> > > There's the TLS emulation too, but that writes via the vectors mapping
> > > at 0xffff0ff0.
> > 
> > Ok, so that should be safe. Can we change the fiq code to also use the
> > high mapping and then take the __ro_after_init patch on top?
> 
> We can't - if the kernel is configured without the kuser helpers in
> the vectors page, it's mapped read-only. I'm not sure what the
> intersection is between platforms that can have FIQs and platforms
> that can disable the kuser helpers.

From Kconfig logic and callers of set_fiq_handler(), theoretically
there is just i.MX3, but I think they never use fiq in their
audio drivers in practice already, and Mark Brown mentioned
that we could remove fiq support in the imx audio driver (don't
remember the details at the moment).

If we can prove that i.MX3 PCM FIQ support is never used, then the
intersection is empty, and all machines that use FIQ require kuser
helpers.

This may change with Daniel Thompson's patches that use the FIQ
for NMI backtrace.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1461118

On 11/08/16 17:02, Arnd Bergmann wrote:
> On Thursday, August 11, 2016 12:02:42 AM CEST Russell King - ARM Linux wrote:
>> On Wed, Aug 10, 2016 at 09:31:05PM +0200, Arnd Bergmann wrote:
>>> On Wednesday, August 10, 2016 11:12:53 AM CEST Russell King - ARM Linux wrote:
>>>> There's the TLS emulation too, but that writes via the vectors mapping
>>>> at 0xffff0ff0.
>>>
>>> Ok, so that should be safe. Can we change the fiq code to also use the
>>> high mapping and then take the __ro_after_init patch on top?
>>
>> We can't - if the kernel is configured without the kuser helpers in
>> the vectors page, it's mapped read-only. I'm not sure what the
>> intersection is between platforms that can have FIQs and platforms
>> that can disable the kuser helpers.
>
> From Kconfig logic and callers of set_fiq_handler(), theoretically
> there is just i.MX3, but I think they never use fiq in their
> audio drivers in practice already, and Mark Brown mentioned
> that we could remove fiq support in the imx audio driver (don't
> remember the details at the moment).
>
> If we can prove that i.MX3 PCM FIQ support is never used, then the
> intersection is empty, and all machines that use FIQ require kuser
> helpers.
>
> This may change with Daniel Thompson's patches that use the FIQ
> for NMI backtrace.

It shouldn't do!

All the work I did (and am, very slowly, still doing) worked by using 
the default FIQ handler provided at boot time to jump into the perf code.

Nothing I have done or plan to do needs set_fiq_handler() to remain 
functional.

Likewise, nothing I have done should cause set_fiq_handler() to stop 
working for people who do still use it. FWIW I got the impression over 
the last few years that the most significant uses of FIQ on modern 
systems are out-of-tree uses who have designed custom FPGA hardware (and 
presumably designed them with very short FIFOs).


Daniel.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web