* Re: [PATCH v5 1/7] ASoC: dt-bindings: fsl_asrc: Add new property fsl,asrc-format
From: Nicolin Chen @ 2020-03-23 21:20 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland, devicetree, alsa-devel, timur, Xiubo.Lee,
linuxppc-dev, Shengjiu Wang, tiwai, lgirdwood, perex, broonie,
festevam, linux-kernel
In-Reply-To: <20200320173213.GA9093@bogus>
On Fri, Mar 20, 2020 at 11:32:13AM -0600, Rob Herring wrote:
> On Mon, Mar 09, 2020 at 02:19:44PM -0700, Nicolin Chen wrote:
> > On Mon, Mar 09, 2020 at 11:58:28AM +0800, Shengjiu Wang wrote:
> > > In order to support new EASRC and simplify the code structure,
> > > We decide to share the common structure between them. This bring
> > > a problem that EASRC accept format directly from devicetree, but
> > > ASRC accept width from devicetree.
> > >
> > > In order to align with new ESARC, we add new property fsl,asrc-format.
> > > The fsl,asrc-format can replace the fsl,asrc-width, then driver
> > > can accept format from devicetree, don't need to convert it to
> > > format through width.
> > >
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > > Documentation/devicetree/bindings/sound/fsl,asrc.txt | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> > > index cb9a25165503..780455cf7f71 100644
> > > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> > > @@ -51,6 +51,11 @@ Optional properties:
> > > will be in use as default. Otherwise, the big endian
> > > mode will be in use for all the device registers.
> > >
> > > + - fsl,asrc-format : Defines a mutual sample format used by DPCM Back
> > > + Ends, which can replace the fsl,asrc-width.
> > > + The value is SNDRV_PCM_FORMAT_S16_LE, or
> > > + SNDRV_PCM_FORMAT_S24_LE
> >
> > I am still holding the concern at the DT binding of this format,
> > as it uses values from ASoC header file instead of a dt-binding
> > header file -- not sure if we can do this. Let's wait for Rob's
> > comments.
>
> I assume those are an ABI as well, so it's okay to copy them unless we
They are defined under include/uapi. So I think we can use them?
> already have some format definitions for DT. But it does need to be copy
> in a header under include/dt-bindings/.
Shengjiu is actually quoting those integral values, rather than
those macros, so actually no need copy to include/dt-bindings,
yet whoever adds this format property to a new DT would need to
look up the value in a header file under include/uapi. I's just
wondering if that's okay.
Thanks
^ permalink raw reply
* Re: [PATCH v8 04/14] powerpc/vas: Alloc and setup IRQ and trigger port address
From: Haren Myneni @ 2020-03-23 19:02 UTC (permalink / raw)
To: Cédric Le Goater
Cc: mikey, herbert, Frederic Barrat, npiggin, hch, oohall, sukadev,
linuxppc-dev, ajd
In-Reply-To: <bd846a9c-0e21-1d97-0b03-e01c35ff01ae@kaod.org>
On Mon, 2020-03-23 at 10:27 +0100, Cédric Le Goater wrote:
> On 3/23/20 10:06 AM, Cédric Le Goater wrote:
> > On 3/19/20 7:14 AM, Haren Myneni wrote:
> >>
> >> Alloc IRQ and get trigger port address for each VAS instance. Kernel
> >> register this IRQ per VAS instance and sets this port for each send
> >> window. NX interrupts the kernel when it sees page fault.
> >
> > I don't understand why this is not done by the OPAL driver for each VAS
> > of the system. Is the VAS unit very different from OpenCAPI regarding
> > the fault ?
>
> I checked the previous patchsets and I see that v3 was more like I expected
> it: one interrupt for faults allocated by the skiboot driver and exposed
> in the DT.
>
> What made you change your mind ?
>
> This version is hijacking the lowlevel routines of the XIVE irqchip which
> is not the best approach. OCXL is doing that because it needs to allocate
> interrupts for the user space processes using the AFU and we should rework
> that part.
>
> However, the translation fault interrupt is allocated by skiboot.
Sorry my mistake. I should have CC you earlier.
Each VAS instance will generate fault interrupt which is per chip. There
won't be other job completion interrupts.
Correct, V3 used allocating interrupts per chip in skiboot and exposed
in DT. Since XIVE code has similar feature, exploited this approach so
that we do not need skiboot changes.
Thanks
Haren
>
> Sorry for the noise, I would like to understand more how this works. I also
> have passthrough in mind.
>
> C.
>
^ permalink raw reply
* Re: [PATCH 0/10] NS 8250 UART Device Tree improvements
From: Rob Herring @ 2020-03-23 19:01 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Mark Rutland, devicetree, Mans Rullgard, Marc Gonzalez,
Greg Kroah-Hartman, linuxppc-dev, Haojian Zhuang,
linux-kernel@vger.kernel.org, Paul Mackerras,
open list:SERIAL DRIVERS, Robert Jarzmik, Daniel Mack
In-Reply-To: <20200320174107.29406-1-lkundrak@v3.sk>
On Fri, Mar 20, 2020 at 11:41 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> Hi,
>
> this series aims to make it possible to validate NS 8250 compatible serial port
> nodes in Device Tree. It ultimately ends up converting the 8250.txt binding
> specification to YAML for json-schema.
>
> It starts by fixing up a couple of issues that would fail validation of
> device trees for various boards. Note there might be validation issues in other
> boards -- I don't have computing power to run "make dtbs_check" with
> CONFIG_OF_ALL_DTBS=y at the moment. I'm happy to fix up issues if somebody
> runs the test output to me.
>
> Unless someone has a different idea, I'd like to submit this to arm-soc once
> I get the Acks from PXA and Tango maintainers as well as DT reviewers:
>
> [PATCH 01/10] ARM: dts: pxa*: Don't redeclare phandle references
> [PATCH 02/10] ARM: dts: pxa*: Fix serial port names
> [PATCH 03/10] ARM: dts: pxa*: Make the serial ports compatible with
> [PATCH 04/10] ARM: dts: mmp2-brownstone: Don't redeclare phandle
> [PATCH 05/10] ARM: dts: mmp*: Fix serial port names
> [PATCH 06/10] ARM: dts: mmp*: Make the serial ports compatible with
> [PATCH 07/10] ARM: dts: tango4: Make /serial compatible with ns16550a
For 1-7:
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 10/10] dt-bindings: serial: Convert 8250 to json-schema
From: Rob Herring @ 2020-03-23 19:00 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Mark Rutland, devicetree, Mans Rullgard, Marc Gonzalez,
Greg Kroah-Hartman, linuxppc-dev, Haojian Zhuang,
linux-kernel@vger.kernel.org, Paul Mackerras,
open list:SERIAL DRIVERS, Robert Jarzmik, Daniel Mack
In-Reply-To: <20200320174107.29406-11-lkundrak@v3.sk>
On Fri, Mar 20, 2020 at 11:41 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> Some fixes were done during the conversion:
Thanks for doing this!
>
> Slightly better examples. The original example was for an OMAP serial
> port, which is not even described by this binding, but by
> omap_serial.txt instead.
>
> Added compatible strings, that were used, byt not documented:
> andestech,uart16550, cavium,octeon-3860-uart, fsl,16550-FIFO64,
> nvidia,tegra186-uart, nvidia,tegra194-uart, nxp,lpc1850-uart,
> opencores,uart16550-rtlsvn105, ralink,mt7620a-uart, ralink,rt3052-uart,
> ralink,rt3883-uart and xlnx,xps-uart16550-2.00.b.
>
> Removed "serial" compatible string. It's redundant with the node name
> (which, in OFW, serves the same purpose as the compatible string).
Yeah, that seems fairly useless to me.
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
> .../devicetree/bindings/serial/8250.txt | 102 --------
> .../devicetree/bindings/serial/8250.yaml | 246 ++++++++++++++++++
> 2 files changed, 246 insertions(+), 102 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/serial/8250.txt
> create mode 100644 Documentation/devicetree/bindings/serial/8250.yaml
[...]
> diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
> new file mode 100644
> index 0000000000000..88192f94385b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/8250.yaml
> @@ -0,0 +1,246 @@
> +# Copyright 2020 Lubomir Rintel <lkundrak@v3.sk>
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serial/8250.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: UART (Universal Asynchronous Receiver/Transmitter) bindings
Perhaps: "8250 series UART..."
> +
> +maintainers:
> + - devicetree@vger.kernel.org
> +
> +allOf:
> + - $ref: /schemas/serial.yaml#
> + - if:
> + not:
> + required:
> + - clock-frequency
> + then:
> + required:
> + - clocks
> + - if:
> + not:
> + required:
> + - clocks
> + then:
> + required:
> + - clock-frequency
anyOf:
- required: [ clock-frequency ]
- required: [ clocks ]
> + - if:
> + required:
> + - aspeed,sirq-polarity-sense
> + then:
> + properties:
> + compatible:
> + const: aspeed,ast2500-vuart
> + - if:
> + properties:
> + compatible:
> + const: mrvl,mmp-uart
> + then:
> + properties:
> + reg-shift:
> + const: 2
> + required:
> + - reg-shift
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: ns8250
> + - const: ns16450
> + - const: ns16550
> + - const: ns16550a
> + - const: ns16850
> + - const: aspeed,ast2400-vuart
> + - const: aspeed,ast2500-vuart
> + - const: intel,xscale-uart
> + - const: mrvl,pxa-uart
> + - const: nuvoton,npcm750-uart
> + - const: nvidia,tegra20-uart
> + - const: nxp,lpc3220-uart
> + - items:
> + - enum:
> + - altr,16550-FIFO32
> + - altr,16550-FIFO64
> + - altr,16550-FIFO128
> + - fsl,16550-FIFO64
> + - fsl,ns16550
> + - andestech,uart16550
> + - nxp,lpc1850-uart
> + - opencores,uart16550-rtlsvn105
> + - ti,da830-uart
> + - const: ns16550a
> + - items:
> + - enum:
> + - ns16750
> + - cavium,octeon-3860-uart
> + - xlnx,xps-uart16550-2.00.b
> + - ralink,rt2880-uart
> + - const: ns16550
Seems to me these are just as likely to be wrong as the fsl,ns16550 as
it's unlikely the FIFO is broken. Perhaps a note here to use
'ns16550a' instead unless you have a broken FIFO.
> + - items:
> + - enum:
> + - ralink,mt7620a-uart
> + - ralink,rt3052-uart
> + - ralink,rt3883-uart
> + - const: ralink,rt2880-uart
> + - const: ns16550
> + - items:
> + - enum:
> + - mediatek,mt7622-btif
> + - mediatek,mt7623-btif
> + - const: mediatek,mtk-btif
> + - items:
> + - enum:
> + - mediatek,mt7622-btif
> + - mediatek,mt7623-btif
> + - const: mediatek,mtk-btif
> + - items:
> + - const: mrvl,mmp-uart
> + - const: intel,xscale-uart
> + - items:
> + - enum:
> + - nvidia,tegra30-uart
> + - nvidia,tegra114-uart
> + - nvidia,tegra124-uart
> + - nvidia,tegra186-uart
> + - nvidia,tegra194-uart
> + - nvidia,tegra210-uart
> + - const: nvidia,tegra20-uart
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clock-frequency: true
> +
> + clocks:
> + maxItems: 1
> +
> + current-speed:
> + description: The current active speed of the UART.
Needs a type reference.
> + reg-offset:
> + description: |
> + Offset to apply to the mapbase from the start of the registers.
> +
> + reg-shift:
> + description: Quantity to shift the register offsets by.
> +
> + reg-io-width:
> + description: |
> + The size (in bytes) of the IO accesses that should be performed on the
> + device. There are some systems that require 32-bit accesses to the
> + UART (e.g. TI davinci).
We'll assume these have a type definition elsewhere.
> +
> + used-by-rtas:
> + description: |
> + Set to indicate that the port is in use by the OpenFirmware RTAS and
> + should not be registered.
Needs a type reference.
> +
> + no-loopback-test:
> + description: |
> + Set to indicate that the port does not implements loopback test mode.
s/implements/implement/
Needs a type reference (or 'type: boolean').
> +
> + fifo-size:
> + description: The fifo size of the UART.
> +
> + auto-flow-control:
> + description: |
> + One way to enable automatic flow control support. The driver is
> + allowed to detect support for the capability even without this
> + property.
> +
> + tx-threshold:
> + description: |
> + Specify the TX FIFO low water indication for parts with programmable
> + TX FIFO thresholds.
Need a type reference for all these.
> +
> + resets:
> + description: Phandle + reset specifier pairs
Is there a range of number of resets? Use minItems/maxItems.
> +
> + overrun-throttle-ms:
> + description: |
> + How long to pause uart rx when input overrun is encountered.
> +
> + rts-gpios:
> + description: |
> + Specify a GPIO for RTS line to be used instead of the peripheral
> + function pin for the UART feature.
No need to define properties from serial.yaml again. Just 'rts-gpios: true'.
> +
> + cts-gpios:
> + description: |
> + Specify a GPIO for CTS line to be used instead of the peripheral
> + function pin for the UART feature.
> +
> + dtr-gpios:
> + description: |
> + Specify a GPIO for DTR line to be used instead of the peripheral
> + function pin for the UART feature.
> +
> + dsr-gpios:
> + description: |
> + Specify a GPIO for DSR line to be used instead of the peripheral
> + function pin for the UART feature.
> +
> + rng-gpios:
> + description: |
> + Specify a GPIO for RNG line to be used instead of the peripheral
> + function pin for the UART feature.
> +
> + dcd-gpios:
> + description: |
> + Specify a GPIO for DCD line to be used instead of the peripheral
> + function pin for the UART feature.
> +
> + aspeed,sirq-polarity-sense:
> + $ref: '/schemas/types.yaml#/definitions/phandle-array'
> + description: |
> + Phandle to aspeed,ast2500-scu compatible syscon alongside register
> + offset and bit number to identify how the SIRQ polarity should be
> + configured. One possible data source is the LPC/eSPI mode bit. Only
> + applicable to aspeed,ast2500-vuart.
> +
> +required:
> + - reg
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + serial@80230000 {
> + compatible = "ns8250";
> + reg = <0x80230000 0x100>;
> + interrupts = <10>;
> + reg-shift = <2>;
> + clock-frequency = <48000000>;
> + };
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + serial@49042000 {
> + compatible = "andestech,uart16550", "ns16550a";
> + reg = <0x49042000 0x400>;
> + interrupts = <80>;
> + clock-frequency = <48000000>;
> + cts-gpios = <&gpio3 5 GPIO_ACTIVE_LOW>;
> + rts-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
> + dtr-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
> + dsr-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
> + dcd-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
> + rng-gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
> + };
> + - |
> + #include <dt-bindings/clock/aspeed-clock.h>
> + serial@1e787000 {
> + compatible = "aspeed,ast2500-vuart";
> + reg = <0x1e787000 0x40>;
> + reg-shift = <2>;
> + interrupts = <8>;
> + clocks = <&syscon ASPEED_CLK_APB>;
> + no-loopback-test;
> + aspeed,sirq-polarity-sense = <&syscon 0x70 25>;
> + };
> +
> +...
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH] Documentation: Clarify better about the rwsem non-owner release issue
From: Joel Fernandes @ 2020-03-23 18:23 UTC (permalink / raw)
To: Kalle Valo
Cc: linux-doc, Peter Zijlstra, Sebastian Andrzej Siewior,
linux-kernel, netdev, Will Deacon, Thomas Gleixner,
Davidlohr Bueso, Arnd Bergmann, Jonathan Corbet, Ingo Molnar,
Ingo Molnar, Paul E . McKenney, linuxppc-dev, Steven Rostedt,
Bjorn Helgaas, Kurt Schwemmer,
linux-pci@vger.kernel.org Felipe Balbi, Logan Gunthorpe,
Randy Dunlap, linux-wireless, Oleg Nesterov, Greg Kroah-Hartman,
Linus Torvalds, David S. Miller
In-Reply-To: <87a748khlo.fsf@kamboji.qca.qualcomm.com>
On Sun, Mar 22, 2020 at 08:51:15AM +0200, Kalle Valo wrote:
> "Joel Fernandes (Google)" <joel@joelfernandes.org> writes:
>
> > Reword and clarify better about the rwsem non-owner release issue.
> >
> > Link: https://lore.kernel.org/linux-pci/20200321212144.GA6475@google.com/
> >
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>
> There's something wrong with your linux-pci and linux-usb addresses:
>
> "linux-pci@vger.kernel.org Felipe Balbi" <balbi@kernel.org>,
>
>
> "linux-usb@vger.kernel.org Kalle Valo" <kvalo@codeaurora.org>,
Not sure. It appears fine in the archive. Thomas, let me know if you wanted
me to resend the diff patch. Hopefully it squashed fine into your original
patch.
thanks,
- Joel
^ permalink raw reply
* Re: [PATCH v8 03/14] powerpc/vas: Define nx_fault_stamp in coprocessor_request_block
From: Haren Myneni @ 2020-03-23 18:13 UTC (permalink / raw)
To: Michael Ellerman
Cc: mikey, ajd, Nicholas Piggin, hch, oohall, sukadev, linuxppc-dev,
herbert
In-Reply-To: <87bloncnnx.fsf@mpe.ellerman.id.au>
On Mon, 2020-03-23 at 22:32 +1100, Michael Ellerman wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
> > Haren Myneni's on March 19, 2020 4:13 pm:
> >>
> >> Kernel sets fault address and status in CRB for NX page fault on user
> >> space address after processing page fault. User space gets the signal
> >> and handles the fault mentioned in CRB by bringing the page in to
> >> memory and send NX request again.
> >>
> >> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> >> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> >> ---
> >> arch/powerpc/include/asm/icswx.h | 18 +++++++++++++++++-
> >> 1 file changed, 17 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
> >> index 9872f85..b233d1e 100644
> >> --- a/arch/powerpc/include/asm/icswx.h
> >> +++ b/arch/powerpc/include/asm/icswx.h
> >
> > "icswx" is not a thing anymore, after 6ff4d3e96652 ("powerpc: Remove old
> > unused icswx based coprocessor support").
>
> Yeah that commit ripped out some parts of the previous attempt at a user
> visible API for this sort of "coprocessor" stuff. VAS is yet another
> attempt to do something useful with most of the same pieces but some
> slightly different details.
>
> > I guess NX is reusing some
> > things from it, but it would be good to get rid of the cruft and re-name
> > this file and and relevant names.
>
> > NX already uses this file, so I guesss that can happen after this series.
>
> A lot of the CRB/CSB stuff is still the same, and P8 still uses icswx.
> But I'd be happy if the header was renamed eventually, as icswx is now a
> legacy name.
We can move all macros and struct definitions to vas.h and remove
icswx.h. Can I do this after this series?
Thanks
Haren
>
> cheers
^ permalink raw reply
* Re: [PATCH 08/10] powerpc/fsl: Make serial ports compatible with ns16550a
From: Rob Herring @ 2020-03-23 17:59 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Mark Rutland, devicetree, Mans Rullgard, Marc Gonzalez,
Greg Kroah-Hartman, linuxppc-dev, Haojian Zhuang,
linux-kernel@vger.kernel.org, Paul Mackerras,
open list:SERIAL DRIVERS, Robert Jarzmik, Daniel Mack
In-Reply-To: <20200320174107.29406-9-lkundrak@v3.sk>
On Fri, Mar 20, 2020 at 11:41 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> There are separate compatible strings for ns16550 and ns16550a and the
> Freescale serial port is compatible with the latter one, with working
> FIFO.
I don't think changing this is right. First, 'ns16550' is what's
documented in the DT spec (and ePAPR which it's based on).
Second, these dts files are pretty mature/stable, so I'm less inclined
to change them. AFAICT, this change either enables the fifo (which
would be an untested change in behavior) or has no effect because the
fifo is auto-detected.
> Use the appropriate compatible string that is also consistent with ARM64
> Freescale boards and thus avoiding one special case in the binding schema.
Well, that's annoying...
Rob
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Christoph Hellwig @ 2020-03-23 17:22 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Alexey Kardashevskiy, Greg Kroah-Hartman, Joerg Roedel,
Robin Murphy, linux-kernel, iommu, linuxppc-dev,
Christoph Hellwig, Lu Baolu
In-Reply-To: <87sghz2ibh.fsf@linux.ibm.com>
On Mon, Mar 23, 2020 at 09:07:38PM +0530, Aneesh Kumar K.V wrote:
>
> This is what I was trying, but considering I am new to DMA subsystem, I
> am not sure I got all the details correct. The idea is to look at the
> cpu addr and see if that can be used in direct map fashion(is
> bus_dma_limit the right restriction here?) if not fallback to dynamic
> IOMMU mapping.
I don't think we can throw all these complications into the dma
mapping code. At some point I also wonder what the point is,
especially for scatterlist mappings, where the iommu can coalesce.
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Christoph Hellwig @ 2020-03-23 17:20 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
iommu, Aneesh Kumar K.V, linuxppc-dev, Christoph Hellwig,
Lu Baolu
In-Reply-To: <37ce1b7e-264d-292d-32b1-093b24b3525c@ozlabs.ru>
On Mon, Mar 23, 2020 at 07:58:01PM +1100, Alexey Kardashevskiy wrote:
> >> 0x100.0000.0000 .. 0x101.0000.0000
> >>
> >> 2x4G, each is 1TB aligned. And we can map directly only the first 4GB
> >> (because of the maximum IOMMU table size) but not the other. And 1:1 on
> >> that "pseries" is done with offset=0x0800.0000.0000.0000.
> >>
> >> So we want to check every bus address against dev->bus_dma_limit, not
> >> dev->coherent_dma_mask. In the example above I'd set bus_dma_limit to
> >> 0x0800.0001.0000.0000 and 1:1 mapping for the second 4GB would not be
> >> tried. Does this sound reasonable? Thanks,
> >
> > bus_dma_limit is just another limiting factor applied on top of
> > coherent_dma_mask or dma_mask respectively.
>
> This is not enough for the task: in my example, I'd set bus limit to
> 0x0800.0001.0000.0000 but this would disable bypass for all RAM
> addresses - the first and the second 4GB blocks.
So what about something like the version here:
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-bypass.3
^ permalink raw reply
* Re: [RFC PATCH 0/3] Use per-CPU temporary mappings for patching
From: Christophe Leroy @ 2020-03-23 16:59 UTC (permalink / raw)
To: Christopher M Riedl; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <738663735.45060.1584982175261@privateemail.com>
Le 23/03/2020 à 17:49, Christopher M Riedl a écrit :
>
>> On March 23, 2020 9:04 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>
>>
>> On 03/23/2020 11:30 AM, Christophe Leroy wrote:
>>>
>>>
>>> On 03/23/2020 04:52 AM, Christopher M. Riedl wrote:
>>>> When compiled with CONFIG_STRICT_KERNEL_RWX, the kernel must create
>>>> temporary mappings when patching itself. These mappings temporarily
>>>> override the strict RWX text protections to permit a write. Currently,
>>>> powerpc allocates a per-CPU VM area for patching. Patching occurs as
>>>> follows:
>>>>
>>>> 1. Map page of text to be patched to per-CPU VM area w/
>>>> PAGE_KERNEL protection
>>>> 2. Patch text
>>>> 3. Remove the temporary mapping
>>>>
>>>> While the VM area is per-CPU, the mapping is actually inserted into the
>>>> kernel page tables. Presumably, this could allow another CPU to access
>>>> the normally write-protected text - either malicously or accidentally -
>>>> via this same mapping if the address of the VM area is known. Ideally,
>>>> the mapping should be kept local to the CPU doing the patching (or any
>>>> other sensitive operations requiring temporarily overriding memory
>>>> protections) [0].
>>>>
>>>> x86 introduced "temporary mm" structs which allow the creation of
>>>> mappings local to a particular CPU [1]. This series intends to bring the
>>>> notion of a temporary mm to powerpc and harden powerpc by using such a
>>>> mapping for patching a kernel with strict RWX permissions.
>>>>
>>>> The first patch introduces the temporary mm struct and API for powerpc
>>>> along with a new function to retrieve a current hw breakpoint.
>>>>
>>>> The second patch uses the `poking_init` init hook added by the x86
>>>> patches to initialize a temporary mm and patching address. The patching
>>>> address is randomized between 0 and DEFAULT_MAP_WINDOW-PAGE_SIZE. The
>>>> upper limit is necessary due to how the hash MMU operates - by default
>>>> the space above DEFAULT_MAP_WINDOW is not available. For now, both hash
>>>> and radix randomize inside this range. The number of possible random
>>>> addresses is dependent on PAGE_SIZE and limited by DEFAULT_MAP_WINDOW.
>>>>
>>>> Bits of entropy with 64K page size on BOOK3S_64:
>>>>
>>>> bits-o-entropy = log2(DEFAULT_MAP_WINDOW_USER64 / PAGE_SIZE)
>>>>
>>>> PAGE_SIZE=64K, DEFAULT_MAP_WINDOW_USER64=128TB
>>>> bits-o-entropy = log2(128TB / 64K)
>>>> bits-o-entropy = 31
>>>>
>>>> Currently, randomization occurs only once during initialization at boot.
>>>>
>>>> The third patch replaces the VM area with the temporary mm in the
>>>> patching code. The page for patching has to be mapped PAGE_SHARED with
>>>> the hash MMU since hash prevents the kernel from accessing userspace
>>>> pages with PAGE_PRIVILEGED bit set. There is on-going work on my side to
>>>> explore if this is actually necessary in the hash codepath.
>>>>
>>>> Testing so far is limited to booting on QEMU (power8 and power9 targets)
>>>> and a POWER8 VM along with setting some simple xmon breakpoints (which
>>>> makes use of code-patching). A POC lkdtm test is in-progress to actually
>>>> exploit the existing vulnerability (ie. the mapping during patching is
>>>> exposed in kernel page tables and accessible by other CPUS) - this will
>>>> accompany a future v1 of this series.
>>>
>>> Got following failures on an 8xx. Note that "fault blocked by AP
>>> register !" means an unauthorised access from Kernel to Userspace.
>>>
>
> So I am pretty ignorant on 8xx. Can you share a config I can build?
> Can I test this on my end w/ QEMU? I suspect there are some subtleties
> in the 8xx MMU I did not consider.
>
>
As config you can take mpc885_ads_defconfig
Unfortunately the 8xx is not supported by QEMU AFAIK.
By the way, first point is you are using userspace for the mapping, this
means you have to modify the patching code to use put_user(), otherwise
accesses are blocked when CONFIG_PPC_KUAP is set. That should also be
the case on power9 with Radix.
Christophe
^ permalink raw reply
* Re: hardcoded SIGSEGV in __die() ?
From: Joakim Tjernlund @ 2020-03-23 15:44 UTC (permalink / raw)
To: christophe.leroy@c-s.fr, linuxppc-dev@ozlabs.org
In-Reply-To: <43a21b20-4214-1043-0919-1bd8078d14a6@c-s.fr>
On Mon, 2020-03-23 at 16:31 +0100, Christophe Leroy wrote:
>
> Le 23/03/2020 à 16:08, Joakim Tjernlund a écrit :
> > On Mon, 2020-03-23 at 15:45 +0100, Christophe Leroy wrote:
> > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> > >
> > >
> > > Le 23/03/2020 à 15:43, Christophe Leroy a écrit :
> > > > Le 23/03/2020 à 15:17, Joakim Tjernlund a écrit :
> > > > > In __die(), see below, there is this call to notify_send() with
> > > > > SIGSEGV hardcoded, this seems odd
> > > > > to me as the variable "err" holds the true signal(in my case SIGBUS)
> > > > > Should not SIGSEGV be replaced with the true signal no.?
> > > >
> > > > As far as I can see, comes from
> > > > https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3D66fcb1059&data=02%7C01%7CJoakim.Tjernlund%40infinera.com%7Cefe6d37a85e1494658ec08d7cf3f513f%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637205743206770599&sdata=k8%2Bs7ifiCyuNzXuOhykjXUEtWzD62q3HGIIiavqE6%2FA%3D&reserved=0
> > > >
> > >
> > > And
> > > https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3Dae87221d3ce49d9de1e43756da834fd0bf05a2ad&data=02%7C01%7CJoakim.Tjernlund%40infinera.com%7Cefe6d37a85e1494658ec08d7cf3f513f%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637205743206770599&sdata=oCU%2FMelrWDOCjmGOfVuNp2tM%2BwQ%2BRD25jzRWoGbHAew%3D&reserved=0
> > > shows it is (was?) similar on x86.
> > >
> >
> > I tried to follow that chain thinking it would end up sending a signal to user space but I cannot see
> > that happens. Seems to be related to debugging.
> >
> > In short, I cannot see any signal being delivered to user space. If so that would explain why
> > our user space process never dies.
> > Is there a signal hidden in machine_check handler for SIGBUS I cannot see?
> >
>
> Isn't it done in do_exit(), called from oops_end() ?
hmm, so it seems. The odd thing though is that do_exit takes an exit code, not signal number.
Also, feels a bit odd to force an exit(that we haven't seen happening) rather than just a signal.
Jocke
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Aneesh Kumar K.V @ 2020-03-23 15:37 UTC (permalink / raw)
To: Christoph Hellwig, Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
iommu, linuxppc-dev, Christoph Hellwig, Lu Baolu
In-Reply-To: <20200323085059.GA32528@lst.de>
Christoph Hellwig <hch@lst.de> writes:
> On Mon, Mar 23, 2020 at 09:37:05AM +0100, Christoph Hellwig wrote:
>> > > + /*
>> > > + * Allows IOMMU drivers to bypass dynamic translations if the DMA mask
>> > > + * is large enough.
>> > > + */
>> > > + if (dev->dma_ops_bypass) {
>> > > + if (min_not_zero(dev->coherent_dma_mask, dev->bus_dma_limit) >=
>> > > + dma_direct_get_required_mask(dev))
>> > > + return true;
>> > > + }
>> >
>> >
>> > Why not do this in dma_map_direct() as well?
>>
>> Mostly beacuse it is a relatively expensive operation, including a
>> fls64.
>
> Which I guess isn't too bad compared to a dynamic IOMMU mapping. Can
> you just send a draft patch for what you'd like to see for ppc?
This is what I was trying, but considering I am new to DMA subsystem, I
am not sure I got all the details correct. The idea is to look at the
cpu addr and see if that can be used in direct map fashion(is
bus_dma_limit the right restriction here?) if not fallback to dynamic
IOMMU mapping.
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index e486d1d78de2..bc7e6a8b2caa 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -31,6 +31,87 @@ static inline bool dma_iommu_map_bypass(struct device *dev,
(!iommu_fixed_is_weak || (attrs & DMA_ATTR_WEAK_ORDERING));
}
+static inline bool __dma_direct_map_capable(struct device *dev, struct page *page,
+ unsigned long offset, size_t size)
+{
+ phys_addr_t phys = page_to_phys(page) + offset;
+ dma_addr_t dma_addr = phys_to_dma(dev, phys);
+ dma_addr_t end = dma_addr + size - 1;
+
+ return end <= min_not_zero(*dev->dma_mask, dev->bus_dma_limit);
+}
+
+static inline bool dma_direct_map_capable(struct device *dev, struct page *page,
+ unsigned long offset, size_t size,
+ unsigned long attrs)
+{
+ if (!dma_iommu_map_bypass(dev, attrs))
+ return false;
+
+ if (!dev->dma_mask)
+ return false;
+
+ return __dma_direct_map_capable(dev, page, offset, size);
+}
+
+
+static inline bool dma_direct_unmap_capable(struct device *dev, dma_addr_t addr, size_t size,
+ unsigned long attrs)
+{
+ dma_addr_t end = addr + size - 1;
+
+ if (!dma_iommu_map_bypass(dev, attrs))
+ return false;
+
+ if (!dev->dma_mask)
+ return false;
+
+ return end <= min_not_zero(*dev->dma_mask, dev->bus_dma_limit);
+}
+
+static inline bool dma_direct_sg_map_capable(struct device *dev, struct scatterlist *sglist,
+ int nelems, unsigned long attrs)
+{
+ int i;
+ struct scatterlist *sg;
+
+ if (!dma_iommu_map_bypass(dev, attrs))
+ return false;
+
+ if (!dev->dma_mask)
+ return false;
+
+ for_each_sg(sglist, sg, nelems, i) {
+ if (!__dma_direct_map_capable(dev, sg_page(sg),
+ sg->offset, sg->length))
+ return false;
+ }
+ return true;
+}
+
+static inline bool dma_direct_sg_unmap_capable(struct device *dev, struct scatterlist *sglist,
+ int nelems, unsigned long attrs)
+{
+ int i;
+ dma_addr_t end;
+ struct scatterlist *sg;
+
+ if (!dma_iommu_map_bypass(dev, attrs))
+ return false;
+
+ if (!dev->dma_mask)
+ return false;
+
+ for_each_sg(sglist, sg, nelems, i) {
+ end = sg->dma_address + sg_dma_len(sg);
+
+ if (end > min_not_zero(*dev->dma_mask, dev->bus_dma_limit))
+ return false;
+ }
+ return true;
+}
+
+
/* Allocates a contiguous real buffer and creates mappings over it.
* Returns the virtual address of the buffer and sets dma_handle
* to the dma address (mapping) of the first page.
@@ -67,7 +148,7 @@ static dma_addr_t dma_iommu_map_page(struct device *dev, struct page *page,
enum dma_data_direction direction,
unsigned long attrs)
{
- if (dma_iommu_map_bypass(dev, attrs))
+ if (dma_direct_map_capable(dev, page, offset, size, attrs))
return dma_direct_map_page(dev, page, offset, size, direction,
attrs);
return iommu_map_page(dev, get_iommu_table_base(dev), page, offset,
@@ -79,7 +160,7 @@ static void dma_iommu_unmap_page(struct device *dev, dma_addr_t dma_handle,
size_t size, enum dma_data_direction direction,
unsigned long attrs)
{
- if (!dma_iommu_map_bypass(dev, attrs))
+ if (!dma_direct_unmap_capable(dev, dma_handle, size, attrs))
iommu_unmap_page(get_iommu_table_base(dev), dma_handle, size,
direction, attrs);
else
@@ -91,7 +172,7 @@ static int dma_iommu_map_sg(struct device *dev, struct scatterlist *sglist,
int nelems, enum dma_data_direction direction,
unsigned long attrs)
{
- if (dma_iommu_map_bypass(dev, attrs))
+ if (dma_direct_sg_map_capable(dev, sglist, nelems, attrs))
return dma_direct_map_sg(dev, sglist, nelems, direction, attrs);
return ppc_iommu_map_sg(dev, get_iommu_table_base(dev), sglist, nelems,
dma_get_mask(dev), direction, attrs);
@@ -101,7 +182,7 @@ static void dma_iommu_unmap_sg(struct device *dev, struct scatterlist *sglist,
int nelems, enum dma_data_direction direction,
unsigned long attrs)
{
- if (!dma_iommu_map_bypass(dev, attrs))
+ if (!dma_direct_sg_unmap_capable(dev, sglist, nelems, attrs))
ppc_iommu_unmap_sg(get_iommu_table_base(dev), sglist, nelems,
direction, attrs);
else
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 99f72162dd85..702a680f5766 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1119,6 +1119,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
spin_unlock(&direct_window_list_lock);
dma_addr = be64_to_cpu(ddwprop->dma_base);
+ dev->dev.bus_dma_limit = dma_addr + query.largest_available_block;
goto out_unlock;
out_free_window:
^ permalink raw reply related
* Re: hardcoded SIGSEGV in __die() ?
From: Christophe Leroy @ 2020-03-23 15:31 UTC (permalink / raw)
To: Joakim Tjernlund, linuxppc-dev@ozlabs.org
In-Reply-To: <4f4f2c97f7393f21f507c58def88514c9f670e0a.camel@infinera.com>
Le 23/03/2020 à 16:08, Joakim Tjernlund a écrit :
> On Mon, 2020-03-23 at 15:45 +0100, Christophe Leroy wrote:
>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>
>>
>> Le 23/03/2020 à 15:43, Christophe Leroy a écrit :
>>>
>>> Le 23/03/2020 à 15:17, Joakim Tjernlund a écrit :
>>>> In __die(), see below, there is this call to notify_send() with
>>>> SIGSEGV hardcoded, this seems odd
>>>> to me as the variable "err" holds the true signal(in my case SIGBUS)
>>>> Should not SIGSEGV be replaced with the true signal no.?
>>>
>>> As far as I can see, comes from
>>> https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3D66fcb1059&data=02%7C01%7CJoakim.Tjernlund%40infinera.com%7C4291ac1b501e4296869a08d7cf38cdb4%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637205715189366995&sdata=Z2bFsmDlD2MKhLACQvayk9ejz0dqgMEOlBTlocAmtTg%3D&reserved=0
>>>
>>
>> And
>> https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3Dae87221d3ce49d9de1e43756da834fd0bf05a2ad&data=02%7C01%7CJoakim.Tjernlund%40infinera.com%7C4291ac1b501e4296869a08d7cf38cdb4%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637205715189366995&sdata=97kyz3Ur88BhDUUYzya5t%2FFQVhXYu6qiHoW8hsEg81s%3D&reserved=0
>> shows it is (was?) similar on x86.
>>
>
> I tried to follow that chain thinking it would end up sending a signal to user space but I cannot see
> that happens. Seems to be related to debugging.
>
> In short, I cannot see any signal being delivered to user space. If so that would explain why
> our user space process never dies.
> Is there a signal hidden in machine_check handler for SIGBUS I cannot see?
>
Isn't it done in do_exit(), called from oops_end() ?
Christophe
^ permalink raw reply
* [PATCH] completion: Use lockdep_assert_RT_in_threaded_ctx() in complete_all()
From: Sebastian Siewior @ 2020-03-23 15:20 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Randy Dunlap, linux-ia64, Peter Zijlstra, linux-pci,
Oleg Nesterov, Guo Ren, Joel Fernandes, Vincent Chen, Ingo Molnar,
Jonathan Corbet, Davidlohr Bueso, linux-acpi, Brian Cain,
Davidlohr Bueso, Paul E . McKenney, linux-hexagon,
Rafael J. Wysocki, linux-csky, Linus Torvalds, Darren Hart,
Zhang Rui, Len Brown, Fenghua Yu, Arnd Bergmann, linux-pm,
linuxppc-dev, Greentime Hu, Bjorn Helgaas, Kurt Schwemmer,
platform-driver-x86, Kalle Valo, kbuild test robot, Felipe Balbi,
Michal Simek, Tony Luck, Nick Hu, Geoff Levand, netdev, linux-usb,
linux-wireless, LKML, Greg Kroah-Hartman, Logan Gunthorpe,
David S. Miller, Andy Shevchenko
In-Reply-To: <20200321113242.317954042@linutronix.de>
The warning was intended to spot complete_all() users from hardirq
context on PREEMPT_RT. The warning as-is will also trigger in interrupt
handlers, which are threaded on PREEMPT_RT, which was not intended.
Use lockdep_assert_RT_in_threaded_ctx() which triggers in non-preemptive
context on PREEMPT_RT.
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Reported-by: kernel test robot <rong.a.chen@intel.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
include/linux/lockdep.h | 15 +++++++++++++++
kernel/sched/completion.c | 2 +-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 425b4ceb7cd07..206774ac69460 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -711,6 +711,21 @@ do { \
# define lockdep_assert_in_irq() do { } while (0)
#endif
+#ifdef CONFIG_PROVE_RAW_LOCK_NESTING
+
+# define lockdep_assert_RT_in_threaded_ctx() do { \
+ WARN_ONCE(debug_locks && !current->lockdep_recursion && \
+ current->hardirq_context && \
+ !(current->hardirq_threaded || current->irq_config), \
+ "Not in threaded context on PREEMPT_RT as expected\n"); \
+} while (0)
+
+#else
+
+# define lockdep_assert_RT_in_threaded_ctx() do { } while (0)
+
+#endif
+
#ifdef CONFIG_LOCKDEP
void lockdep_rcu_suspicious(const char *file, const int line, const char *s);
#else
diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c
index f15e96164ff1e..a778554f9dad7 100644
--- a/kernel/sched/completion.c
+++ b/kernel/sched/completion.c
@@ -58,7 +58,7 @@ void complete_all(struct completion *x)
{
unsigned long flags;
- WARN_ON(irqs_disabled());
+ lockdep_assert_RT_in_threaded_ctx();
raw_spin_lock_irqsave(&x->wait.lock, flags);
x->done = UINT_MAX;
--
2.26.0.rc2
^ permalink raw reply related
* Re: hardcoded SIGSEGV in __die() ?
From: Joakim Tjernlund @ 2020-03-23 15:08 UTC (permalink / raw)
To: christophe.leroy@c-s.fr, linuxppc-dev@ozlabs.org
In-Reply-To: <c14de482-6784-f1ac-f675-d771e55ac688@c-s.fr>
On Mon, 2020-03-23 at 15:45 +0100, Christophe Leroy wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> Le 23/03/2020 à 15:43, Christophe Leroy a écrit :
> >
> > Le 23/03/2020 à 15:17, Joakim Tjernlund a écrit :
> > > In __die(), see below, there is this call to notify_send() with
> > > SIGSEGV hardcoded, this seems odd
> > > to me as the variable "err" holds the true signal(in my case SIGBUS)
> > > Should not SIGSEGV be replaced with the true signal no.?
> >
> > As far as I can see, comes from
> > https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3D66fcb1059&data=02%7C01%7CJoakim.Tjernlund%40infinera.com%7C4291ac1b501e4296869a08d7cf38cdb4%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637205715189366995&sdata=Z2bFsmDlD2MKhLACQvayk9ejz0dqgMEOlBTlocAmtTg%3D&reserved=0
> >
>
> And
> https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3Dae87221d3ce49d9de1e43756da834fd0bf05a2ad&data=02%7C01%7CJoakim.Tjernlund%40infinera.com%7C4291ac1b501e4296869a08d7cf38cdb4%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637205715189366995&sdata=97kyz3Ur88BhDUUYzya5t%2FFQVhXYu6qiHoW8hsEg81s%3D&reserved=0
> shows it is (was?) similar on x86.
>
I tried to follow that chain thinking it would end up sending a signal to user space but I cannot see
that happens. Seems to be related to debugging.
In short, I cannot see any signal being delivered to user space. If so that would explain why
our user space process never dies.
Is there a signal hidden in machine_check handler for SIGBUS I cannot see?
Jocke
^ permalink raw reply
* Patch "mm, slub: prevent kmalloc_node crashes and memory leaks" has been added to the 5.4-stable tree
From: gregkh @ 2020-03-23 14:59 UTC (permalink / raw)
To: 088b5996-faae-8a56-ef9c-5b567125ae54, 20200317092624.GB22538,
20200320115533.9604-1-vbabka,
3381CD91-AB3D-4773-BA04-E7A072A63968, akpm, bharata, cl,
fff0e636-4c36-ed10-281c-8cdb0687c839, gregkh, iamjoonsoo.kim,
ktkhai, linuxppc-dev, mgorman, mhocko, mpe, nathanl, penberg,
puvichakravarthy, rientjes, sachinp, srikar, torvalds, vbabka
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
mm, slub: prevent kmalloc_node crashes and memory leaks
to the 5.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
and it can be found in the queue-5.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 0715e6c516f106ed553828a671d30ad9a3431536 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Sat, 21 Mar 2020 18:22:37 -0700
Subject: mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka <vbabka@suse.cz>
commit 0715e6c516f106ed553828a671d30ad9a3431536 upstream.
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP ___slab_alloc+0x1f4/0x760
LR __slab_alloc+0x34/0x60
Call Trace:
___slab_alloc+0x334/0x760 (unreliable)
__slab_alloc+0x34/0x60
__kmalloc_node+0x110/0x490
kvmalloc_node+0x58/0x110
mem_cgroup_css_online+0x108/0x270
online_css+0x48/0xd0
cgroup_apply_control_enable+0x2ec/0x4d0
cgroup_mkdir+0x228/0x5f0
kernfs_iop_mkdir+0x90/0xf0
vfs_mkdir+0x110/0x230
do_mkdirat+0xb0/0x1a0
system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate
shrinker_map on appropriate NUMA node" [2] which effectively calls
kmalloc_node for each possible node. SLUB however only allocates
kmem_cache_node on online N_NORMAL_MEMORY nodes, and relies on
node_to_mem_node to return such valid node for other nodes since commit
a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating
on memoryless node"). This is however not true in this configuration
where the _node_numa_mem_ array is not initialized for nodes 0 and 2-31,
thus it contains zeroes and get_partial() ends up accessing
non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3]
where a similar PowerPC configuration, but with mainline kernel without
patch [2] ends up allocating large amounts of pages by kmalloc-1k
kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also
lead to an infinite loop with CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node()
anymore and instead simply falling back to NUMA_NO_NODE, when
kmalloc_node(node) is attempted for a node that's not online, or has no
usable memory. The "usable memory" condition is also changed from
node_present_pages() to N_NORMAL_MEMORY node state, as that is exactly
the condition that SLUB uses to allocate kmem_cache_node structures.
The check in get_partial() is removed completely, as the checks in
___slab_alloc() are now sufficient to prevent get_partial() being
reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: <stable@vger.kernel.org>
Link: http://lkml.kernel.org/r/20200320115533.9604-1-vbabka@suse.cz
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1953,8 +1953,6 @@ static void *get_partial(struct kmem_cac
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2543,17 +2541,27 @@ static void *___slab_alloc(struct kmem_c
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
Patches currently in stable-queue which might be from vbabka@suse.cz are
queue-5.4/mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
queue-5.4/mm-do-not-allow-madv_pageout-for-cow-pages.patch
^ permalink raw reply
* Re: [PATCH v4 01/17] cpu: Add new {add,remove}_cpu() functions
From: Paul E. McKenney @ 2020-03-23 15:00 UTC (permalink / raw)
To: Qais Yousef
Cc: Juergen Gross, Mark Rutland, Lorenzo Pieralisi, linux-parisc,
Helge Deller, x86, linux-kernel, sparclinux, xen-devel,
Thomas Gleixner, linuxppc-dev, David S. Miller, linux-arm-kernel
In-Reply-To: <20200323135110.30522-2-qais.yousef@arm.com>
On Mon, Mar 23, 2020 at 01:50:54PM +0000, Qais Yousef wrote:
> The new functions use device_{online,offline}() which are userspace
> safe.
>
> This is in preparation to move cpu_{up, down} kernel users to use
> a safer interface that is not racy with userspace.
>
> Suggested-by: "Paul E. McKenney" <paulmck@kernel.org>
> Signed-off-by: Qais Yousef <qais.yousef@arm.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: "Paul E. McKenney" <paulmck@kernel.org>
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
> CC: Helge Deller <deller@gmx.de>
> CC: Michael Ellerman <mpe@ellerman.id.au>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Juergen Gross <jgross@suse.com>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> CC: xen-devel@lists.xenproject.org
> CC: linux-parisc@vger.kernel.org
> CC: sparclinux@vger.kernel.org
> CC: linuxppc-dev@lists.ozlabs.org
> CC: linux-arm-kernel@lists.infradead.org
> CC: x86@kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
> include/linux/cpu.h | 2 ++
> kernel/cpu.c | 24 ++++++++++++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 1ca2baf817ed..cf8cf38dca43 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -89,6 +89,7 @@ extern ssize_t arch_cpu_release(const char *, size_t);
> #ifdef CONFIG_SMP
> extern bool cpuhp_tasks_frozen;
> int cpu_up(unsigned int cpu);
> +int add_cpu(unsigned int cpu);
> void notify_cpu_starting(unsigned int cpu);
> extern void cpu_maps_update_begin(void);
> extern void cpu_maps_update_done(void);
> @@ -118,6 +119,7 @@ extern void cpu_hotplug_disable(void);
> extern void cpu_hotplug_enable(void);
> void clear_tasks_mm_cpumask(int cpu);
> int cpu_down(unsigned int cpu);
> +int remove_cpu(unsigned int cpu);
>
> #else /* CONFIG_HOTPLUG_CPU */
>
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 9c706af713fb..069802f7010f 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1057,6 +1057,18 @@ int cpu_down(unsigned int cpu)
> }
> EXPORT_SYMBOL(cpu_down);
>
> +int remove_cpu(unsigned int cpu)
> +{
> + int ret;
> +
> + lock_device_hotplug();
> + ret = device_offline(get_cpu_device(cpu));
> + unlock_device_hotplug();
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(remove_cpu);
> +
> #else
> #define takedown_cpu NULL
> #endif /*CONFIG_HOTPLUG_CPU*/
> @@ -1209,6 +1221,18 @@ int cpu_up(unsigned int cpu)
> }
> EXPORT_SYMBOL_GPL(cpu_up);
>
> +int add_cpu(unsigned int cpu)
> +{
> + int ret;
> +
> + lock_device_hotplug();
> + ret = device_online(get_cpu_device(cpu));
> + unlock_device_hotplug();
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(add_cpu);
> +
> #ifdef CONFIG_PM_SLEEP_SMP
> static cpumask_var_t frozen_cpus;
>
> --
> 2.17.1
>
^ permalink raw reply
* Patch "mm, slub: prevent kmalloc_node crashes and memory leaks" has been added to the 4.19-stable tree
From: gregkh @ 2020-03-23 14:58 UTC (permalink / raw)
To: 088b5996-faae-8a56-ef9c-5b567125ae54, 20200317092624.GB22538,
20200320115533.9604-1-vbabka,
3381CD91-AB3D-4773-BA04-E7A072A63968, akpm, bharata, cl,
fff0e636-4c36-ed10-281c-8cdb0687c839, gregkh, iamjoonsoo.kim,
ktkhai, linuxppc-dev, mgorman, mhocko, mpe, nathanl, penberg,
puvichakravarthy, rientjes, sachinp, srikar, torvalds, vbabka
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
mm, slub: prevent kmalloc_node crashes and memory leaks
to the 4.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
and it can be found in the queue-4.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 0715e6c516f106ed553828a671d30ad9a3431536 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Sat, 21 Mar 2020 18:22:37 -0700
Subject: mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka <vbabka@suse.cz>
commit 0715e6c516f106ed553828a671d30ad9a3431536 upstream.
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP ___slab_alloc+0x1f4/0x760
LR __slab_alloc+0x34/0x60
Call Trace:
___slab_alloc+0x334/0x760 (unreliable)
__slab_alloc+0x34/0x60
__kmalloc_node+0x110/0x490
kvmalloc_node+0x58/0x110
mem_cgroup_css_online+0x108/0x270
online_css+0x48/0xd0
cgroup_apply_control_enable+0x2ec/0x4d0
cgroup_mkdir+0x228/0x5f0
kernfs_iop_mkdir+0x90/0xf0
vfs_mkdir+0x110/0x230
do_mkdirat+0xb0/0x1a0
system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate
shrinker_map on appropriate NUMA node" [2] which effectively calls
kmalloc_node for each possible node. SLUB however only allocates
kmem_cache_node on online N_NORMAL_MEMORY nodes, and relies on
node_to_mem_node to return such valid node for other nodes since commit
a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating
on memoryless node"). This is however not true in this configuration
where the _node_numa_mem_ array is not initialized for nodes 0 and 2-31,
thus it contains zeroes and get_partial() ends up accessing
non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3]
where a similar PowerPC configuration, but with mainline kernel without
patch [2] ends up allocating large amounts of pages by kmalloc-1k
kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also
lead to an infinite loop with CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node()
anymore and instead simply falling back to NUMA_NO_NODE, when
kmalloc_node(node) is attempted for a node that's not online, or has no
usable memory. The "usable memory" condition is also changed from
node_present_pages() to N_NORMAL_MEMORY node state, as that is exactly
the condition that SLUB uses to allocate kmem_cache_node structures.
The check in get_partial() is removed completely, as the checks in
___slab_alloc() are now sufficient to prevent get_partial() being
reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: <stable@vger.kernel.org>
Link: http://lkml.kernel.org/r/20200320115533.9604-1-vbabka@suse.cz
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1906,8 +1906,6 @@ static void *get_partial(struct kmem_cac
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2504,17 +2502,27 @@ static void *___slab_alloc(struct kmem_c
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
Patches currently in stable-queue which might be from vbabka@suse.cz are
queue-4.19/mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
^ permalink raw reply
* Patch "mm, slub: prevent kmalloc_node crashes and memory leaks" has been added to the 4.14-stable tree
From: gregkh @ 2020-03-23 14:58 UTC (permalink / raw)
To: 088b5996-faae-8a56-ef9c-5b567125ae54, 20200317092624.GB22538,
20200320115533.9604-1-vbabka,
3381CD91-AB3D-4773-BA04-E7A072A63968, akpm, bharata, cl,
fff0e636-4c36-ed10-281c-8cdb0687c839, gregkh, iamjoonsoo.kim,
ktkhai, linuxppc-dev, mgorman, mhocko, mpe, nathanl, penberg,
puvichakravarthy, rientjes, sachinp, srikar, torvalds, vbabka
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
mm, slub: prevent kmalloc_node crashes and memory leaks
to the 4.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
and it can be found in the queue-4.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 0715e6c516f106ed553828a671d30ad9a3431536 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Sat, 21 Mar 2020 18:22:37 -0700
Subject: mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka <vbabka@suse.cz>
commit 0715e6c516f106ed553828a671d30ad9a3431536 upstream.
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP ___slab_alloc+0x1f4/0x760
LR __slab_alloc+0x34/0x60
Call Trace:
___slab_alloc+0x334/0x760 (unreliable)
__slab_alloc+0x34/0x60
__kmalloc_node+0x110/0x490
kvmalloc_node+0x58/0x110
mem_cgroup_css_online+0x108/0x270
online_css+0x48/0xd0
cgroup_apply_control_enable+0x2ec/0x4d0
cgroup_mkdir+0x228/0x5f0
kernfs_iop_mkdir+0x90/0xf0
vfs_mkdir+0x110/0x230
do_mkdirat+0xb0/0x1a0
system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate
shrinker_map on appropriate NUMA node" [2] which effectively calls
kmalloc_node for each possible node. SLUB however only allocates
kmem_cache_node on online N_NORMAL_MEMORY nodes, and relies on
node_to_mem_node to return such valid node for other nodes since commit
a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating
on memoryless node"). This is however not true in this configuration
where the _node_numa_mem_ array is not initialized for nodes 0 and 2-31,
thus it contains zeroes and get_partial() ends up accessing
non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3]
where a similar PowerPC configuration, but with mainline kernel without
patch [2] ends up allocating large amounts of pages by kmalloc-1k
kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also
lead to an infinite loop with CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node()
anymore and instead simply falling back to NUMA_NO_NODE, when
kmalloc_node(node) is attempted for a node that's not online, or has no
usable memory. The "usable memory" condition is also changed from
node_present_pages() to N_NORMAL_MEMORY node state, as that is exactly
the condition that SLUB uses to allocate kmem_cache_node structures.
The check in get_partial() is removed completely, as the checks in
___slab_alloc() are now sufficient to prevent get_partial() being
reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: <stable@vger.kernel.org>
Link: http://lkml.kernel.org/r/20200320115533.9604-1-vbabka@suse.cz
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1923,8 +1923,6 @@ static void *get_partial(struct kmem_cac
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2521,17 +2519,27 @@ static void *___slab_alloc(struct kmem_c
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
Patches currently in stable-queue which might be from vbabka@suse.cz are
queue-4.14/mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
^ permalink raw reply
* Patch "mm, slub: prevent kmalloc_node crashes and memory leaks" has been added to the 4.9-stable tree
From: gregkh @ 2020-03-23 14:57 UTC (permalink / raw)
To: 088b5996-faae-8a56-ef9c-5b567125ae54, 20200317092624.GB22538,
20200320115533.9604-1-vbabka,
3381CD91-AB3D-4773-BA04-E7A072A63968, akpm, bharata, cl,
fff0e636-4c36-ed10-281c-8cdb0687c839, gregkh, iamjoonsoo.kim,
ktkhai, linuxppc-dev, mgorman, mhocko, mpe, nathanl, penberg,
puvichakravarthy, rientjes, sachinp, srikar, torvalds, vbabka
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
mm, slub: prevent kmalloc_node crashes and memory leaks
to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 0715e6c516f106ed553828a671d30ad9a3431536 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Sat, 21 Mar 2020 18:22:37 -0700
Subject: mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka <vbabka@suse.cz>
commit 0715e6c516f106ed553828a671d30ad9a3431536 upstream.
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP ___slab_alloc+0x1f4/0x760
LR __slab_alloc+0x34/0x60
Call Trace:
___slab_alloc+0x334/0x760 (unreliable)
__slab_alloc+0x34/0x60
__kmalloc_node+0x110/0x490
kvmalloc_node+0x58/0x110
mem_cgroup_css_online+0x108/0x270
online_css+0x48/0xd0
cgroup_apply_control_enable+0x2ec/0x4d0
cgroup_mkdir+0x228/0x5f0
kernfs_iop_mkdir+0x90/0xf0
vfs_mkdir+0x110/0x230
do_mkdirat+0xb0/0x1a0
system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate
shrinker_map on appropriate NUMA node" [2] which effectively calls
kmalloc_node for each possible node. SLUB however only allocates
kmem_cache_node on online N_NORMAL_MEMORY nodes, and relies on
node_to_mem_node to return such valid node for other nodes since commit
a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating
on memoryless node"). This is however not true in this configuration
where the _node_numa_mem_ array is not initialized for nodes 0 and 2-31,
thus it contains zeroes and get_partial() ends up accessing
non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3]
where a similar PowerPC configuration, but with mainline kernel without
patch [2] ends up allocating large amounts of pages by kmalloc-1k
kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also
lead to an infinite loop with CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node()
anymore and instead simply falling back to NUMA_NO_NODE, when
kmalloc_node(node) is attempted for a node that's not online, or has no
usable memory. The "usable memory" condition is also changed from
node_present_pages() to N_NORMAL_MEMORY node state, as that is exactly
the condition that SLUB uses to allocate kmem_cache_node structures.
The check in get_partial() is removed completely, as the checks in
___slab_alloc() are now sufficient to prevent get_partial() being
reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: <stable@vger.kernel.org>
Link: http://lkml.kernel.org/r/20200320115533.9604-1-vbabka@suse.cz
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1909,8 +1909,6 @@ static void *get_partial(struct kmem_cac
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2506,17 +2504,27 @@ static void *___slab_alloc(struct kmem_c
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist);
c->page = NULL;
Patches currently in stable-queue which might be from vbabka@suse.cz are
queue-4.9/mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
^ permalink raw reply
* Patch "mm, slub: prevent kmalloc_node crashes and memory leaks" has been added to the 4.4-stable tree
From: gregkh @ 2020-03-23 14:57 UTC (permalink / raw)
To: 088b5996-faae-8a56-ef9c-5b567125ae54, 20200317092624.GB22538,
20200320115533.9604-1-vbabka,
3381CD91-AB3D-4773-BA04-E7A072A63968, akpm, bharata, cl,
fff0e636-4c36-ed10-281c-8cdb0687c839, gregkh, iamjoonsoo.kim,
ktkhai, linuxppc-dev, mgorman, mhocko, mpe, nathanl, penberg,
puvichakravarthy, rientjes, sachinp, srikar, torvalds, vbabka
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
mm, slub: prevent kmalloc_node crashes and memory leaks
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 0715e6c516f106ed553828a671d30ad9a3431536 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Sat, 21 Mar 2020 18:22:37 -0700
Subject: mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka <vbabka@suse.cz>
commit 0715e6c516f106ed553828a671d30ad9a3431536 upstream.
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP ___slab_alloc+0x1f4/0x760
LR __slab_alloc+0x34/0x60
Call Trace:
___slab_alloc+0x334/0x760 (unreliable)
__slab_alloc+0x34/0x60
__kmalloc_node+0x110/0x490
kvmalloc_node+0x58/0x110
mem_cgroup_css_online+0x108/0x270
online_css+0x48/0xd0
cgroup_apply_control_enable+0x2ec/0x4d0
cgroup_mkdir+0x228/0x5f0
kernfs_iop_mkdir+0x90/0xf0
vfs_mkdir+0x110/0x230
do_mkdirat+0xb0/0x1a0
system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate
shrinker_map on appropriate NUMA node" [2] which effectively calls
kmalloc_node for each possible node. SLUB however only allocates
kmem_cache_node on online N_NORMAL_MEMORY nodes, and relies on
node_to_mem_node to return such valid node for other nodes since commit
a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating
on memoryless node"). This is however not true in this configuration
where the _node_numa_mem_ array is not initialized for nodes 0 and 2-31,
thus it contains zeroes and get_partial() ends up accessing
non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3]
where a similar PowerPC configuration, but with mainline kernel without
patch [2] ends up allocating large amounts of pages by kmalloc-1k
kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also
lead to an infinite loop with CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node()
anymore and instead simply falling back to NUMA_NO_NODE, when
kmalloc_node(node) is attempted for a node that's not online, or has no
usable memory. The "usable memory" condition is also changed from
node_present_pages() to N_NORMAL_MEMORY node state, as that is exactly
the condition that SLUB uses to allocate kmem_cache_node structures.
The check in get_partial() is removed completely, as the checks in
___slab_alloc() are now sufficient to prevent get_partial() being
reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: <stable@vger.kernel.org>
Link: http://lkml.kernel.org/r/20200320115533.9604-1-vbabka@suse.cz
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1777,8 +1777,6 @@ static void *get_partial(struct kmem_cac
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2355,17 +2353,27 @@ static void *___slab_alloc(struct kmem_c
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist);
c->page = NULL;
Patches currently in stable-queue which might be from vbabka@suse.cz are
queue-4.4/mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
^ permalink raw reply
* Patch "mm, slub: prevent kmalloc_node crashes and memory leaks" has been added to the 5.5-stable tree
From: gregkh @ 2020-03-23 14:55 UTC (permalink / raw)
To: 088b5996-faae-8a56-ef9c-5b567125ae54, 20200317092624.GB22538,
20200320115533.9604-1-vbabka,
3381CD91-AB3D-4773-BA04-E7A072A63968, akpm, bharata, cl,
fff0e636-4c36-ed10-281c-8cdb0687c839, gregkh, iamjoonsoo.kim,
ktkhai, linuxppc-dev, mgorman, mhocko, mpe, nathanl, penberg,
puvichakravarthy, rientjes, sachinp, srikar, torvalds, vbabka
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
mm, slub: prevent kmalloc_node crashes and memory leaks
to the 5.5-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
and it can be found in the queue-5.5 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 0715e6c516f106ed553828a671d30ad9a3431536 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Sat, 21 Mar 2020 18:22:37 -0700
Subject: mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka <vbabka@suse.cz>
commit 0715e6c516f106ed553828a671d30ad9a3431536 upstream.
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP ___slab_alloc+0x1f4/0x760
LR __slab_alloc+0x34/0x60
Call Trace:
___slab_alloc+0x334/0x760 (unreliable)
__slab_alloc+0x34/0x60
__kmalloc_node+0x110/0x490
kvmalloc_node+0x58/0x110
mem_cgroup_css_online+0x108/0x270
online_css+0x48/0xd0
cgroup_apply_control_enable+0x2ec/0x4d0
cgroup_mkdir+0x228/0x5f0
kernfs_iop_mkdir+0x90/0xf0
vfs_mkdir+0x110/0x230
do_mkdirat+0xb0/0x1a0
system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate
shrinker_map on appropriate NUMA node" [2] which effectively calls
kmalloc_node for each possible node. SLUB however only allocates
kmem_cache_node on online N_NORMAL_MEMORY nodes, and relies on
node_to_mem_node to return such valid node for other nodes since commit
a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating
on memoryless node"). This is however not true in this configuration
where the _node_numa_mem_ array is not initialized for nodes 0 and 2-31,
thus it contains zeroes and get_partial() ends up accessing
non-allocated kmem_cache_node.
A related issue was reported by Bharata (originally by Ramachandran) [3]
where a similar PowerPC configuration, but with mainline kernel without
patch [2] ends up allocating large amounts of pages by kmalloc-1k
kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also
lead to an infinite loop with CONFIG_SLUB_CPU_PARTIAL [4].
This patch should fix both issues by not relying on node_to_mem_node()
anymore and instead simply falling back to NUMA_NO_NODE, when
kmalloc_node(node) is attempted for a node that's not online, or has no
usable memory. The "usable memory" condition is also changed from
node_present_pages() to N_NORMAL_MEMORY node state, as that is exactly
the condition that SLUB uses to allocate kmem_cache_node structures.
The check in get_partial() is removed completely, as the checks in
___slab_alloc() are now sufficient to prevent get_partial() being
reached with an invalid node.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Bharata B Rao <bharata@linux.ibm.com>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: <stable@vger.kernel.org>
Link: http://lkml.kernel.org/r/20200320115533.9604-1-vbabka@suse.cz
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
mm/slub.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1954,8 +1954,6 @@ static void *get_partial(struct kmem_cac
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2544,17 +2542,27 @@ static void *___slab_alloc(struct kmem_c
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no normal memory, just
+ * ignore the node constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ !node_state(node, N_NORMAL_MEMORY)))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if (!node_state(node, N_NORMAL_MEMORY)) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
Patches currently in stable-queue which might be from vbabka@suse.cz are
queue-5.5/mm-slub-prevent-kmalloc_node-crashes-and-memory-leaks.patch
queue-5.5/mm-do-not-allow-madv_pageout-for-cow-pages.patch
^ permalink raw reply
* Re: hardcoded SIGSEGV in __die() ?
From: Christophe Leroy @ 2020-03-23 14:45 UTC (permalink / raw)
To: Joakim Tjernlund, linuxppc-dev@ozlabs.org
In-Reply-To: <b20d978b-268b-773a-a43e-7ff4c741f2df@c-s.fr>
Le 23/03/2020 à 15:43, Christophe Leroy a écrit :
>
>
> Le 23/03/2020 à 15:17, Joakim Tjernlund a écrit :
>> In __die(), see below, there is this call to notify_send() with
>> SIGSEGV hardcoded, this seems odd
>> to me as the variable "err" holds the true signal(in my case SIGBUS)
>> Should not SIGSEGV be replaced with the true signal no.?
>
> As far as I can see, comes from
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=66fcb1059
>
And
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ae87221d3ce49d9de1e43756da834fd0bf05a2ad
shows it is (was?) similar on x86.
Christophe
^ permalink raw reply
* Re: hardcoded SIGSEGV in __die() ?
From: Christophe Leroy @ 2020-03-23 14:43 UTC (permalink / raw)
To: Joakim Tjernlund, linuxppc-dev@ozlabs.org
In-Reply-To: <73da05c0f54692a36471a2539dbd9b30594b687a.camel@infinera.com>
Le 23/03/2020 à 15:17, Joakim Tjernlund a écrit :
> In __die(), see below, there is this call to notify_send() with SIGSEGV hardcoded, this seems odd
> to me as the variable "err" holds the true signal(in my case SIGBUS)
> Should not SIGSEGV be replaced with the true signal no.?
As far as I can see, comes from
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=66fcb1059
Christophe
^ permalink raw reply
* hardcoded SIGSEGV in __die() ?
From: Joakim Tjernlund @ 2020-03-23 14:17 UTC (permalink / raw)
To: linuxppc-dev@ozlabs.org
In __die(), see below, there is this call to notify_send() with SIGSEGV hardcoded, this seems odd
to me as the variable "err" holds the true signal(in my case SIGBUS)
Should not SIGSEGV be replaced with the true signal no.?
Jocke
static int __die(const char *str, struct pt_regs *regs, long err)
{
printk("Oops: %s, sig: %ld [#%d]\n", str, err, ++die_counter);
if (IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN))
printk("LE ");
else
printk("BE ");
if (IS_ENABLED(CONFIG_PREEMPT))
pr_cont("PREEMPT ");
if (IS_ENABLED(CONFIG_SMP))
pr_cont("SMP NR_CPUS=%d ", NR_CPUS);
if (debug_pagealloc_enabled())
pr_cont("DEBUG_PAGEALLOC ");
if (IS_ENABLED(CONFIG_NUMA))
pr_cont("NUMA ");
pr_cont("%s\n", ppc_md.name ? ppc_md.name : "");
if (notify_die(DIE_OOPS, str, regs, err, 255, SIGSEGV) == NOTIFY_STOP)
return 1;
print_modules();
show_regs(regs);
return 0;
}
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox