* Re: [PATCH v10 0/5] Kernel parameter parser cleanup/enhancement
From: Andrew Morton @ 2018-06-19 23:36 UTC (permalink / raw)
To: Michal Suchanek
Cc: Jonathan Corbet, Arnd Bergmann, Frederic Weisbecker, Ingo Molnar,
Aaron Wu, Tony Luck, Thomas Gleixner, Steven Rostedt,,
Laura Abbott, Dominik Brodowski, Alexey Dobriyan, Tom Lendacky,
Jeffrey Hugo, Baoquan He, Ilya Matveychikov, linux-doc,
linux-kernel
In-Reply-To: <cover.1528215659.git.msuchanek@suse.de>
On Tue, 5 Jun 2018 18:43:07 +0200 Michal Suchanek <msuchanek@suse.de> wrote:
> due to work on the fadump_extra_args I looked at the kernel parameter parser
> and found its grammar rather curious.
>
> It supports double quotes but not any other quoting characters so double quotes
> cannot be quoted. What's more, the quotes can be anywhere in the parameter
> name or value and are interpteted but are removed only from start and end of
> the parameter value.
>
> These are the patches not specific to fadump which somewhat straighten the
> qouting grammar to make it on par with common shell interpreters.
>
> Specifically double and single quotes can be used for quoting as well as
> backslashes with the usual shell semantic. All quoting characters are removed
> while the parameters are parsed.
Well. It's nice. I guess. Is there any demand for these
capabilities? I don't recall ever having seen a complaint - kernel
parameters tend to be pretty simple things.
Also, the break_arg_end() and squash_char() macros make me want to cry.
A macro which changes control flow hidden inside another macro! Are
they reeeealy necessary? Can't be done with some C helpers? Maybe put
inquote, backslash, args, i into a new struct parser_state and pass a
pointer to that around the place? At the very least, those macros
should be apologetically documented :(
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 06/10] x86/cet: Add arch_prctl functions for shadow stack
From: Yu-cheng Yu @ 2018-06-19 22:38 UTC (permalink / raw)
To: Andy Lutomirski, Kees Cook
Cc: Andy Lutomirski, H. J. Lu, Thomas Gleixner, LKML, linux-doc,
Linux-MM, linux-arch, X86 ML, H. Peter Anvin, Ingo Molnar,
Shanbhogue, Vedvyas, Ravi V. Shankar, Dave Hansen,
Jonathan Corbet, Oleg Nesterov, Arnd Bergmann, mike.kravetz,
Florian Weimer
In-Reply-To: <446EB18D-EF06-4A04-AF62-E72C68D96A84@amacapital.net>
On Tue, 2018-06-19 at 13:47 -0700, Andy Lutomirski wrote:
> >
> > On Jun 19, 2018, at 1:12 PM, Kees Cook <keescook@chromium.org>
> > wrote:
> >
> > >
> > > On Tue, Jun 19, 2018 at 10:20 AM, Andy Lutomirski <luto@amacapita
> > > l.net> wrote:
> > >
> > > >
> > > > On Jun 19, 2018, at 10:07 AM, Kees Cook <keescook@chromium.org>
> > > > wrote:
> > > >
> > > > Does it provide anything beyond what PR_DUMPABLE does?
> > > What do you mean?
> > I was just going by the name of it. I wasn't sure what "ptrace CET
> > lock" meant, so I was trying to understand if it was another "you
> > can't ptrace me" toggle, and if so, wouldn't it be redundant with
> > PR_SET_DUMPABLE = 0, etc.
> >
> No, other way around. The valid CET states are on/unlocked,
> off/unlocked, on/locked, off/locked. arch_prctl can freely the state
> unless locked. ptrace can change it no matter what. The lock is to
> prevent the existence of a gadget to disable CET (unless the gadget
> involves ptrace, but I don’t think that’s a real concern).
We have the arch_prctl now and only need to add ptrace lock/unlock.
Back to the dlopen() "relaxed" mode. Would the following work?
If the lib being loaded does not use setjmp/getcontext families (the
loader knows?), then the loader leaves shstk on. Otherwise, if the
system-wide setting is "relaxed", the loader turns off shstk and issues
a warning. In addition, if (dlopen == relaxed), then cet is not locked
in any time.
The system-wide setting (somewhere in /etc?) can be:
dlopen=force|relaxed /* controls dlopen of non-cet libs */
exec=force|relaxed /* controls exec of non-cet apps */
--
Yu-cheng
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v11 00/13] Intel SGX1 support
From: Josh Triplett @ 2018-06-19 21:48 UTC (permalink / raw)
To: Pavel Machek
Cc: Jarkko Sakkinen, x86, platform-driver-x86, dave.hansen,
sean.j.christopherson, nhorman, npmccallum, Alexei Starovoitov,
Andi Kleen, Andrew Morton, Andy Lutomirski, Borislav Petkov,
David S. Miller, David Woodhouse, Greg Kroah-Hartman,
H. Peter Anvin, Ingo Molnar, open list:INTEL SGX,
Janakarajan Natarajan, Kirill A. Shutemov, Konrad Rzeszutek Wilk,
open list:KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86), Len Brown,
Linus Walleij, open list:CRYPTO API, open list:DOCUMENTATION,
open list, open list:SPARSE CHECKER, Mauro Carvalho Chehab,
Peter Zijlstra, Rafael J. Wysocki, Randy Dunlap, Ricardo Neri,
Thomas Gleixner, Tom Lendacky, Vikas Shivappa
In-Reply-To: <20180619200414.GA3143@amd>
On Tue, Jun 19, 2018 at 10:04:15PM +0200, Pavel Machek wrote:
> On Tue 2018-06-19 17:59:43, Jarkko Sakkinen wrote:
> > On Tue, Jun 12, 2018 at 12:50:12PM +0200, Pavel Machek wrote:
> > > On Fri 2018-06-08 19:09:35, Jarkko Sakkinen wrote:
> > > > Intel(R) SGX is a set of CPU instructions that can be used by applications
> > > > to set aside private regions of code and data. The code outside the enclave
> > > > is disallowed to access the memory inside the enclave by the CPU access
> > > > control. In a way you can think that SGX provides inverted sandbox. It
> > > > protects the application from a malicious host.
> > >
> > > Do you intend to allow non-root applications to use SGX?
> > >
> > > What are non-evil uses for SGX?
> > >
> > > ...because it is quite useful for some kinds of evil:
> >
> > The default permissions for the device are 600.
>
> Good. This does not belong to non-root.
There are entirely legitimate use cases for using this as an
unprivileged user. However, that'll be up to system and distribution
policy, which can evolve over time, and it makes sense for the *initial*
kernel permission to start out root-only and then adjust permissions via
udev.
> What are some non-evil uses for SGX?
Building a software certificate store. Hardening key-agent software like
ssh-agent or gpg-agent. Building a challenge-response authentication
system. Providing more assurance that your server infrastructure is
uncompromised. Offloading computation to a system without having to
fully trust that system.
As one of many possibilities, imagine a distcc that didn't have to trust
the compile nodes. The compile nodes could fail to return results at
all, but they couldn't alter the results.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 0/5] Expose PCIe AER stats via sysfs
From: Alex G. @ 2018-06-19 22:29 UTC (permalink / raw)
To: Steven Rostedt
Cc: Bjorn Helgaas, Rajat Jain, Bjorn Helgaas, Jonathan Corbet,
Philippe Ombredanne, Kate Stewart, Thomas Gleixner,
Greg Kroah-Hartman, Frederick Lawler, Oza Pawandeep, Keith Busch,
Gabriele Paoloni, Thomas Tai, linux-pci, linux-doc, linux-kernel,
Jes Sorensen, Kyle McMartin, rajatxjain
In-Reply-To: <20180619182536.648e763f@gandalf.local.home>
On 06/19/2018 05:25 PM, Steven Rostedt wrote:
> On Tue, 19 Jun 2018 17:20:28 -0500
> "Alex G." <mr.nuke.me@gmail.com> wrote:
>
>> I see the next phoronix headline:
>>
>> "Linux maintainer breaks things and watches in amusement as contributors
>> rush to the fix"
>
> But isn't this the daily routine of every Linux maintainer?
Phoronix wrote articles about much less interesting things than that.
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 0/5] Expose PCIe AER stats via sysfs
From: Steven Rostedt @ 2018-06-19 22:25 UTC (permalink / raw)
To: Alex G.
Cc: Bjorn Helgaas, Rajat Jain, Bjorn Helgaas, Jonathan Corbet,
Philippe Ombredanne, Kate Stewart, Thomas Gleixner,
Greg Kroah-Hartman, Frederick Lawler, Oza Pawandeep, Keith Busch,
Gabriele Paoloni, Thomas Tai, linux-pci, linux-doc, linux-kernel,
Jes Sorensen, Kyle McMartin, rajatxjain
In-Reply-To: <c6a5f1ca-a3da-4dd8-1178-a017d6d771c6@gmail.com>
On Tue, 19 Jun 2018 17:20:28 -0500
"Alex G." <mr.nuke.me@gmail.com> wrote:
> I see the next phoronix headline:
>
> "Linux maintainer breaks things and watches in amusement as contributors
> rush to the fix"
But isn't this the daily routine of every Linux maintainer?
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 0/5] Expose PCIe AER stats via sysfs
From: Alex G. @ 2018-06-19 22:20 UTC (permalink / raw)
To: Bjorn Helgaas, Rajat Jain
Cc: Bjorn Helgaas, Jonathan Corbet, Philippe Ombredanne, Kate Stewart,
Thomas Gleixner, Greg Kroah-Hartman, Frederick Lawler,
Oza Pawandeep, Keith Busch, Gabriele Paoloni, Thomas Tai,
Steven Rostedt (VMware), linux-pci, linux-doc, linux-kernel,
Jes Sorensen, Kyle McMartin, rajatxjain
In-Reply-To: <20180619221651.GH33049@bhelgaas-glaptop.roam.corp.google.com>
On 06/19/2018 05:16 PM, Bjorn Helgaas wrote:
> On Wed, May 23, 2018 at 10:58:03AM -0700, Rajat Jain wrote:
>> This patchset exposes the AER stats via the sysfs attributes.
>>
>> Patchset v2 has minor changes to v1 based on the review comments,
>> no functional change.
>> Primarily:
>> * Fix license header
>> * Use tabs instead of spaces
>> * Remove use on unlikely() etc
>> * Move documentation to Documentation/ABI/
>>
>> Rajat Jain (5):
>> PCI/AER: Define and allocate aer_stats structure for AER capable
>> devices
>> PCI/AER: Add sysfs stats for AER capable devices
>> PCI/AER: Add sysfs attributes to provide breakdown of AERs
>> PCI/AER: Add sysfs attributes for rootport cumulative stats
>> Documentation/ABI: Add details of PCI AER statistics
>>
>> .../testing/sysfs-bus-pci-devices-aer_stats | 103 ++++++++++
>> Documentation/PCI/pcieaer-howto.txt | 5 +
>> drivers/pci/pci-sysfs.c | 3 +
>> drivers/pci/pci.h | 4 +-
>> drivers/pci/pcie/aer/Makefile | 2 +-
>> drivers/pci/pcie/aer/aerdrv.h | 15 ++
>> drivers/pci/pcie/aer/aerdrv_core.c | 11 +
>> drivers/pci/pcie/aer/aerdrv_errprint.c | 7 +-
>> drivers/pci/pcie/aer/aerdrv_stats.c | 192 ++++++++++++++++++
>> drivers/pci/probe.c | 1 +
>> include/linux/pci.h | 3 +
>> 11 files changed, 342 insertions(+), 4 deletions(-)
>> create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
>> create mode 100644 drivers/pci/pcie/aer/aerdrv_stats.c
>
> I broke this by putting all the AER code in one file in v4.18-rc1,
I see the next phoronix headline:
"Linux maintainer breaks things and watches in amusement as contributors
rush to the fix"
;)
Alex
> sorry! Would you mind rebasing these on top of that?
>
> Since everything AER-related is now in aer.c, I'd suggest putting the
> stats code there, too.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 0/5] Expose PCIe AER stats via sysfs
From: Rajat Jain @ 2018-06-19 22:17 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, Jonathan Corbet, Philippe Ombredanne, Kate Stewart,
Thomas Gleixner, Greg Kroah-Hartman, Frederick Lawler,
Oza Pawandeep, Busch, Keith, Gabriele Paoloni, Alexandru Gagniuc,
Thomas Tai, Steven Rostedt, linux-pci, linux-doc,
Linux Kernel Mailing List, Jes Sorensen, Kyle McMartin,
Rajat Jain
In-Reply-To: <20180619221651.GH33049@bhelgaas-glaptop.roam.corp.google.com>
Sure, no problem. Will do.
On Tue, Jun 19, 2018 at 3:16 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Wed, May 23, 2018 at 10:58:03AM -0700, Rajat Jain wrote:
> > This patchset exposes the AER stats via the sysfs attributes.
> >
> > Patchset v2 has minor changes to v1 based on the review comments,
> > no functional change.
> > Primarily:
> > * Fix license header
> > * Use tabs instead of spaces
> > * Remove use on unlikely() etc
> > * Move documentation to Documentation/ABI/
> >
> > Rajat Jain (5):
> > PCI/AER: Define and allocate aer_stats structure for AER capable
> > devices
> > PCI/AER: Add sysfs stats for AER capable devices
> > PCI/AER: Add sysfs attributes to provide breakdown of AERs
> > PCI/AER: Add sysfs attributes for rootport cumulative stats
> > Documentation/ABI: Add details of PCI AER statistics
> >
> > .../testing/sysfs-bus-pci-devices-aer_stats | 103 ++++++++++
> > Documentation/PCI/pcieaer-howto.txt | 5 +
> > drivers/pci/pci-sysfs.c | 3 +
> > drivers/pci/pci.h | 4 +-
> > drivers/pci/pcie/aer/Makefile | 2 +-
> > drivers/pci/pcie/aer/aerdrv.h | 15 ++
> > drivers/pci/pcie/aer/aerdrv_core.c | 11 +
> > drivers/pci/pcie/aer/aerdrv_errprint.c | 7 +-
> > drivers/pci/pcie/aer/aerdrv_stats.c | 192 ++++++++++++++++++
> > drivers/pci/probe.c | 1 +
> > include/linux/pci.h | 3 +
> > 11 files changed, 342 insertions(+), 4 deletions(-)
> > create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
> > create mode 100644 drivers/pci/pcie/aer/aerdrv_stats.c
>
> I broke this by putting all the AER code in one file in v4.18-rc1,
> sorry! Would you mind rebasing these on top of that?
>
> Since everything AER-related is now in aer.c, I'd suggest putting the
> stats code there, too.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 0/5] Expose PCIe AER stats via sysfs
From: Bjorn Helgaas @ 2018-06-19 22:16 UTC (permalink / raw)
To: Rajat Jain
Cc: Bjorn Helgaas, Jonathan Corbet, Philippe Ombredanne, Kate Stewart,
Thomas Gleixner, Greg Kroah-Hartman, Frederick Lawler,
Oza Pawandeep, Keith Busch, Gabriele Paoloni, Alexandru Gagniuc,
Thomas Tai, Steven Rostedt (VMware), linux-pci, linux-doc,
linux-kernel, Jes Sorensen, Kyle McMartin, rajatxjain
In-Reply-To: <20180523175808.28030-1-rajatja@google.com>
On Wed, May 23, 2018 at 10:58:03AM -0700, Rajat Jain wrote:
> This patchset exposes the AER stats via the sysfs attributes.
>
> Patchset v2 has minor changes to v1 based on the review comments,
> no functional change.
> Primarily:
> * Fix license header
> * Use tabs instead of spaces
> * Remove use on unlikely() etc
> * Move documentation to Documentation/ABI/
>
> Rajat Jain (5):
> PCI/AER: Define and allocate aer_stats structure for AER capable
> devices
> PCI/AER: Add sysfs stats for AER capable devices
> PCI/AER: Add sysfs attributes to provide breakdown of AERs
> PCI/AER: Add sysfs attributes for rootport cumulative stats
> Documentation/ABI: Add details of PCI AER statistics
>
> .../testing/sysfs-bus-pci-devices-aer_stats | 103 ++++++++++
> Documentation/PCI/pcieaer-howto.txt | 5 +
> drivers/pci/pci-sysfs.c | 3 +
> drivers/pci/pci.h | 4 +-
> drivers/pci/pcie/aer/Makefile | 2 +-
> drivers/pci/pcie/aer/aerdrv.h | 15 ++
> drivers/pci/pcie/aer/aerdrv_core.c | 11 +
> drivers/pci/pcie/aer/aerdrv_errprint.c | 7 +-
> drivers/pci/pcie/aer/aerdrv_stats.c | 192 ++++++++++++++++++
> drivers/pci/probe.c | 1 +
> include/linux/pci.h | 3 +
> 11 files changed, 342 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
> create mode 100644 drivers/pci/pcie/aer/aerdrv_stats.c
I broke this by putting all the AER code in one file in v4.18-rc1,
sorry! Would you mind rebasing these on top of that?
Since everything AER-related is now in aer.c, I'd suggest putting the
stats code there, too.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 06/10] x86/cet: Add arch_prctl functions for shadow stack
From: Andy Lutomirski @ 2018-06-19 20:47 UTC (permalink / raw)
To: Kees Cook
Cc: Yu-cheng Yu, Andy Lutomirski, H. J. Lu, Thomas Gleixner, LKML,
linux-doc, Linux-MM, linux-arch, X86 ML, H. Peter Anvin,
Ingo Molnar, Shanbhogue, Vedvyas, Ravi V. Shankar, Dave Hansen,
Jonathan Corbet, Oleg Nesterov, Arnd Bergmann, mike.kravetz,
Florian Weimer
In-Reply-To: <CAGXu5jLEMy_T_5OtXLT+pUCt=Nk53nBbuRvrUgJBhq-4RZ=yCA@mail.gmail.com>
> On Jun 19, 2018, at 1:12 PM, Kees Cook <keescook@chromium.org> wrote:
>
>> On Tue, Jun 19, 2018 at 10:20 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>
>>> On Jun 19, 2018, at 10:07 AM, Kees Cook <keescook@chromium.org> wrote:
>>>
>>> Does it provide anything beyond what PR_DUMPABLE does?
>>
>> What do you mean?
>
> I was just going by the name of it. I wasn't sure what "ptrace CET
> lock" meant, so I was trying to understand if it was another "you
> can't ptrace me" toggle, and if so, wouldn't it be redundant with
> PR_SET_DUMPABLE = 0, etc.
>
No, other way around. The valid CET states are on/unlocked, off/unlocked, on/locked, off/locked. arch_prctl can freely the state unless locked. ptrace can change it no matter what. The lock is to prevent the existence of a gadget to disable CET (unless the gadget involves ptrace, but I don’t think that’s a real concern).--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v11 00/13] Intel SGX1 support
From: Peter Zijlstra @ 2018-06-19 20:36 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Pavel Machek, x86, platform-driver-x86, dave.hansen,
sean.j.christopherson, nhorman, npmccallum, Alexei Starovoitov,
Andi Kleen, Andrew Morton, Andy Lutomirski, Borislav Petkov,
David S. Miller, David Woodhouse, Greg Kroah-Hartman,
H. Peter Anvin, Ingo Molnar, open list:INTEL SGX,
Janakarajan Natarajan, Kirill A. Shutemov, Konrad Rzeszutek Wilk,
open list:KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86), Len Brown,
Linus Walleij, open list:CRYPTO API, open list:DOCUMENTATION,
open list, open list:SPARSE CHECKER, Mauro Carvalho Chehab,
Rafael J. Wysocki, Randy Dunlap, Ricardo Neri, Thomas Gleixner,
Tom Lendacky, Vikas Shivappa
In-Reply-To: <20180619145943.GC8034@linux.intel.com>
*sigh*, could you not cross-post with closed/moderated lists, that's rude.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v11 00/13] Intel SGX1 support
From: Peter Zijlstra @ 2018-06-19 20:23 UTC (permalink / raw)
To: Pavel Machek
Cc: Jarkko Sakkinen, x86, platform-driver-x86, dave.hansen,
sean.j.christopherson, nhorman, npmccallum, Alexei Starovoitov,
Andi Kleen, Andrew Morton, Andy Lutomirski, Borislav Petkov,
David S. Miller, David Woodhouse, Greg Kroah-Hartman,
H. Peter Anvin, Ingo Molnar, open list:INTEL SGX,
Janakarajan Natarajan, Kirill A. Shutemov, Konrad Rzeszutek Wilk,
open list:KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86), Len Brown,
Linus Walleij, open list:CRYPTO API, open list:DOCUMENTATION,
open list, open list:SPARSE CHECKER, Mauro Carvalho Chehab,
Rafael J. Wysocki, Randy Dunlap, Ricardo Neri, Thomas Gleixner,
Tom Lendacky, Vikas Shivappa
In-Reply-To: <20180619200414.GA3143@amd>
On Tue, Jun 19, 2018 at 10:04:15PM +0200, Pavel Machek wrote:
> What are some non-evil uses for SGX?
Soft-TPM is one that was proposed at some time.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 06/10] x86/cet: Add arch_prctl functions for shadow stack
From: Kees Cook @ 2018-06-19 20:12 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Yu-cheng Yu, Andy Lutomirski, H. J. Lu, Thomas Gleixner, LKML,
linux-doc, Linux-MM, linux-arch, X86 ML, H. Peter Anvin,
Ingo Molnar, Shanbhogue, Vedvyas, Ravi V. Shankar, Dave Hansen,
Jonathan Corbet, Oleg Nesterov, Arnd Bergmann, mike.kravetz,
Florian Weimer
In-Reply-To: <0AF8B71E-B6CC-42DE-B95C-93896196C3D7@amacapital.net>
On Tue, Jun 19, 2018 at 10:20 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
>> On Jun 19, 2018, at 10:07 AM, Kees Cook <keescook@chromium.org> wrote:
>>
>> Does it provide anything beyond what PR_DUMPABLE does?
>
> What do you mean?
I was just going by the name of it. I wasn't sure what "ptrace CET
lock" meant, so I was trying to understand if it was another "you
can't ptrace me" toggle, and if so, wouldn't it be redundant with
PR_SET_DUMPABLE = 0, etc.
-Kees
--
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v11 00/13] Intel SGX1 support
From: Pavel Machek @ 2018-06-19 20:04 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: x86, platform-driver-x86, dave.hansen, sean.j.christopherson,
nhorman, npmccallum, Alexei Starovoitov, Andi Kleen,
Andrew Morton, Andy Lutomirski, Borislav Petkov, David S. Miller,
David Woodhouse, Greg Kroah-Hartman, H. Peter Anvin, Ingo Molnar,
open list:INTEL SGX, Janakarajan Natarajan, Kirill A. Shutemov,
Konrad Rzeszutek Wilk,
open list:KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86), Len Brown,
Linus Walleij, open list:CRYPTO API, open list:DOCUMENTATION,
open list, open list:SPARSE CHECKER, Mauro Carvalho Chehab,
Peter Zijlstra, Rafael J. Wysocki, Randy Dunlap, Ricardo Neri,
Thomas Gleixner, Tom Lendacky, Vikas Shivappa
In-Reply-To: <20180619145943.GC8034@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]
On Tue 2018-06-19 17:59:43, Jarkko Sakkinen wrote:
> On Tue, Jun 12, 2018 at 12:50:12PM +0200, Pavel Machek wrote:
> > On Fri 2018-06-08 19:09:35, Jarkko Sakkinen wrote:
> > > Intel(R) SGX is a set of CPU instructions that can be used by applications
> > > to set aside private regions of code and data. The code outside the enclave
> > > is disallowed to access the memory inside the enclave by the CPU access
> > > control. In a way you can think that SGX provides inverted sandbox. It
> > > protects the application from a malicious host.
> >
> > Do you intend to allow non-root applications to use SGX?
> >
> > What are non-evil uses for SGX?
> >
> > ...because it is quite useful for some kinds of evil:
>
> The default permissions for the device are 600.
Good. This does not belong to non-root. But question still remains:
What are some non-evil uses for SGX?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH] coda: stop using 'struct timespec' in user API
From: Arnd Bergmann @ 2018-06-19 19:19 UTC (permalink / raw)
To: Arnd Bergmann, y2038 Mailman List, Jonathan Corbet,
Deepa Dinamani, open list:DOCUMENTATION,
Linux Kernel Mailing List
In-Reply-To: <CAK8P3a1L5srfOR1vXQX5PM5FTXN4ZM+qAtqDthppiX=f1osFbQ@mail.gmail.com>
On Tue, Jun 19, 2018 at 9:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Jun 19, 2018 at 6:56 PM, Jan Harkes <jaharkes@cs.cmu.edu> wrote:
>>> An open question is what should happen to actual times past y2038,
>>> as they are now truncated to the last valid date when sent to user
>>
>> That is definitely quite a hard problem because this propagates all the
>> way back to the Coda file servers and how they store metadata.
>> In fact the existing client-server protocol only uses 32-bit time in
>> seconds, so we already lose the nanosecond resolution and 64-bit systems
>> don't actually benefit from having the extra bits in their struct timespec.
I couldn't find out enough background for this, maybe you can fill it
in: I see that there is a user space component and a server component,
but I'm not sure if there is exactly one of each, or if there are multiple
implementations that are written against the same interface.
If we only have one code base, it should be fairly straightforward to
make it deal with 'unsigned' timestamps consistently, which would
let the code work fine until 2106 rather than wrapping around from
2038 to 1902.
> > Not exposing an internal kernel datatype is definitely an improvement,
> > so this is an ACK for me.
To clarify: the problem isn't as much the internal kernel type, but the
glibc internal type that we know is going to change with future glibc
versions. This means it is important that the header file change makes
it into every user space program that uses the psdev interface.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] coda: stop using 'struct timespec' in user API
From: Arnd Bergmann @ 2018-06-19 19:13 UTC (permalink / raw)
To: Arnd Bergmann, y2038 Mailman List, Jonathan Corbet,
Deepa Dinamani, open list:DOCUMENTATION,
Linux Kernel Mailing List
In-Reply-To: <20180619165633.acyxrweiedyhvre7@cs.cmu.edu>
On Tue, Jun 19, 2018 at 6:56 PM, Jan Harkes <jaharkes@cs.cmu.edu> wrote:
> On Tue, Jun 19, 2018 at 05:37:35PM +0200, Arnd Bergmann wrote:
>> Unfortunately, this breaks the layout of the coda_vattr structure, so
>> we need to redefine that in terms of something that does not change.
>> I'm introducing a new 'struct vtimespec' structure here that keeps
>> the existing layout, and the same change has to be done in the coda
>> user space copy of linux/coda.h before anyone can use that on a 32-bit
>> architecture with 64-bit time_t.
>
> This looks good to me.
>
>> An open question is what should happen to actual times past y2038,
>> as they are now truncated to the last valid date when sent to user
>
> That is definitely quite a hard problem because this propagates all the
> way back to the Coda file servers and how they store metadata.
> In fact the existing client-server protocol only uses 32-bit time in
> seconds, so we already lose the nanosecond resolution and 64-bit systems
> don't actually benefit from having the extra bits in their struct timespec.
>
> Not exposing an internal kernel datatype is definitely an improvement,
> so this is an ACK for me.
Thanks,
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] coda: stop using 'struct timespec' in user API
From: Jan Harkes @ 2018-06-19 16:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: y2038, Jonathan Corbet, Deepa Dinamani, linux-doc, linux-kernel
In-Reply-To: <20180619153822.3638475-1-arnd@arndb.de>
On Tue, Jun 19, 2018 at 05:37:35PM +0200, Arnd Bergmann wrote:
> Unfortunately, this breaks the layout of the coda_vattr structure, so
> we need to redefine that in terms of something that does not change.
> I'm introducing a new 'struct vtimespec' structure here that keeps
> the existing layout, and the same change has to be done in the coda
> user space copy of linux/coda.h before anyone can use that on a 32-bit
> architecture with 64-bit time_t.
This looks good to me.
> An open question is what should happen to actual times past y2038,
> as they are now truncated to the last valid date when sent to user
That is definitely quite a hard problem because this propagates all the
way back to the Coda file servers and how they store metadata.
In fact the existing client-server protocol only uses 32-bit time in
seconds, so we already lose the nanosecond resolution and 64-bit systems
don't actually benefit from having the extra bits in their struct timespec.
Not exposing an internal kernel datatype is definitely an improvement,
so this is an ACK for me.
Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 06/10] x86/cet: Add arch_prctl functions for shadow stack
From: Andy Lutomirski @ 2018-06-19 17:20 UTC (permalink / raw)
To: Kees Cook
Cc: Yu-cheng Yu, Andy Lutomirski, H. J. Lu, Thomas Gleixner, LKML,
linux-doc, Linux-MM, linux-arch, X86 ML, H. Peter Anvin,
Ingo Molnar, Shanbhogue, Vedvyas, Ravi V. Shankar, Dave Hansen,
Jonathan Corbet, Oleg Nesterov, Arnd Bergmann, mike.kravetz,
Florian Weimer
In-Reply-To: <CAGXu5jJ4ivrvi-kG0iY=4C0mQQXBDXwPdfY36Dk+JqOpX19n0w@mail.gmail.com>
> On Jun 19, 2018, at 10:07 AM, Kees Cook <keescook@chromium.org> wrote:
>
>> On Tue, Jun 19, 2018 at 9:59 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>>> On Tue, 2018-06-19 at 09:44 -0700, Kees Cook wrote:
>>> On Tue, Jun 19, 2018 at 7:50 AM, Andy Lutomirski <luto@amacapital.net
>>>> wrote:
>>>>
>>>>>
>>>>> On Jun 18, 2018, at 5:52 PM, Kees Cook <keescook@chromium.org>
>>>>> wrote:
>>>>> Following Linus's request for "slow introduction" of new security
>>>>> features, likely the best approach is to default to "relaxed"
>>>>> (with a
>>>>> warning about down-grades), and allow distros/end-users to pick
>>>>> "forced" if they know their libraries are all CET-enabled.
>>>> I still don’t get what “relaxed” is for. I think the right design
>>>> is:
>>>>
>>>> Processes start with CET on or off depending on the ELF note, but
>>>> they start with CET unlocked no matter what. They can freely switch
>>>> CET on and off (subject to being clever enough not to crash if they
>>>> turn it on and then return right off the end of the shadow stack)
>>>> until they call ARCH_CET_LOCK.
>>> I'm fine with this. I'd expect modern loaders to just turn on CET and
>>> ARCH_CET_LOCK immediately and be done with it. :P
>>
>> This is the current implementation. If the loader has CET in its ELF
>> header, it is executed with CET on. The loader will turn off CET if
>> the application being loaded does not support it (in the ELF header).
>> The loader calls ARCH_CET_LOCK before passing to the application. But
>> how do we handle dlopen?
>
> I thought CET_LOCK would not get set in "relaxed" mode, due to dlopen
> usage, and that would be the WARN case. People without dlopen concerns
> can boot with "enforced" mode? If a system builder knows there are no
> legacy dlopens they build with enforced enabled, etc.
I think we’re getting ahead of ourselves. dlopen() of a non-CET-aware library in a CET process is distinctly non-trivial, especially in a multithreaded process. I think getting it right will require *userspace* support. It certainly needs ld.so to issue to arch_prctl at a bare minimum. So I see no point to a kernel-supplied “relaxed” mode. I think there may be demand for a ld.so relaxed mode, but it will have nothing to do with boot options.
It’s potentially helpful to add an arch_prctl that turns CET off for all threads, but only if unlocked. It would obviously be one hell of a gadget.
>
>>>> Ptrace gets new APIs to turn CET on and off and to lock and unlock
>>>> it. If an attacker finds a “ptrace me and turn off CET” gadget,
>>>> then they might as well just do “ptrace me and write shell code”
>>>> instead. It’s basically the same gadget. Keep in mind that the
>>>> actual sequence of syscalls to do this is incredibly complicated.
>>> Right -- if an attacker can control ptrace of the target, we're way
>>> past CET. The only concern I have, though, is taking advantage of
>>> expected ptracing. For example: browsers tend to have crash handlers
>>> that launch a ptracer. If ptracing disabled CET for all threads, this
>>> won't by safe: an attacker just gains control in two threads, crashes
>>> one to get the ptracer to attach, which disables CET in the other
>>> thread and the attacker continues ROP as normal. As long as the
>>> ptrace
>>> disabling is thread-specific, I think this will be okay.
>>
>> If ptrace can turn CET on/off and it is thread-specific, do we still
>> need ptrace lock/unlock?
Let me clarify. I don’t think ptrace() should have any automatic effect on CET. I think there should be an explicit way to ask ptrace to twiddle CET, and it should probably apply per thread.
>
> Does it provide anything beyond what PR_DUMPABLE does?
What do you mean?
>
> -Kees
>
> --
> Kees Cook
> Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 06/10] x86/cet: Add arch_prctl functions for shadow stack
From: Kees Cook @ 2018-06-19 17:07 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: Andy Lutomirski, Andy Lutomirski, H. J. Lu, Thomas Gleixner, LKML,
linux-doc, Linux-MM, linux-arch, X86 ML, H. Peter Anvin,
Ingo Molnar, Shanbhogue, Vedvyas, Ravi V. Shankar, Dave Hansen,
Jonathan Corbet, Oleg Nesterov, Arnd Bergmann, mike.kravetz,
Florian Weimer
In-Reply-To: <1529427588.23068.7.camel@intel.com>
On Tue, Jun 19, 2018 at 9:59 AM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> On Tue, 2018-06-19 at 09:44 -0700, Kees Cook wrote:
>> On Tue, Jun 19, 2018 at 7:50 AM, Andy Lutomirski <luto@amacapital.net
>> > wrote:
>> >
>> > >
>> > > On Jun 18, 2018, at 5:52 PM, Kees Cook <keescook@chromium.org>
>> > > wrote:
>> > > Following Linus's request for "slow introduction" of new security
>> > > features, likely the best approach is to default to "relaxed"
>> > > (with a
>> > > warning about down-grades), and allow distros/end-users to pick
>> > > "forced" if they know their libraries are all CET-enabled.
>> > I still don’t get what “relaxed” is for. I think the right design
>> > is:
>> >
>> > Processes start with CET on or off depending on the ELF note, but
>> > they start with CET unlocked no matter what. They can freely switch
>> > CET on and off (subject to being clever enough not to crash if they
>> > turn it on and then return right off the end of the shadow stack)
>> > until they call ARCH_CET_LOCK.
>> I'm fine with this. I'd expect modern loaders to just turn on CET and
>> ARCH_CET_LOCK immediately and be done with it. :P
>
> This is the current implementation. If the loader has CET in its ELF
> header, it is executed with CET on. The loader will turn off CET if
> the application being loaded does not support it (in the ELF header).
> The loader calls ARCH_CET_LOCK before passing to the application. But
> how do we handle dlopen?
I thought CET_LOCK would not get set in "relaxed" mode, due to dlopen
usage, and that would be the WARN case. People without dlopen concerns
can boot with "enforced" mode? If a system builder knows there are no
legacy dlopens they build with enforced enabled, etc.
>> > Ptrace gets new APIs to turn CET on and off and to lock and unlock
>> > it. If an attacker finds a “ptrace me and turn off CET” gadget,
>> > then they might as well just do “ptrace me and write shell code”
>> > instead. It’s basically the same gadget. Keep in mind that the
>> > actual sequence of syscalls to do this is incredibly complicated.
>> Right -- if an attacker can control ptrace of the target, we're way
>> past CET. The only concern I have, though, is taking advantage of
>> expected ptracing. For example: browsers tend to have crash handlers
>> that launch a ptracer. If ptracing disabled CET for all threads, this
>> won't by safe: an attacker just gains control in two threads, crashes
>> one to get the ptracer to attach, which disables CET in the other
>> thread and the attacker continues ROP as normal. As long as the
>> ptrace
>> disabling is thread-specific, I think this will be okay.
>
> If ptrace can turn CET on/off and it is thread-specific, do we still
> need ptrace lock/unlock?
Does it provide anything beyond what PR_DUMPABLE does?
-Kees
--
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 06/10] x86/cet: Add arch_prctl functions for shadow stack
From: Yu-cheng Yu @ 2018-06-19 16:59 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski
Cc: Andy Lutomirski, H. J. Lu, Thomas Gleixner, LKML, linux-doc,
Linux-MM, linux-arch, X86 ML, H. Peter Anvin, Ingo Molnar,
Shanbhogue, Vedvyas, Ravi V. Shankar, Dave Hansen,
Jonathan Corbet, Oleg Nesterov, Arnd Bergmann, mike.kravetz,
Florian Weimer
In-Reply-To: <CAGXu5jJNgu4bW_Zthqjfpe9gLxK0zxG8QFEqqK+pJNebz6tUaw@mail.gmail.com>
On Tue, 2018-06-19 at 09:44 -0700, Kees Cook wrote:
> On Tue, Jun 19, 2018 at 7:50 AM, Andy Lutomirski <luto@amacapital.net
> > wrote:
> >
> > >
> > > On Jun 18, 2018, at 5:52 PM, Kees Cook <keescook@chromium.org>
> > > wrote:
> > > Following Linus's request for "slow introduction" of new security
> > > features, likely the best approach is to default to "relaxed"
> > > (with a
> > > warning about down-grades), and allow distros/end-users to pick
> > > "forced" if they know their libraries are all CET-enabled.
> > I still don’t get what “relaxed” is for. I think the right design
> > is:
> >
> > Processes start with CET on or off depending on the ELF note, but
> > they start with CET unlocked no matter what. They can freely switch
> > CET on and off (subject to being clever enough not to crash if they
> > turn it on and then return right off the end of the shadow stack)
> > until they call ARCH_CET_LOCK.
> I'm fine with this. I'd expect modern loaders to just turn on CET and
> ARCH_CET_LOCK immediately and be done with it. :P
This is the current implementation. If the loader has CET in its ELF
header, it is executed with CET on. The loader will turn off CET if
the application being loaded does not support it (in the ELF header).
The loader calls ARCH_CET_LOCK before passing to the application. But
how do we handle dlopen?
> >
> > Ptrace gets new APIs to turn CET on and off and to lock and unlock
> > it. If an attacker finds a “ptrace me and turn off CET” gadget,
> > then they might as well just do “ptrace me and write shell code”
> > instead. It’s basically the same gadget. Keep in mind that the
> > actual sequence of syscalls to do this is incredibly complicated.
> Right -- if an attacker can control ptrace of the target, we're way
> past CET. The only concern I have, though, is taking advantage of
> expected ptracing. For example: browsers tend to have crash handlers
> that launch a ptracer. If ptracing disabled CET for all threads, this
> won't by safe: an attacker just gains control in two threads, crashes
> one to get the ptracer to attach, which disables CET in the other
> thread and the attacker continues ROP as normal. As long as the
> ptrace
> disabling is thread-specific, I think this will be okay.
If ptrace can turn CET on/off and it is thread-specific, do we still
need ptrace lock/unlock?
Yu-cheng
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 06/10] x86/cet: Add arch_prctl functions for shadow stack
From: Kees Cook @ 2018-06-19 16:44 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andy Lutomirski, H. J. Lu, Thomas Gleixner, Yu-cheng Yu, LKML,
linux-doc, Linux-MM, linux-arch, X86 ML, H. Peter Anvin,
Ingo Molnar, Shanbhogue, Vedvyas, Ravi V. Shankar, Dave Hansen,
Jonathan Corbet, Oleg Nesterov, Arnd Bergmann, mike.kravetz,
Florian Weimer
In-Reply-To: <569B4719-6283-4575-A16E-D0A78D280F4E@amacapital.net>
On Tue, Jun 19, 2018 at 7:50 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Jun 18, 2018, at 5:52 PM, Kees Cook <keescook@chromium.org> wrote:
>> Following Linus's request for "slow introduction" of new security
>> features, likely the best approach is to default to "relaxed" (with a
>> warning about down-grades), and allow distros/end-users to pick
>> "forced" if they know their libraries are all CET-enabled.
>
> I still don’t get what “relaxed” is for. I think the right design is:
>
> Processes start with CET on or off depending on the ELF note, but they start with CET unlocked no matter what. They can freely switch CET on and off (subject to being clever enough not to crash if they turn it on and then return right off the end of the shadow stack) until they call ARCH_CET_LOCK.
I'm fine with this. I'd expect modern loaders to just turn on CET and
ARCH_CET_LOCK immediately and be done with it. :P
> Ptrace gets new APIs to turn CET on and off and to lock and unlock it. If an attacker finds a “ptrace me and turn off CET” gadget, then they might as well just do “ptrace me and write shell code” instead. It’s basically the same gadget. Keep in mind that the actual sequence of syscalls to do this is incredibly complicated.
Right -- if an attacker can control ptrace of the target, we're way
past CET. The only concern I have, though, is taking advantage of
expected ptracing. For example: browsers tend to have crash handlers
that launch a ptracer. If ptracing disabled CET for all threads, this
won't by safe: an attacker just gains control in two threads, crashes
one to get the ptracer to attach, which disables CET in the other
thread and the attacker continues ROP as normal. As long as the ptrace
disabling is thread-specific, I think this will be okay.
-Kees
--
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH -tip v6 26/27] Documentation: kprobes: Add how to change the execution path
From: Randy Dunlap @ 2018-06-19 16:31 UTC (permalink / raw)
To: Masami Hiramatsu, Thomas Gleixner, Ingo Molnar
Cc: Ingo Molnar, H . Peter Anvin, linux-kernel,
Ananth N Mavinakayanahalli, Andrew Morton, Steven Rostedt,
linux-arch, Jonathan Corbet, linux-doc
In-Reply-To: <152942500680.15209.12374262914863044775.stgit@devbox>
On 06/19/2018 09:16 AM, Masami Hiramatsu wrote:
> Add a section that explaining how to change the execution
> path with kprobes and warnings for some arch.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-doc@vger.kernel.org
> ---
> Documentation/kprobes.txt | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
> index 3e9e99ea751b..8a98eed1521b 100644
> --- a/Documentation/kprobes.txt
> +++ b/Documentation/kprobes.txt
> @@ -80,6 +80,26 @@ After the instruction is single-stepped, Kprobes executes the
> "post_handler," if any, that is associated with the kprobe.
> Execution then continues with the instruction following the probepoint.
>
Hi,
I have a few small suggestions...
> +Changing Execution Path
> +-----------------------
> +
> +Since the kprobes can probe into a running kernel code, it can change
Since kprobes can probe into running kernel code, it can change
> +the register set, including instruction pointer. This operation
> +requires maximum attention, such as keeping the stack frame, recovering
> +execution path etc. Since it is operated on running kernel and need deep
Since it operates on a running kernel and needs deep
> +knowladge of the archtecture and concurrent computing, you can easily
knowledge of the architecture
> +shot your foot.
shoot
> +
> +If you change the instruction pointer (and set up other related
> +registers) in pre_handler, you must return !0 so that the kprobes
so that kprobes
> +stops single stepping and just returns to given address.
to the given address.
> +This also means post_handler should not be called anymore.
> +
> +Note that this operation may be harder on some architectures which
> +use TOC (Table of Contents) for function call, since you have to
> +setup new TOC for your function in your module, and recover old
setup a new TOC for your function in your module, and recover the old
> +one after back from it.
one after returning from it.
> +
> Return Probes
> -------------
--
~Randy
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 5/5] Documentation/ABI: Add details of PCI AER statistics
From: Rajat Jain @ 2018-06-19 16:31 UTC (permalink / raw)
To: Oza Pawandeep
Cc: Rajat Jain, Bjorn Helgaas, Jonathan Corbet, Philippe Ombredanne,
Kate Stewart, Thomas Gleixner, Greg Kroah-Hartman,
Frederick Lawler, Busch, Keith, Gabriele Paoloni,
Alexandru Gagniuc, Thomas Tai, Steven Rostedt, linux-pci,
linux-doc, Linux Kernel Mailing List, Jes Sorensen, Kyle McMartin
In-Reply-To: <7e146f62d1fa82a6f37848b22efc1b97@codeaurora.org>
On Mon, Jun 18, 2018 at 11:03 PM, <poza@codeaurora.org> wrote:
> On 2018-06-19 05:41, Rajat Jain wrote:
>>
>> Hello,
>>
>> On Sat, Jun 16, 2018 at 10:24 PM <poza@codeaurora.org> wrote:
>>>
>>>
>>> On 2018-05-23 23:28, Rajat Jain wrote:
>>> > Add the PCI AER statistics details to
>>> > Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
>>> > and provide a pointer to it in
>>> > Documentation/PCI/pcieaer-howto.txt
>>> >
>>> > Signed-off-by: Rajat Jain <rajatja@google.com>
>>> > ---
>>> > v2: Move the documentation to Documentation/ABI/
>>> >
>>> > .../testing/sysfs-bus-pci-devices-aer_stats | 103 ++++++++++++++++++
>>> > Documentation/PCI/pcieaer-howto.txt | 5 +
>>> > 2 files changed, 108 insertions(+)
>>> > create mode 100644
>>> > Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
>>> >
>>> > diff --git a/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
>>> > b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
>>> > new file mode 100644
>>> > index 000000000000..f55c389290ac
>>> > --- /dev/null
>>> > +++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
>>> > @@ -0,0 +1,103 @@
>>> > +==========================
>>> > +PCIe Device AER statistics
>>> > +==========================
>>> > +These attributes show up under all the devices that are AER capable.
>>> > These
>>> > +statistical counters indicate the errors "as seen/reported by the
>>> > device".
>>> > +Note that this may mean that if an end point is causing problems, the
>>> > AER
>>> > +counters may increment at its link partner (e.g. root port) because
>>> > the
>>> > +errors will be "seen" / reported by the link partner and not the the
>>> > +problematic end point itself (which may report all counters as 0 as it
>>> > never
>>> > +saw any problems).
>>> > +
>>> > +Where:
>>> > /sys/bus/pci/devices/<dev>/aer_stats/dev_total_cor_errs
>>> > +Date: May 2018
>>> > +Kernel Version: 4.17.0
>>> > +Contact: linux-pci@vger.kernel.org, rajatja@google.com
>>> > +Description: Total number of correctable errors seen and reported by
>>> > this
>>> > + PCI device using ERR_COR.
>>> > +
>>> > +Where:
>>> > /sys/bus/pci/devices/<dev>/aer_stats/dev_total_fatal_errs
>>> > +Date: May 2018
>>> > +Kernel Version: 4.17.0
>>> > +Contact: linux-pci@vger.kernel.org, rajatja@google.com
>>> > +Description: Total number of uncorrectable fatal errors seen and
>>> > reported
>>> > + by this PCI device using ERR_FATAL.
>>> > +
>>> > +Where:
>>> > /sys/bus/pci/devices/<dev>/aer_stats/dev_total_nonfatal_errs
>>> > +Date: May 2018
>>> > +Kernel Version: 4.17.0
>>> > +Contact: linux-pci@vger.kernel.org, rajatja@google.com
>>> > +Description: Total number of uncorrectable non-fatal errors seen and
>>> > reported
>>> > + by this PCI device using ERR_NONFATAL.
>>> > +
>>> > +Where:
>>> > /sys/bus/pci/devices/<dev>/aer_stats/dev_breakdown_correctable
>>> > +Date: May 2018
>>> > +Kernel Version: 4.17.0
>>> > +Contact: linux-pci@vger.kernel.org, rajatja@google.com
>>> > +Description: Breakdown of of correctable errors seen and reported by
>>> > this
>>> > + PCI device using ERR_COR. A sample result looks like
>>> > this:
>>> > +-----------------------------------------
>>> > +Receiver Error = 0x174
>>> > +Bad TLP = 0x19
>>> > +Bad DLLP = 0x3
>>> > +RELAY_NUM Rollover = 0x0
>>> > +Replay Timer Timeout = 0x1
>>> > +Advisory Non-Fatal = 0x0
>>> > +Corrected Internal Error = 0x0
>>> > +Header Log Overflow = 0x0
>>> > +-----------------------------------------
>>> why hex display ? decimal is easy to read as these are counters.
>>
>>
>> Have no particular preference. Since these can be potentially large
>> numbers, just had a random thought that hex might make it more
>> concise. I can change to decimal if that is preferable.
>>
>>> > +
>>> > +Where:
>>> > /sys/bus/pci/devices/<dev>/aer_stats/dev_breakdown_uncorrectable
>>> > +Date: May 2018
>>> > +Kernel Version: 4.17.0
>>> > +Contact: linux-pci@vger.kernel.org, rajatja@google.com
>>> > +Description: Breakdown of of correctable errors seen and reported by
>>> > this
>>> > + PCI device using ERR_FATAL or ERR_NONFATAL. A sample
>>> > result
>>> > + looks like this:
>>> > +-----------------------------------------
>>> > +Undefined = 0x0
>>> > +Data Link Protocol = 0x0
>>> > +Surprise Down Error = 0x0
>>> > +Poisoned TLP = 0x0
>>> > +Flow Control Protocol = 0x0
>>> > +Completion Timeout = 0x0
>>> > +Completer Abort = 0x0
>>> > +Unexpected Completion = 0x0
>>> > +Receiver Overflow = 0x0
>>> > +Malformed TLP = 0x0
>>> > +ECRC = 0x0
>>> > +Unsupported Request = 0x0
>>> > +ACS Violation = 0x0
>>> > +Uncorrectable Internal Error = 0x0
>>> > +MC Blocked TLP = 0x0
>>> > +AtomicOp Egress Blocked = 0x0
>>> > +TLP Prefix Blocked Error = 0x0
>>> > +-----------------------------------------
>>> > +
>>> > +============================
>>> > +PCIe Rootport AER statistics
>>> > +============================
>>> > +These attributes showup under only the rootports that are AER capable.
>>> > These
>>> > +indicate the number of error messages as "reported to" the rootport.
>>> > Please note
>>> > +that the rootports also transmit (internally) the ERR_* messages for
>>> > errors seen
>>> > +by the internal rootport PCI device, so these counters includes them
>>> > and are
>>> > +thus cumulative of all the error messages on the PCI hierarchy
>>> > originating
>>> > +at that root port.
>>>
>>> what about switches and bridges ?
>>
>>
>> What about them? AIUI, the switches forward the ERR_ messages from
>> downstream devices to the rootport, like they do with standard
>> messages. They can potentially generate their own ERR_ message and
>> that would be reported no different than other end point devices.
>
>
>
> yes, what I meant to ask is; the ERR_FATAL msg coming from EP, can be
> contained by switch
> and the error handling code thinks that, the error is contained by switch
> irrespective of
> AER or DPC, and it will think that the problem could be with Switch/bridge
> upstream link.
>
> hence the pci_dev of the switch where you should be increment your counters.
> of course ER_FATAL would have traversed till RP, but that doesnt meant that
> you account the error there.
In this case, for the pci_dev for the rootport:
- rootport_total_fatal_errors will be incremented (since it will get ERR_FATAL)
- dev_total_fatal_errors will not be incremented.
The dev_total_fatal_errors will be incremented only for the pci device
identified by the "Error Source Identification Register" in the PCIe
spec. Does this help clarify?
>
>
>>
>>> Also Can you give some idea as e.g what is the difference between
>>> dev_total_fatal_errs and rootport_total_fatal_errs (assuming that both
>>> are same pci_dev.
>>
>>
>> For a pci_dev representing the rootport:
>>
>> dev_total_fatal_errors = how many times this PCI device *experienced*
>> a fatal problem on its own (i.e. either link issues while talking to
>> its link partner, or some internal errors).
>>
>> rootport_total_fatal_errors = how many times this rootport was
>> *informed* about a problem (via ERR_* messages) in the PCI hierarchy
>> that originates at it (can be any link further downstream). This
>> includes the dev_total_fatal_errors also, because any errors detected
>> by the rootport are also "informed" to itself via ERR_* messages. In
>> reality, this is just the total number of ERR_FATAL messages received
>> at the rootport. This sysfs attribute will only exist for root ports.
>>
>>>
>>> rootport_total_fatal_errs gives me an idea that how many times things
>>> have been failed under this pci_dev ?
>>
>>
>> Yes, as above.
>>
>>> which means num of downstream link problems. but I am still trying to
>>> make sense as how it could be used,
>>> since we dont have BDF information associated with the number of errors
>>> anywhere (except these AER print messages)
>>>
>>
>> Agree. That is a limitation. The challenges being more record keeping,
>> more complicated sysfs representation, and given that PCI devices may
>> come and go, how do we know it is the same device before we collate
>> their stats etc.
>>
>>>
>>> and dev_total_fatal_errs as you mentioned above that problematic EP,
>>> then say root-port will report it and increment
>>> dev_total_fatal_errs ++
>>> does it also increment root-port_total_fatal_errs ++ in above scenario ?
>>
>>
>> Yes, as above, it will also root-port_total_fatal_errs++ for the root
>> port of that hierarchy.
>>
>> Thanks,
>>
>> Rajat
>>
>>>
>>> > +
>>> > +Where:
>>> > /sys/bus/pci/devices/<dev>/aer_stats/rootport_total_cor_errs
>>> > +Date: May 2018
>>> > +Kernel Version: 4.17.0
>>> > +Contact: linux-pci@vger.kernel.org, rajatja@google.com
>>> > +Description: Total number of ERR_COR messages reported to rootport.
>>> > +
>>> > +Where:
>>> > /sys/bus/pci/devices/<dev>/aer_stats/rootport_total_fatal_errs
>>> > +Date: May 2018
>>> > +Kernel Version: 4.17.0
>>> > +Contact: linux-pci@vger.kernel.org, rajatja@google.com
>>> > +Description: Total number of ERR_FATAL messages reported to rootport.
>>> > +
>>> > +Where:
>>> > /sys/bus/pci/devices/<dev>/aer_stats/rootport_total_nonfatal_errs
>>> > +Date: May 2018
>>> > +Kernel Version: 4.17.0
>>> > +Contact: linux-pci@vger.kernel.org, rajatja@google.com
>>> > +Description: Total number of ERR_NONFATAL messages reported to
>>> > rootport.
>>> > diff --git a/Documentation/PCI/pcieaer-howto.txt
>>> > b/Documentation/PCI/pcieaer-howto.txt
>>> > index acd0dddd6bb8..91b6e677cb8c 100644
>>> > --- a/Documentation/PCI/pcieaer-howto.txt
>>> > +++ b/Documentation/PCI/pcieaer-howto.txt
>>> > @@ -73,6 +73,11 @@ In the example, 'Requester ID' means the ID of the
>>> > device who sends
>>> > the error message to root port. Pls. refer to pci express specs for
>>> > other fields.
>>> >
>>> > +2.4 AER Statistics / Counters
>>> > +
>>> > +When PCIe AER errors are captured, the counters / statistics are also
>>> > exposed
>>> > +in form of sysfs attributes which are documented at
>>> > +Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
>>> >
>>> > 3. Developer Guide
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH -tip v6 26/27] Documentation: kprobes: Add how to change the execution path
From: Masami Hiramatsu @ 2018-06-19 16:16 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar
Cc: Masami Hiramatsu, Ingo Molnar, H . Peter Anvin, linux-kernel,
Ananth N Mavinakayanahalli, Andrew Morton, Steven Rostedt,
linux-arch, Jonathan Corbet, linux-doc
In-Reply-To: <152942424698.15209.15245996287444292393.stgit@devbox>
Add a section that explaining how to change the execution
path with kprobes and warnings for some arch.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
---
Documentation/kprobes.txt | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
index 3e9e99ea751b..8a98eed1521b 100644
--- a/Documentation/kprobes.txt
+++ b/Documentation/kprobes.txt
@@ -80,6 +80,26 @@ After the instruction is single-stepped, Kprobes executes the
"post_handler," if any, that is associated with the kprobe.
Execution then continues with the instruction following the probepoint.
+Changing Execution Path
+-----------------------
+
+Since the kprobes can probe into a running kernel code, it can change
+the register set, including instruction pointer. This operation
+requires maximum attention, such as keeping the stack frame, recovering
+execution path etc. Since it is operated on running kernel and need deep
+knowladge of the archtecture and concurrent computing, you can easily
+shot your foot.
+
+If you change the instruction pointer (and set up other related
+registers) in pre_handler, you must return !0 so that the kprobes
+stops single stepping and just returns to given address.
+This also means post_handler should not be called anymore.
+
+Note that this operation may be harder on some architectures which
+use TOC (Table of Contents) for function call, since you have to
+setup new TOC for your function in your module, and recover old
+one after back from it.
+
Return Probes
-------------
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH -tip v6 25/27] kprobes/x86: Do not disable preempt on int3 path
From: Masami Hiramatsu @ 2018-06-19 16:16 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar
Cc: Masami Hiramatsu, Ingo Molnar, H . Peter Anvin, linux-kernel,
Ananth N Mavinakayanahalli, Andrew Morton, Steven Rostedt,
linux-arch, x86, linux-doc
In-Reply-To: <152942424698.15209.15245996287444292393.stgit@devbox>
Since int3 and debug exception(for singlestep) are run with
IRQ disabled and while running single stepping we drop IF
from regs->flags, that path must not be preemptible. So we
can remove the preempt disable/enable calls from that path.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Suggested-by: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: linux-doc@vger.kernel.org
---
Changes in v3:
- Split user-side changes to another patch
Changes in v2:
- Include user-side changes.
---
Documentation/kprobes.txt | 11 +++++------
arch/x86/kernel/kprobes/core.c | 18 ++++--------------
arch/x86/kernel/kprobes/opt.c | 1 -
3 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
index 907a3017c0f2..3e9e99ea751b 100644
--- a/Documentation/kprobes.txt
+++ b/Documentation/kprobes.txt
@@ -566,12 +566,11 @@ the same handler) may run concurrently on different CPUs.
Kprobes does not use mutexes or allocate memory except during
registration and unregistration.
-Probe handlers are run with preemption disabled. Depending on the
-architecture and optimization state, handlers may also run with
-interrupts disabled (e.g., kretprobe handlers and optimized kprobe
-handlers run without interrupt disabled on x86/x86-64). In any case,
-your handler should not yield the CPU (e.g., by attempting to acquire
-a semaphore).
+Probe handlers are run with preemption disabled or interrupt disabled,
+which depends on the architecture and optimization state. (e.g.,
+kretprobe handlers and optimized kprobe handlers run without interrupt
+disabled on x86/x86-64). In any case, your handler should not yield
+the CPU (e.g., by attempting to acquire a semaphore, or waiting I/O).
Since a return probe is implemented by replacing the return
address with the trampoline's address, stack backtraces and calls
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 814e26b7c8a2..f7104b256de7 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -594,7 +594,6 @@ static void setup_singlestep(struct kprobe *p, struct pt_regs *regs,
* stepping.
*/
regs->ip = (unsigned long)p->ainsn.insn;
- preempt_enable_no_resched();
return;
}
#endif
@@ -667,12 +666,10 @@ int kprobe_int3_handler(struct pt_regs *regs)
addr = (kprobe_opcode_t *)(regs->ip - sizeof(kprobe_opcode_t));
/*
- * We don't want to be preempted for the entire
- * duration of kprobe processing. We conditionally
- * re-enable preemption at the end of this function,
- * and also in reenter_kprobe() and setup_singlestep().
+ * We don't want to be preempted for the entire duration of kprobe
+ * processing. Since int3 and debug trap disables irqs and we clear
+ * IF while singlestepping, it must be no preemptible.
*/
- preempt_disable();
kcb = get_kprobe_ctlblk();
p = get_kprobe(addr);
@@ -694,10 +691,8 @@ int kprobe_int3_handler(struct pt_regs *regs)
*/
if (!p->pre_handler || !p->pre_handler(p, regs))
setup_singlestep(p, regs, kcb, 0);
- else {
+ else
reset_current_kprobe();
- preempt_enable_no_resched();
- }
return 1;
}
} else if (*addr != BREAKPOINT_INSTRUCTION) {
@@ -711,11 +706,9 @@ int kprobe_int3_handler(struct pt_regs *regs)
* the original instruction.
*/
regs->ip = (unsigned long)addr;
- preempt_enable_no_resched();
return 1;
} /* else: not a kprobe fault; let the kernel handle it */
- preempt_enable_no_resched();
return 0;
}
NOKPROBE_SYMBOL(kprobe_int3_handler);
@@ -966,8 +959,6 @@ int kprobe_debug_handler(struct pt_regs *regs)
}
reset_current_kprobe();
out:
- preempt_enable_no_resched();
-
/*
* if somebody else is singlestepping across a probe point, flags
* will have TF set, in which case, continue the remaining processing
@@ -1014,7 +1005,6 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
restore_previous_kprobe(kcb);
else
reset_current_kprobe();
- preempt_enable_no_resched();
} else if (kcb->kprobe_status == KPROBE_HIT_ACTIVE ||
kcb->kprobe_status == KPROBE_HIT_SSDONE) {
/*
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 203d398802a3..eaf02f2e7300 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -491,7 +491,6 @@ int setup_detour_execution(struct kprobe *p, struct pt_regs *regs, int reenter)
regs->ip = (unsigned long)op->optinsn.insn + TMPL_END_IDX;
if (!reenter)
reset_current_kprobe();
- preempt_enable_no_resched();
return 1;
}
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH -tip v6 01/27] Documentation/kprobes: Fix to remove remaining jprobe
From: Masami Hiramatsu @ 2018-06-19 16:04 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar
Cc: Masami Hiramatsu, Ingo Molnar, H . Peter Anvin, linux-kernel,
Ananth N Mavinakayanahalli, Andrew Morton, Steven Rostedt,
linux-arch, Jonathan Corbet, linux-doc
In-Reply-To: <152942424698.15209.15245996287444292393.stgit@devbox>
Remove jps from the document, since jprobe is removed.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
---
Documentation/kprobes.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
index 22208bf2386d..5ae80baf3921 100644
--- a/Documentation/kprobes.txt
+++ b/Documentation/kprobes.txt
@@ -474,7 +474,7 @@ error occurs during registration, all probes in the array, up to
the bad probe, are safely unregistered before the register_*probes
function returns.
-- kps/rps/jps: an array of pointers to ``*probe`` data structures
+- kps/rps: an array of pointers to ``*probe`` data structures
- num: the number of the array entries.
.. note::
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
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