usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Stefan Wahren]

Hi,

i want to remind about an issue which was originally reported by Wayne
Piekarski [1]. I'm able to reproduce this oops with Mainline Linux 5.0.2
on a Raspberry Pi 3B+ (arm64/defconfig) and according to Jan Kratochvil
[2] this applies to 5.1.0 and 5.2.0.

The crash is reproducible since commit c55191e96ca ("arm64: mm: apply
r/o permissions of VM areas to its linear alias as well"), but the root
cause of the crash was introduced much earlier with commit 56406e017a88
("usb: dwc2: Fix DMA alignment to start at allocated boundary").

I tested successfully the following workarounds with the RPi 3B+:

1) Disable RODATA_FULL_DEFAULT_ENABLED

2) revert commit 56406e017a88 ("usb: dwc2: Fix DMA alignment to start at
allocated boundary")

It would be nice if someone can come up with a proper solution.

Regards
Stefan

[1] - https://marc.info/?l=linux-usb&m=155440243702650&w=2
[2] - https://bugzilla.kernel.org/show_bug.cgi?id=203149

usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Stefan Wahren]

Hi,

i want to remind about an issue which was originally reported by Wayne
Piekarski [1]. I'm able to reproduce this oops with Mainline Linux 5.0.2
on a Raspberry Pi 3B+ (arm64/defconfig) and according to Jan Kratochvil
[2] this applies to 5.1.0 and 5.2.0.

The crash is reproducible since commit c55191e96ca ("arm64: mm: apply
r/o permissions of VM areas to its linear alias as well"), but the root
cause of the crash was introduced much earlier with commit 56406e017a88
("usb: dwc2: Fix DMA alignment to start at allocated boundary").

I tested successfully the following workarounds with the RPi 3B+:

1) Disable RODATA_FULL_DEFAULT_ENABLED

2) revert commit 56406e017a88 ("usb: dwc2: Fix DMA alignment to start at
allocated boundary")

It would be nice if someone can come up with a proper solution.

Regards
Stefan

[1] - https://marc.info/?l=linux-usb&m=155440243702650&w=2
[2] - https://bugzilla.kernel.org/show_bug.cgi?id=203149


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Antti Seppälä]

On Sun, 26 May 2019 at 13:11, Stefan Wahren <wahrenst@gmx.net> wrote:
>
> Hi,
>
> i want to remind about an issue which was originally reported by Wayne
> Piekarski [1]. I'm able to reproduce this oops with Mainline Linux 5.0.2
> on a Raspberry Pi 3B+ (arm64/defconfig) and according to Jan Kratochvil
> [2] this applies to 5.1.0 and 5.2.0.
>
> The crash is reproducible since commit c55191e96ca ("arm64: mm: apply
> r/o permissions of VM areas to its linear alias as well"), but the root
> cause of the crash was introduced much earlier with commit 56406e017a88
> ("usb: dwc2: Fix DMA alignment to start at allocated boundary").
>
> I tested successfully the following workarounds with the RPi 3B+:
>
> 1) Disable RODATA_FULL_DEFAULT_ENABLED
>
> 2) revert commit 56406e017a88 ("usb: dwc2: Fix DMA alignment to start at
> allocated boundary")
>
> It would be nice if someone can come up with a proper solution.
>
> Regards
> Stefan
>
> [1] - https://marc.info/?l=linux-usb&m=155440243702650&w=2
> [2] - https://bugzilla.kernel.org/show_bug.cgi?id=203149
>

Hello.

This is just a shot in the dark but have you tried to apply DMA cache
alignment issue fix [1] as a third workaround alternative?
If it helps the fix should be merged upstream.

[1] - https://patchwork.kernel.org/patch/10817377/

Br,
-- 
Antti

Re: usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Antti Seppälä]

