From: Konrad Rzeszutek Wilk <konrad@darnok.org>
To: Eric Yang <yu.yang_3@nxp.com>
Cc: Robin Murphy <robin.murphy@arm.com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
"iommu@lists.linux-foundation.org"
<iommu@lists.linux-foundation.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Kees Cook <keescook@chromium.org>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Ingo Molnar <mingo@kernel.org>, Al Viro <viro@zeniv.linux.org.uk>,
Andrey Ryabinin <aryabinin@virtuozzo.com>,
Andrew Morton <akpm@linux-foundation.org>,
David Miller <davem@davemloft.net>
Subject: Re: No check of the size passed to unmap_single in swiotlb
Date: Fri, 24 Nov 2017 08:33:43 -0500 [thread overview]
Message-ID: <20171124133341.GB16160@localhost.localdomain> (raw)
In-Reply-To: <VI1PR04MB30713D88BF08BE52B2C85D458C210@VI1PR04MB3071.eurprd04.prod.outlook.com>
..snip..
> > There doesn't seem to be much good reason for SWIOTLB to be more special
> > than other DMA API backends, and not all of them have enough internal state to
> > be able to make such a check. It's also not necessarily possible to "prevent
> > damage" anyway - if a driver does pass a bogus size for dma_unmap_single(...,
> > DMA_FROM_DEVICE), SWIOTLB might be able to keep itself internally consistent,
> > but it still can't prevent the arch code in the middle from invalidating the wrong
> > cache lines and potentially corrupting adjacent memory.
> >
> > In short, trying to work around broken drivers is a much worse idea than just
> > fixing those drivers, and that's what we already have dma-debug for.
> >
> > Robin.
>
> Hi Robin,
>
> I agree that hacking kernel to fix broken drivers is not acceptable, actually we spent days to fight driver supplier with this, they do not want to change their code and want fix it directly in kernel.
You could upstream the driver? That way it will be fixed.
I am not sure if you are aware but there is a staging directory
as well in Linux.
>
> I tried Dma-debug yesterday, it works very well, but I think only the size mismatch check may not be enough for the map entry corrupt situation, some run-time warning may be better when the real corruption happen.
>
> For most of the dma-api backend, the size mismatch may do no harm at all, and even in SWIOTLB itself when the bounce buffer is not used, the size mismatch do no harm either. In our case, the same buggy driver works well when board has 2G DDR, but panic frequently in 4G DDR because of the use of bounce buffer and these corrupted map entries. it is hard to catch this kind of bugs, for when the corruption happen, the kernel has all kind of reasons to panic, but not even one may directly point to the real source.
>
> Add the warning messages is a big convenience for figure this kind of issues, at least to me and the AP driver supplier, such warnings may save weeks of hard debug time.
I would prefer that all of those warning messages go in the DMA debug
API - as you can have multiple DMA different backends.
In other words, instead of being in one implementation it would make
much more sense to be in the generic API layer.
next prev parent reply other threads:[~2017-11-24 13:33 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <VI1PR04MB30715EA7212DC661481041438C220@VI1PR04MB3071.eurprd04.prod.outlook.com>
[not found] ` <VI1PR04MB30715EA7212DC661481041438C220-mr6QIVyDiCFHIX2o+zdTns9NdZoXdze2vxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-11-20 16:26 ` No check of the size passed to unmap_single in swiotlb Konrad Rzeszutek Wilk
[not found] ` <20171120162652.GR24312-he5eyhs8q0BAdwtm4QZOy9BPR1lH4CV8@public.gmane.org>
2017-11-20 16:50 ` Robin Murphy
[not found] ` <bb5a338c-4fd1-dbc4-e2be-663df0887504-5wv7dgnIgG8@public.gmane.org>
2017-11-23 9:08 ` Eric Yang
2017-11-24 13:33 ` Konrad Rzeszutek Wilk [this message]
[not found] ` <VI1PR04MB30713D88BF08BE52B2C85D458C210-mr6QIVyDiCFHIX2o+zdTns9NdZoXdze2vxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-11-28 14:18 ` Robin Murphy
[not found] ` <c372988a-e7cb-fe47-aec4-538c8808c3be-5wv7dgnIgG8@public.gmane.org>
2017-11-30 3:05 ` Eric Yang
2017-11-22 3:23 ` Eric Yang
[not found] ` <VI1PR04MB3071FBA364C78317F3C3A25D8C200-mr6QIVyDiCFHIX2o+zdTns9NdZoXdze2vxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-11-22 13:15 ` Robin Murphy
[not found] ` <c678cf02-700d-48c0-4160-8ee248a26da5-5wv7dgnIgG8@public.gmane.org>
2017-11-23 7:47 ` Eric Yang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171124133341.GB16160@localhost.localdomain \
--to=konrad@darnok.org \
--cc=akpm@linux-foundation.org \
--cc=aryabinin@virtuozzo.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=geert+renesas@glider.be \
--cc=gregkh@linuxfoundation.org \
--cc=iommu@lists.linux-foundation.org \
--cc=keescook@chromium.org \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=robin.murphy@arm.com \
--cc=viro@zeniv.linux.org.uk \
--cc=yu.yang_3@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox