From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: Add translation functions for /dev/mem read/write
Date: Fri, 5 May 2017 15:55:06 +0100 [thread overview]
Message-ID: <20170505145505.GF14111@arm.com> (raw)
In-Reply-To: <CAKv+Gu-z8_OeTEp-4DSLtfvirKHct8sD+uThz4wseuZgHBxyoA@mail.gmail.com>
On Thu, May 04, 2017 at 07:40:50AM +0100, Ard Biesheuvel wrote:
> On 3 May 2017 at 22:47, Goel, Sameer <sgoel@codeaurora.org> wrote:
> >
> >
> > On 5/3/2017 2:18 PM, Leif Lindholm wrote:
> >> On Wed, May 03, 2017 at 11:07:45AM -0600, Goel, Sameer wrote:
> >>> On 5/3/2017 5:26 AM, Will Deacon wrote:
> >>>> [adding some /dev/mem fans to cc]
> >>>>
> >>>> On Tue, May 02, 2017 at 02:28:05PM -0600, Sameer Goel wrote:
> >>>>> Port architecture specific xlate and unxlate functions for /dev/mem
> >>>>> read/write. This sets up the mapping for a valid physical address if a
> >>>>> kernel direct mapping is not already present.
> >>>>>
> >>>>> This is a generic issue as a user space app should not be allowed to crash
> >>>>> the kernel.
> >>>>
> >>>>> This issue was observed when systemd tried to access performance
> >>>>> pointer record from the FPDT table.
> >>>>
> >>>> Why is it doing that? Is there not a way to get this via /sys?
> >>>
> >>> There is no ACPI FPDT implementation in the kernel, so the userspace
> >>> systemd code is getting the FPDT table contents from /sys
> >>> and parsing the entries. The performance pointer record is a
> >>> reserved address populated by UEFI and the userspace code tries to
> >>> access it using /dev/mem. The physical address is valid, so cannot
> >>> push back on the user space code.
> >>
> >> OK, so then we need to add support for parsing this table in the
> >> kernel and exposing the referred-to regions in a controllable fashion.
> >> Maybe something that belongs under /sys/firmware/efi (adding Matt), or
> >> maybe something that deserves its own driver.
> >>
> >> The only two use-cases for /dev/mem on arm64 are:
> >> - Implementing interfaces in the kernel takes up-front effort.
> >> - Being able to accidentally panic the kernel from userland.
> >>
> > We will see this issue with any access using /dev/mem to a MEMBLOCK_NOMAP marked
> > memblock. The kernel crash issue has to be fixed irrespective of ACPI FPDT support.
> >
>
> I reported the same issue a couple of weeks ago [0]. So while we all
> agree that such accesses shouldn't oops the kernel, I think we may
> disagree on whether such accesses should be allowed in the first
> place, especially when using read/write on /dev/mem (as opposed to
> mmap())
Did you plan to respin those patches to address Alex's comments? I agree
that it would be good to close the oops, regardless of the rest of the
discussion here.
Will
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "Goel, Sameer" <sgoel@codeaurora.org>,
Leif Lindholm <leif.lindholm@linaro.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Shanker Donthineni <shankerd@codeaurora.org>,
Mark Rutland <mark.rutland@arm.com>,
Matt Fleming <matt@codeblueprint.co.uk>
Subject: Re: [PATCH] arm64: Add translation functions for /dev/mem read/write
Date: Fri, 5 May 2017 15:55:06 +0100 [thread overview]
Message-ID: <20170505145505.GF14111@arm.com> (raw)
In-Reply-To: <CAKv+Gu-z8_OeTEp-4DSLtfvirKHct8sD+uThz4wseuZgHBxyoA@mail.gmail.com>
On Thu, May 04, 2017 at 07:40:50AM +0100, Ard Biesheuvel wrote:
> On 3 May 2017 at 22:47, Goel, Sameer <sgoel@codeaurora.org> wrote:
> >
> >
> > On 5/3/2017 2:18 PM, Leif Lindholm wrote:
> >> On Wed, May 03, 2017 at 11:07:45AM -0600, Goel, Sameer wrote:
> >>> On 5/3/2017 5:26 AM, Will Deacon wrote:
> >>>> [adding some /dev/mem fans to cc]
> >>>>
> >>>> On Tue, May 02, 2017 at 02:28:05PM -0600, Sameer Goel wrote:
> >>>>> Port architecture specific xlate and unxlate functions for /dev/mem
> >>>>> read/write. This sets up the mapping for a valid physical address if a
> >>>>> kernel direct mapping is not already present.
> >>>>>
> >>>>> This is a generic issue as a user space app should not be allowed to crash
> >>>>> the kernel.
> >>>>
> >>>>> This issue was observed when systemd tried to access performance
> >>>>> pointer record from the FPDT table.
> >>>>
> >>>> Why is it doing that? Is there not a way to get this via /sys?
> >>>
> >>> There is no ACPI FPDT implementation in the kernel, so the userspace
> >>> systemd code is getting the FPDT table contents from /sys
> >>> and parsing the entries. The performance pointer record is a
> >>> reserved address populated by UEFI and the userspace code tries to
> >>> access it using /dev/mem. The physical address is valid, so cannot
> >>> push back on the user space code.
> >>
> >> OK, so then we need to add support for parsing this table in the
> >> kernel and exposing the referred-to regions in a controllable fashion.
> >> Maybe something that belongs under /sys/firmware/efi (adding Matt), or
> >> maybe something that deserves its own driver.
> >>
> >> The only two use-cases for /dev/mem on arm64 are:
> >> - Implementing interfaces in the kernel takes up-front effort.
> >> - Being able to accidentally panic the kernel from userland.
> >>
> > We will see this issue with any access using /dev/mem to a MEMBLOCK_NOMAP marked
> > memblock. The kernel crash issue has to be fixed irrespective of ACPI FPDT support.
> >
>
> I reported the same issue a couple of weeks ago [0]. So while we all
> agree that such accesses shouldn't oops the kernel, I think we may
> disagree on whether such accesses should be allowed in the first
> place, especially when using read/write on /dev/mem (as opposed to
> mmap())
Did you plan to respin those patches to address Alex's comments? I agree
that it would be good to close the oops, regardless of the rest of the
discussion here.
Will
next prev parent reply other threads:[~2017-05-05 14:55 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-02 20:28 [PATCH] arm64: Add translation functions for /dev/mem read/write Sameer Goel
2017-05-02 20:28 ` Sameer Goel
2017-05-03 11:26 ` Will Deacon
2017-05-03 11:26 ` Will Deacon
2017-05-03 17:07 ` Goel, Sameer
2017-05-03 17:07 ` Goel, Sameer
2017-05-03 20:18 ` Leif Lindholm
2017-05-03 20:18 ` Leif Lindholm
2017-05-03 21:47 ` Goel, Sameer
2017-05-03 21:47 ` Goel, Sameer
2017-05-04 6:40 ` Ard Biesheuvel
2017-05-04 6:40 ` Ard Biesheuvel
2017-05-04 19:58 ` Goel, Sameer
2017-05-04 19:58 ` Goel, Sameer
2017-05-05 14:55 ` Will Deacon [this message]
2017-05-05 14:55 ` Will Deacon
2017-05-05 16:25 ` Ard Biesheuvel
2017-05-05 16:25 ` Ard Biesheuvel
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=20170505145505.GF14111@arm.com \
--to=will.deacon@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.