On Sun, 26 May 2019 at 13:11, Stefan Wahren <wahrenst@gmx.net> wrote:
>
> Hi,
>
> i want to remind about an issue which was originally reported by Wayne
> Piekarski [1]. I'm able to reproduce this oops with Mainline Linux 5.0.2
> on a Raspberry Pi 3B+ (arm64/defconfig) and according to Jan Kratochvil
> [2] this applies to 5.1.0 and 5.2.0.
>
> The crash is reproducible since commit c55191e96ca ("arm64: mm: apply
> r/o permissions of VM areas to its linear alias as well"), but the root
> cause of the crash was introduced much earlier with commit 56406e017a88
> ("usb: dwc2: Fix DMA alignment to start at allocated boundary").
>
> I tested successfully the following workarounds with the RPi 3B+:
>
> 1) Disable RODATA_FULL_DEFAULT_ENABLED
>
> 2) revert commit 56406e017a88 ("usb: dwc2: Fix DMA alignment to start at
> allocated boundary")
>
> It would be nice if someone can come up with a proper solution.
>
> Regards
> Stefan
>
> [1] - https://marc.info/?l=linux-usb&m=155440243702650&w=2
> [2] - https://bugzilla.kernel.org/show_bug.cgi?id=203149
>

Hello.

This is just a shot in the dark but have you tried to apply DMA cache
alignment issue fix [1] as a third workaround alternative?
If it helps the fix should be merged upstream.

[1] - https://patchwork.kernel.org/patch/10817377/

Br,
-- 
Antti

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Ard Biesheuvel]

On Sun, 26 May 2019 at 12:45, Antti Seppälä <a.seppala@gmail.com> wrote:
>
> On Sun, 26 May 2019 at 13:11, Stefan Wahren <wahrenst@gmx.net> wrote:
> >
> > Hi,
> >
> > i want to remind about an issue which was originally reported by Wayne
> > Piekarski [1]. I'm able to reproduce this oops with Mainline Linux 5.0.2
> > on a Raspberry Pi 3B+ (arm64/defconfig) and according to Jan Kratochvil
> > [2] this applies to 5.1.0 and 5.2.0.
> >
> > The crash is reproducible since commit c55191e96ca ("arm64: mm: apply
> > r/o permissions of VM areas to its linear alias as well"), but the root
> > cause of the crash was introduced much earlier with commit 56406e017a88
> > ("usb: dwc2: Fix DMA alignment to start at allocated boundary").
> >
> > I tested successfully the following workarounds with the RPi 3B+:
> >
> > 1) Disable RODATA_FULL_DEFAULT_ENABLED
> >
> > 2) revert commit 56406e017a88 ("usb: dwc2: Fix DMA alignment to start at
> > allocated boundary")
> >
> > It would be nice if someone can come up with a proper solution.
> >
> > Regards
> > Stefan
> >
> > [1] - https://marc.info/?l=linux-usb&m=155440243702650&w=2
> > [2] - https://bugzilla.kernel.org/show_bug.cgi?id=203149
> >
>
> Hello.
>
> This is just a shot in the dark but have you tried to apply DMA cache
> alignment issue fix [1] as a third workaround alternative?
> If it helps the fix should be merged upstream.
>

Is transfer_buffer_length guaranteed to be a multiple of the max
D-cache line size in the system? If not, you will need to explicitly
flush the cache in dwc2_alloc_aligned_buffer() after memcpy()ing the
original buffer address into the newly allocated buffer, or subsequent
cache invalidation done in the DMA layer may wipe it.

Alternatively, you can round up transfer_buffer_length to be a
multiple of L1_CACHE_BYTES, e.g.,

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index b50ec3714fd8..84d43435d86e 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -2480,8 +2480,9 @@ static void dwc2_free_dma_aligned_buffer(struct urb *urb)
                return;

        /* Restore urb->transfer_buffer from the end of the allocated area */
-       memcpy(&stored_xfer_buffer, urb->transfer_buffer +
-              urb->transfer_buffer_length, sizeof(urb->transfer_buffer));
+       memcpy(&stored_xfer_buffer,
+              urb->transfer_buffer +
L1_CACHE_ALIGN(urb->transfer_buffer_length),
+              sizeof(urb->transfer_buffer));

        if (usb_urb_dir_in(urb)) {
                if (usb_pipeisoc(urb->pipe))
@@ -2512,7 +2515,7 @@ static int dwc2_alloc_dma_aligned_buffer(struct
urb *urb, gfp_t mem_flags)
         * pointer. This allocation is guaranteed to be aligned properly for
         * DMA
         */
-       kmalloc_size = urb->transfer_buffer_length +
+       kmalloc_size = L1_CACHE_ALIGN(urb->transfer_buffer_length) +
                sizeof(urb->transfer_buffer);

        kmalloc_ptr = kmalloc(kmalloc_size, mem_flags);
@@ -2523,7 +2526,7 @@ static int dwc2_alloc_dma_aligned_buffer(struct
urb *urb, gfp_t mem_flags)
         * Position value of original urb->transfer_buffer pointer to the end
         * of allocation for later referencing
         */
-       memcpy(kmalloc_ptr + urb->transfer_buffer_length,
+       memcpy(kmalloc_ptr + L1_CACHE_ALIGN(urb->transfer_buffer_length),
               &urb->transfer_buffer, sizeof(urb->transfer_buffer));

        if (usb_urb_dir_out(urb))

Re: usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Ard Biesheuvel]

On Sun, 26 May 2019 at 12:45, Antti Seppälä <a.seppala@gmail.com> wrote:
>
> On Sun, 26 May 2019 at 13:11, Stefan Wahren <wahrenst@gmx.net> wrote:
> >
> > Hi,
> >
> > i want to remind about an issue which was originally reported by Wayne
> > Piekarski [1]. I'm able to reproduce this oops with Mainline Linux 5.0.2
> > on a Raspberry Pi 3B+ (arm64/defconfig) and according to Jan Kratochvil
> > [2] this applies to 5.1.0 and 5.2.0.
> >
> > The crash is reproducible since commit c55191e96ca ("arm64: mm: apply
> > r/o permissions of VM areas to its linear alias as well"), but the root
> > cause of the crash was introduced much earlier with commit 56406e017a88
> > ("usb: dwc2: Fix DMA alignment to start at allocated boundary").
> >
> > I tested successfully the following workarounds with the RPi 3B+:
> >
> > 1) Disable RODATA_FULL_DEFAULT_ENABLED
> >
> > 2) revert commit 56406e017a88 ("usb: dwc2: Fix DMA alignment to start at
> > allocated boundary")
> >
> > It would be nice if someone can come up with a proper solution.
> >
> > Regards
> > Stefan
> >
> > [1] - https://marc.info/?l=linux-usb&m=155440243702650&w=2
> > [2] - https://bugzilla.kernel.org/show_bug.cgi?id=203149
> >
>
> Hello.
>
> This is just a shot in the dark but have you tried to apply DMA cache
> alignment issue fix [1] as a third workaround alternative?
> If it helps the fix should be merged upstream.
>

Is transfer_buffer_length guaranteed to be a multiple of the max
D-cache line size in the system? If not, you will need to explicitly
flush the cache in dwc2_alloc_aligned_buffer() after memcpy()ing the
original buffer address into the newly allocated buffer, or subsequent
cache invalidation done in the DMA layer may wipe it.

Alternatively, you can round up transfer_buffer_length to be a
multiple of L1_CACHE_BYTES, e.g.,

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index b50ec3714fd8..84d43435d86e 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -2480,8 +2480,9 @@ static void dwc2_free_dma_aligned_buffer(struct urb *urb)
                return;

        /* Restore urb->transfer_buffer from the end of the allocated area */
-       memcpy(&stored_xfer_buffer, urb->transfer_buffer +
-              urb->transfer_buffer_length, sizeof(urb->transfer_buffer));
+       memcpy(&stored_xfer_buffer,
+              urb->transfer_buffer +
L1_CACHE_ALIGN(urb->transfer_buffer_length),
+              sizeof(urb->transfer_buffer));

        if (usb_urb_dir_in(urb)) {
                if (usb_pipeisoc(urb->pipe))
@@ -2512,7 +2515,7 @@ static int dwc2_alloc_dma_aligned_buffer(struct
urb *urb, gfp_t mem_flags)
         * pointer. This allocation is guaranteed to be aligned properly for
         * DMA
         */
-       kmalloc_size = urb->transfer_buffer_length +
+       kmalloc_size = L1_CACHE_ALIGN(urb->transfer_buffer_length) +
                sizeof(urb->transfer_buffer);

        kmalloc_ptr = kmalloc(kmalloc_size, mem_flags);
@@ -2523,7 +2526,7 @@ static int dwc2_alloc_dma_aligned_buffer(struct
urb *urb, gfp_t mem_flags)
         * Position value of original urb->transfer_buffer pointer to the end
         * of allocation for later referencing
         */
-       memcpy(kmalloc_ptr + urb->transfer_buffer_length,
+       memcpy(kmalloc_ptr + L1_CACHE_ALIGN(urb->transfer_buffer_length),
               &urb->transfer_buffer, sizeof(urb->transfer_buffer));

        if (usb_urb_dir_out(urb))

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Ard Biesheuvel]

On Sun, 26 May 2019 at 14:58, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Sun, 26 May 2019 at 12:45, Antti Seppälä <a.seppala@gmail.com> wrote:
> >
> > On Sun, 26 May 2019 at 13:11, Stefan Wahren <wahrenst@gmx.net> wrote:
> > >
> > > Hi,
> > >
> > > i want to remind about an issue which was originally reported by Wayne
> > > Piekarski [1]. I'm able to reproduce this oops with Mainline Linux 5.0.2
> > > on a Raspberry Pi 3B+ (arm64/defconfig) and according to Jan Kratochvil
> > > [2] this applies to 5.1.0 and 5.2.0.
> > >
> > > The crash is reproducible since commit c55191e96ca ("arm64: mm: apply
> > > r/o permissions of VM areas to its linear alias as well"), but the root
> > > cause of the crash was introduced much earlier with commit 56406e017a88
> > > ("usb: dwc2: Fix DMA alignment to start at allocated boundary").
> > >
> > > I tested successfully the following workarounds with the RPi 3B+:
> > >
> > > 1) Disable RODATA_FULL_DEFAULT_ENABLED
> > >
> > > 2) revert commit 56406e017a88 ("usb: dwc2: Fix DMA alignment to start at
> > > allocated boundary")
> > >
> > > It would be nice if someone can come up with a proper solution.
> > >
> > > Regards
> > > Stefan
> > >
> > > [1] - https://marc.info/?l=linux-usb&m=155440243702650&w=2
> > > [2] - https://bugzilla.kernel.org/show_bug.cgi?id=203149
> > >
> >
> > Hello.
> >
> > This is just a shot in the dark but have you tried to apply DMA cache
> > alignment issue fix [1] as a third workaround alternative?
> > If it helps the fix should be merged upstream.
> >
>

Apologies, I should have looked at the patch first :-)

It does exactly what I proposed - ensure that DMA related cache
invalidation does not wipe the stored_xfer_buffer pointer.

sorry for the noise

Re: usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Ard Biesheuvel]

On Sun, 26 May 2019 at 14:58, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Sun, 26 May 2019 at 12:45, Antti Seppälä <a.seppala@gmail.com> wrote:
> >
> > On Sun, 26 May 2019 at 13:11, Stefan Wahren <wahrenst@gmx.net> wrote:
> > >
> > > Hi,
> > >
> > > i want to remind about an issue which was originally reported by Wayne
> > > Piekarski [1]. I'm able to reproduce this oops with Mainline Linux 5.0.2
> > > on a Raspberry Pi 3B+ (arm64/defconfig) and according to Jan Kratochvil
> > > [2] this applies to 5.1.0 and 5.2.0.
> > >
> > > The crash is reproducible since commit c55191e96ca ("arm64: mm: apply
> > > r/o permissions of VM areas to its linear alias as well"), but the root
> > > cause of the crash was introduced much earlier with commit 56406e017a88
> > > ("usb: dwc2: Fix DMA alignment to start at allocated boundary").
> > >
> > > I tested successfully the following workarounds with the RPi 3B+:
> > >
> > > 1) Disable RODATA_FULL_DEFAULT_ENABLED
> > >
> > > 2) revert commit 56406e017a88 ("usb: dwc2: Fix DMA alignment to start at
> > > allocated boundary")
> > >
> > > It would be nice if someone can come up with a proper solution.
> > >
> > > Regards
> > > Stefan
> > >
> > > [1] - https://marc.info/?l=linux-usb&m=155440243702650&w=2
> > > [2] - https://bugzilla.kernel.org/show_bug.cgi?id=203149
> > >
> >
> > Hello.
> >
> > This is just a shot in the dark but have you tried to apply DMA cache
> > alignment issue fix [1] as a third workaround alternative?
> > If it helps the fix should be merged upstream.
> >
>

Apologies, I should have looked at the patch first :-)

It does exactly what I proposed - ensure that DMA related cache
invalidation does not wipe the stored_xfer_buffer pointer.

sorry for the noise

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Stefan Wahren]

Hi,

Am 26.05.19 um 12:44 schrieb Antti Seppälä:
>
> Hello.
>
> This is just a shot in the dark but have you tried to apply DMA cache
> alignment issue fix [1] as a third workaround alternative?
> If it helps the fix should be merged upstream.

yes. After applying Martin's patch, i wasn't able to reproduce this
kernel oops.

Thanks to all

>
> [1] - https://patchwork.kernel.org/patch/10817377/
>
> Br,

Re: usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Stefan Wahren]

Hi,

Am 26.05.19 um 12:44 schrieb Antti Seppälä:
>
> Hello.
>
> This is just a shot in the dark but have you tried to apply DMA cache
> alignment issue fix [1] as a third workaround alternative?
> If it helps the fix should be merged upstream.

yes. After applying Martin's patch, i wasn't able to reproduce this
kernel oops.

Thanks to all

>
> [1] - https://patchwork.kernel.org/patch/10817377/
>
> Br,

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Stefan Wahren]

Hi Minas,
hi Felipe,

Am 26.05.19 um 21:58 schrieb Stefan Wahren:
> Hi,
>
> Am 26.05.19 um 12:44 schrieb Antti Seppälä:
>> Hello.
>>
>> This is just a shot in the dark but have you tried to apply DMA cache
>> alignment issue fix [1] as a third workaround alternative?
>> If it helps the fix should be merged upstream.
> yes. After applying Martin's patch, i wasn't able to reproduce this
> kernel oops.
>
> Thanks to all
>
>> [1] - https://patchwork.kernel.org/patch/10817377/
do we need a resend of this patch? Is there reason why it hasn't been
applied?
>>
>> Br,

Re: usb: dwc2: RODATA_FULL_DEFAULT_ENABLED causes kernel oops

[Stefan Wahren]

Hi Minas,
hi Felipe,

Am 26.05.19 um 21:58 schrieb Stefan Wahren:
> Hi,
>
> Am 26.05.19 um 12:44 schrieb Antti Seppälä:
>> Hello.
>>
>> This is just a shot in the dark but have you tried to apply DMA cache
>> alignment issue fix [1] as a third workaround alternative?
>> If it helps the fix should be merged upstream.
> yes. After applying Martin's patch, i wasn't able to reproduce this
> kernel oops.
>
> Thanks to all
>
>> [1] - https://patchwork.kernel.org/patch/10817377/
do we need a resend of this patch? Is there reason why it hasn't been
applied?
>>
>> Br,

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel