* Re: [FSL P5020 P5040 PPC] Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates
From: Michael Ellerman @ 2020-01-28 11:55 UTC (permalink / raw)
To: Christian Zigotzky, Ulf Hansson
Cc: Darren Stevens, mad skateman, linux-mmc@vger.kernel.org,
Julian Margetson, Christoph Hellwig, Rob Herring,
contact@a-eon.com, R.T.Dickinson, linuxppc-dev
In-Reply-To: <de35b0fc-9e24-2989-58f6-ade0585db1a0@xenosoft.de>
Christian Zigotzky <chzigotzky@xenosoft.de> writes:
> On 24 January 2020 at 12:42 pm, Michael Ellerman wrote:
>> Ulf Hansson <ulf.hansson@linaro.org> writes:
>>> On Thu, 16 Jan 2020 at 12:18, Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
>>>> Hi All,
>>>>
>>>> We still need the attached patch for our onboard SD card interface
>>>> [1,2]. Could you please add this patch to the tree?
>>> No, because according to previous discussion that isn't the correct
>>> solution and more importantly it will break other archs (if I recall
>>> correctly).
>>>
>>> Looks like someone from the ppc community needs to pick up the ball.
>> That's a pretty small community these days :) :/
>>
>> Christian can you test this please? I think I got the polarity of all
>> the tests right, but it's Friday night so maybe I'm wrong :)
>>
>> cheers
> Michael,
>
> Thanks a lot for the new patch! I compiled the RC7 of kernel 5.5 with
> your patch again yesterday and the kernel works without any problems
> with our onboard SD cards. [1]
Thanks for testing.
cheers
^ permalink raw reply
* powerpc Linux scv support and scv system call ABI proposal
From: Nicholas Piggin @ 2020-01-28 10:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Tulio Magno Quites Machado Filho, libc-alpha
I would like to enable support for the scv instruction to provide the Linux
system calls.
This requires two things to be defined, firstly how to advertise support
for scv and how to allocate and advertise support for individual scv
vectors. Secondly, how to define a Linux system call ABI with this new
instruction.
I have put together a rough proposal along with some options and
questions. Any thoughts or input would be welcome, I have probably
missed some things so please point them out.
(I will be on vacation for two weeks from the end of the week, may not
get to replying immediately)
Thanks,
Nick
System Call Vectored (scv) ABI
The scv instruction is introduced with POWER9 / ISA3, it comes with an
rfscv counter-part. The benefit of these instructions is performance
(trading slower SRR0/1 with faster LR/CTR registers, and entering the
kernel with MSR[EE] and MSR[RI] left enabled, which can avoid one mtmsrd
instruction. Another benefit is that the ABI can be changed if there is
a good reason to.
The scv instruction has 128 interrupt entry points (not enough to cover
the Linux system call space). The proposal is to assign scv numbers
conservatively. 'scv 0' could be used for the regular Linux system call
ABI initially. Examples of other assignments could be 32-bit compat
system calls, and firmware service calls.
Linux has not enabled FSCR[SCV] yet, so the instruction will trap with illegal
instruction on current environments. Linux has defined a HWCAP2 bit
PPC_FEATURE2_SCV for SCV support, but does not set it.
One option is for PPC_FEATURE2_SCV to indicate 'scv 0' support, and a new HWCAP
bit assigned for each new scv vector supported for userspace. This is the most
regular and flexible approach. It requires the most HWCAP space, but vector
usage is not expected to grow quickly.
Another option is for PPC_FEATURE2_SCV to indicate 'scv 0', and other vectors
will each return -ENOSYS, then when they are assigned to a new ABI, it will
define a particular way they can be queried for support (which would return
something other than -ENOSYS if supported). This will not require more HWCAP
bits, but it's less regular and more complicated to determine.
* Proposal is for PPC_FEATURE2_SCV to indicate 'scv 0' support, all other
vectors will return -ENOSYS, and the decision for how to add support for
a new vector deferred until we see the next user.
* Proposal is for scv 0 to provide the standard Linux system call ABI with some
differences:
- LR is volatile across scv calls. This is necessary for support because the
scv instruction clobbers LR.
- CR1 and CR5-CR7 are volatile. This matches the C ABI and would allow the
system call exit to avoid restoring the CR register.
- Error handling: use of CR0[SO] to indicate error requires a mtcr / mtocr
instruction on the kernel side, and it is currently not implemented well
in glibc, requiring a mfcr (mfocr should be possible and asm goto support
would allow a better implementation). Is it worth continuing this style of
error handling? Or just move to -ve return means error? Using a different
bit would allow the kernel to piggy back the CR return code setting with
a test for the error case exit.
- R2 could be volatile as though it's an external function call, which
would avoid one store in the system call entry path. However it would
require the caller to load R2 after the system call returns, where the
latency of the load can not be overlapped with the costly system call
exit sequence. On balance, it may be better to keep R2 as non-volatile.
- Number of volatile registers available seems sufficient. Linux's 'sc'
handler is badly constrained here, but that is because it is shared
between both hypercall and syscall handlers, which have different
call conventions that share no volatile GPR registers! r9-r12 should
be quite enough.
- Should this be for 64-bit only? 'scv 1' could be reserved for 32-bit
calls if there was interest in developing an ABI for 32-bit programs.
Marginal benefit in avoiding compat syscall selection.
^ permalink raw reply
* Re: [PATCH] powerpc: drmem: avoid NULL pointer dereference when drmem is unavailable
From: Libor Pechacek @ 2020-01-28 10:19 UTC (permalink / raw)
To: Nathan Lynch
Cc: David Hildenbrand, Michal Suchanek, linux-kernel, Paul Mackerras,
Leonardo Bras, Thomas Gleixner, linuxppc-dev, Allison Randal
In-Reply-To: <87o8uudv51.fsf@linux.ibm.com>
Hello Nathan,
On Thu 23-01-20 09:56:10, Nathan Lynch wrote:
> Libor Pechacek <lpechacek@suse.cz> writes:
> > In KVM guests drmem structure is only zero initialized. Trying to
> > manipulate DLPAR parameters results in a crash in this environment.
>
> I think this statement needs qualification. Unless I'm mistaken, this
> happens only when you boot a guest without any hotpluggable memory
> configured, and then try to add or remove memory.
Thanks for the review. The introductory statement can indeed be clearer.
[...]
> > diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> > index c126b94d1943..4ea6af002e27 100644
> > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> > +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> > @@ -236,9 +236,9 @@ static int get_lmb_range(u32 drc_index, int n_lmbs,
> > if (!start)
> > return -EINVAL;
> >
> > - end = &start[n_lmbs - 1];
> > + end = &start[n_lmbs];
> >
> > - last_lmb = &drmem_info->lmbs[drmem_info->n_lmbs - 1];
> > + last_lmb = &drmem_info->lmbs[drmem_info->n_lmbs];
> > if (end > last_lmb)
> > return -EINVAL;
>
> Is this not undefined behavior? I'd rather do this in a way that does
> not involve forming out-of-bounds pointers.
Well, this is a tough question for the case when drmem_info->lmbs was not
allocated. Given that the array does not exist, what bounds are we talking
about?
My patch builds on the fact that NULL[0] is NULL and NULL < NULL is false.
Talking about a pointer to one past the last element of an non-existent array
is too much philosophy for me.
For the case when drmem_info->lmbs is allocated, last_lmb is a pointer to one
past the last element of the array as Michal mentioned.
> Even if it's safe, naming that pointer "last_lmb" now actively hinders
> understanding of the code; it should be named "limit" or something.
Good catch.
[...]
> 1 file changed, 36 insertions(+), 7 deletions(-)
> arch/powerpc/include/asm/drmem.h | 43 +++++++++++++++++++++++++++++++++-------
>
> modified arch/powerpc/include/asm/drmem.h
> @@ -20,19 +20,48 @@ struct drmem_lmb {
>
> struct drmem_lmb_info {
> struct drmem_lmb *lmbs;
> - int n_lmbs;
> + unsigned int n_lmbs;
> u32 lmb_size;
> };
>
> extern struct drmem_lmb_info *drmem_info;
>
> -#define for_each_drmem_lmb_in_range(lmb, start, end) \
> - for ((lmb) = (start); (lmb) <= (end); (lmb)++)
> +static inline bool drmem_present(void)
> +{
> + return drmem_info->lmbs != NULL;
> +}
Yes, use of this test was also my first idea about the fix.
> +static inline struct drmem_lmb *drmem_lmb_index(unsigned int index)
> +{
> + if (!drmem_present())
> + return NULL;
>
> -#define for_each_drmem_lmb(lmb) \
> - for_each_drmem_lmb_in_range((lmb), \
> - &drmem_info->lmbs[0], \
> - &drmem_info->lmbs[drmem_info->n_lmbs - 1])
> + if (WARN_ON(index >= drmem_info->n_lmbs))
> + return NULL;
Why is this WARN_ON needed?
> +
> + return &drmem_info->lmbs[index];
> +}
> +
> +static inline struct drmem_lmb *drmem_first_lmb(void)
> +{
> + return drmem_lmb_index(0);
> +}
> +
> +static inline struct drmem_lmb *drmem_last_lmb(void)
> +{
> + if (!drmem_present())
> + return NULL;
> +
> + return drmem_lmb_index(drmem_info->n_lmbs - 1);
Is the unsigned integer wraparound intended in drmem_info->n_lmbs == 0 case?
> +}
> +
> +#define for_each_drmem_lmb(lmb) \
> + for ((lmb) = drmem_first_lmb(); \
drmem_first_lmb() is essentially a call to drmem_info->lmbs(0). What
happens if drmem_info->n_lmbs is zero and drmem_info->lmbs is not NULL?
> + (lmb) != NULL && (lmb) <= drmem_last_lmb(); \
> + (lmb)++)
> +
> +#define for_each_drmem_lmb_in_range(lmb, start, end) \
> + for ((lmb) = (start); (lmb) <= (end); (lmb)++)
>
> /*
> * The of_drconf_cell_v1 struct defines the layout of the LMB data
>
After all, I don't mind how the bug will be fixed. As you can see, my
preference is towards simpler solutions.
In my opinion your solution special-cased drmem_info->lmbs == NULL and opened
the doorway to the combination of drmem_info->lmbs != NULL &&
!drmem_info->n_lmbs. Maybe the condition can never become true but the code
should IMHO be robust enough to handle it.
Thanks!
Libor
--
Libor Pechacek
SUSE Labs Remember to have fun...
^ permalink raw reply
* Re: [PATCH] powerpc/64: system call implement the bulk of the logic in C fix
From: Michal Suchánek @ 2020-01-28 9:41 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <1580171821.hhr2i98lkm.astroid@bobo.none>
On Tue, Jan 28, 2020 at 10:41:02AM +1000, Nicholas Piggin wrote:
> Michal Suchánek's on January 28, 2020 4:08 am:
> > On Tue, Jan 28, 2020 at 12:17:12AM +1000, Nicholas Piggin wrote:
> >> This incremental patch fixes several soft-mask debug and unsafe
> >> smp_processor_id messages due to tracing and false positives in
> >> "unreconciled" code.
> >>
> >> It also fixes a bug with syscall tracing functions that set registers
> >> (e.g., PTRACE_SETREG) not setting GPRs properly.
> >>
> >> There was a bug reported with the TM selftests, I haven't been able
> >> to reproduce that one.
> >>
> >> I can squash this into the main patch and resend the series if it
> >> helps but the incremental helps to see the bug fixes.
> >
> > There are some whitespace differences between this and the series I have
> > applied locally. What does it apply to?
> >
> > Is there some revision of the patchset I missed?
>
> No I may have just missed some of your whitespace cleanups, or maybe I got
> some that Michael made which you don't have in his next-test branch.
Looks like the latter. I will pick patches from next-test.
Thanks
Michal
^ permalink raw reply
* Re: [PATCH v16 00/23] selftests, powerpc, x86: Memory Protection Keys
From: Sandipan Das @ 2020-01-28 9:38 UTC (permalink / raw)
To: Dave Hansen
Cc: linux-arch, fweimer, aneesh.kumar, x86, linuxram, linuxppc-dev,
mhocko, linux-mm, mingo, linux-kselftest, msuchanek, shuah,
bauerman
In-Reply-To: <3eca7a91-aa3e-cb01-47c8-5d36020993a2@intel.com>
Hi Dave,
On 27/01/20 9:12 pm, Dave Hansen wrote:
>
> How have you tested this patch (and the whole series for that matter)?
>
I replaced the second patch with this one and did a build test.
Till v16, I had tested the whole series (build + run) on both a POWER8
system (with 4K and 64K page sizes) and a Skylake SP system but for
x86_64 only. Following that, I could only do a build test locally on
my laptop for i386 and x86_64 on my laptop as I did not have access to
the Skylake system anymore.
This is how I tested the build process:
$ cd linux
$ make -C tools/testing/selftests
...
make[1]: Entering directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
...
gcc -Wall -I ../../../../usr/include -no-pie -m32 protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
gcc -Wall -I ../../../../usr/include -no-pie -m64 protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
...
$ make -C tools/testing/selftests clean
$ make -C tools/testing/selftests/vm
make: Entering directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
make --no-builtin-rules ARCH=x86_64 -C ../../../.. headers_install
make[1]: Entering directory '/home/sandipan/.devel/linux'
INSTALL ./usr/include
make[1]: Leaving directory '/home/sandipan/.devel/linux'
...
gcc -Wall -I ../../../../usr/include -no-pie -m32 protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
gcc -Wall -I ../../../../usr/include -no-pie -m64 protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
...
$ make -C tools/testing/selftests/vm clean
$ make -C tools/testing/selftests/vm protection_keys
make: Entering directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
gcc -Wall -I ../../../../usr/include -no-pie -m32 protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
gcc -Wall -I ../../../../usr/include -no-pie -m64 protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
make: Leaving directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
$ make -C tools/testing/selftests/vm clean
$ make -C tools/testing/selftests/vm protection_keys_32
make: Entering directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
gcc -Wall -I ../../../../usr/include -no-pie -m32 protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
make: Leaving directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
$ make -C tools/testing/selftests/vm protection_keys_64
make: Entering directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
gcc -Wall -I ../../../../usr/include -no-pie -m64 protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
make: Leaving directory '/home/sandipan/.devel/linux/tools/testing/selftests/vm'
$ make -C tools/testing/selftests/vm clean
$ cd tools/testing/selftests/vm
$ make
make --no-builtin-rules ARCH=x86_64 -C ../../../.. headers_install
make[1]: Entering directory '/home/sandipan/.devel/linux'
INSTALL ./usr/include
make[1]: Leaving directory '/home/sandipan/.devel/linux'
...
gcc -Wall -I ../../../../usr/include -no-pie -m32 protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
gcc -Wall -I ../../../../usr/include -no-pie -m64 protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
...
$ make clean
$ make protection_keys
gcc -Wall -I ../../../../usr/include -no-pie -m32 protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
gcc -Wall -I ../../../../usr/include -no-pie -m64 protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
$ make clean
$ make protection_keys_32
gcc -Wall -I ../../../../usr/include -no-pie -m32 protection_keys.c -lrt -lrt -ldl -lm -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_32
$ make protection_keys_64
gcc -Wall -I ../../../../usr/include -no-pie -m64 protection_keys.c -lrt -lrt -ldl -o /home/sandipan/.devel/linux/tools/testing/selftests/vm/protection_keys_64
- Sandipan
^ permalink raw reply
* Re: vmlinux ELF header sometimes corrupt
From: Rasmus Villemoes @ 2020-01-28 8:12 UTC (permalink / raw)
To: LKML; +Cc: linux-ext4, linuxppc-dev@lists.ozlabs.org,
Linux Kbuild mailing list
In-Reply-To: <71aa76d0-a3b8-b4f3-a7c3-766cfb75412f@rasmusvillemoes.dk>
On 22/01/2020 18.52, Rasmus Villemoes wrote:
> I'm building for a ppc32 (mpc8309) target using Yocto, and I'm hitting a
> very hard to debug problem that maybe someone else has encountered. This
> doesn't happen always, perhaps 1 in 8 times or something like that.
>
> The issue is that when the build gets to do "${CROSS}objcopy -O binary
> ... vmlinux", vmlinux is not (no longer) a proper ELF file, so naturally
> that fails with
>
> powerpc-oe-linux-objcopy:vmlinux: file format not recognized
>
> So I hacked link-vmlinux.sh to stash copies of vmlinux before and after
> sortextable vmlinux. Both of those are proper ELF files, and comparing
> the corrupted vmlinux to vmlinux.after_sort they are identical after the
> first 52 bytes; in vmlinux, those first 52 bytes are all 0.
>
> I also saved stat(1) info to see if vmlinux is being replaced or
> modified in-place.
>
> $ cat vmlinux.stat.after_sort
> File: 'vmlinux'
> Size: 8608456 Blocks: 16696 IO Block: 4096 regular file
> Device: 811h/2065d Inode: 21919132 Links: 1
> Access: (0755/-rwxr-xr-x) Uid: ( 1000/ user) Gid: ( 1001/ user)
> Access: 2020-01-22 10:52:38.946703081 +0000
> Modify: 2020-01-22 10:52:38.954703105 +0000
> Change: 2020-01-22 10:52:38.954703105 +0000
>
> $ stat vmlinux
> File: 'vmlinux'
> Size: 8608456 Blocks: 16688 IO Block: 4096 regular file
> Device: 811h/2065d Inode: 21919132 Links: 1
> Access: (0755/-rwxr-xr-x) Uid: ( 1000/ user) Gid: ( 1001/ user)
> Access: 2020-01-22 17:20:00.650379057 +0000
> Modify: 2020-01-22 10:52:38.954703105 +0000
> Change: 2020-01-22 10:52:38.954703105 +0000
>
> So the inode number and mtime/ctime are exactly the same, but for some
> reason Blocks: has changed? This is on an ext4 filesystem, but I don't
> suspect the filesystem to be broken, because it's always just vmlinux
> that ends up corrupt, and always in exactly this way with the first 52
> bytes having been wiped.
So, I think I take that last part back. I just hit a case where I built
the kernel manually, made a copy of vmlinux to vmlinux.copy, and file(1)
said both were fine (and cmp(1) agreed they were identical). Then I went
off and did work elsewhere with a lot of I/O. When I came back to the
linux build dir, vmlinux was broken, exactly as before. So I now suspect
it to be some kind of "while the file is in the pagecache, everything is
fine, but when it's read back from disk it's broken".
My ext4 fs does have inline_data enabled, which could explain why the
corruption happens in the beginning. It's just very odd that it only
ever seems to trigger for vmlinux and not other files, but perhaps the
I/O patterns that ld and/or sortextable does are exactly what are needed
to trigger the bug.
I've done a long overdue kernel update, and there are quite a few
fs/ext4/ -stable patches in there, so now I'll see if it still happens.
And if anything more comes of this, I'll remove the kbuild and ppc lists
from cc, sorry for the noise.
Rasmus
^ permalink raw reply
* Re: [PASEMI PA6T PPC] Onboard CF card device with new SanDisk High (>8G) CF cards
From: Christoph Hellwig @ 2020-01-28 8:08 UTC (permalink / raw)
To: Christian Zigotzky
Cc: Ulf Hansson, Darren Stevens, R.T.Dickinson,
linux-mmc@vger.kernel.org, Julian Margetson, Christoph Hellwig,
Rob Herring, contact@a-eon.com, mad skateman, linuxppc-dev
In-Reply-To: <688f1943-5b81-6200-d4d9-38d21b69e180@xenosoft.de>
On Tue, Jan 28, 2020 at 08:58:29AM +0100, Christian Zigotzky wrote:
> Hi All,
>
> Which mailing list is responsible for the pata_pcmcia driver? We are using
> new SanDisk High (>8G) CF cards with this driver [1] and we need the
> following line in the file "drivers/ata/pata_pcmcia.c".
>
> + PCMCIA_DEVICE_MANF_CARD(0x00f1, 0x0101), /* SanDisk High (>8G)
> CFA */
Please send a formal patch for it to linux-ide@vger.kernel.org
^ permalink raw reply
* [PASEMI PA6T PPC] Onboard CF card device with new SanDisk High (>8G) CF cards
From: Christian Zigotzky @ 2020-01-28 7:58 UTC (permalink / raw)
To: Michael Ellerman, Ulf Hansson, Darren Stevens, mad skateman,
linux-mmc@vger.kernel.org, Julian Margetson, contact@a-eon.com,
R.T.Dickinson, linuxppc-dev, Rob Herring, Christoph Hellwig
In-Reply-To: <8736c5hyhr.fsf@mpe.ellerman.id.au>
Hi All,
Which mailing list is responsible for the pata_pcmcia driver? We are
using new SanDisk High (>8G) CF cards with this driver [1] and we need
the following line in the file "drivers/ata/pata_pcmcia.c".
+ PCMCIA_DEVICE_MANF_CARD(0x00f1, 0x0101), /* SanDisk High
(>8G) CFA */
Thanks,
Christian
[1] https://forum.hyperion-entertainment.com/viewtopic.php?f=35&t=4282
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Qian Cai @ 2020-01-28 7:12 UTC (permalink / raw)
To: Christophe Leroy
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Linux-MM,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390, x86,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Jason Gunthorpe, Gerald Schaefer, linux-snps-arc, Ingo Molnar,
Kees Cook, Anshuman Khandual, Masahiro Yamada, Mark Brown,
Kirill A . Shutemov, Dan Williams, Vlastimil Babka,
linux-arm-kernel, Sri Krishna chowdary, Ard Biesheuvel,
Greg Kroah-Hartman, Dave Hansen, linux-mips, Ralf Baechle,
linux-kernel, Paul Burton, Mike Rapoport, Vineet Gupta,
Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <a7ba6d8a-6443-5994-6a34-2824aa9b054b@c-s.fr>
> On Jan 28, 2020, at 1:13 AM, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
> ppc32 an indecent / legacy platform ? Are you kidying ?
>
> Powerquicc II PRO for instance is fully supported by the manufacturer and widely used in many small networking devices.
Of course I forgot about embedded devices. The problem is that how many developers are actually going to run this debug option on embedded devices?
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Qian Cai @ 2020-01-28 7:07 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Linux-MM,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390, x86,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Jason Gunthorpe, Gerald Schaefer, linux-snps-arc,
linux-arm-kernel, Ingo Molnar, Kees Cook, Masahiro Yamada,
Mark Brown, Kirill A . Shutemov, Dan Williams, Vlastimil Babka,
Sri Krishna chowdary, Ard Biesheuvel, Greg Kroah-Hartman,
Dave Hansen, linux-mips, Ralf Baechle, linux-kernel, Paul Burton,
Mike Rapoport, Vineet Gupta, Martin Schwidefsky, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <ba5a836a-b594-f846-1734-71dcd5d089e6@arm.com>
> On Jan 28, 2020, at 2:03 AM, Anshuman Khandual <Anshuman.Khandual@arm.com> wrote:
>
> 'allyesconfig' makes 'DEBUG_VM = y' which in turn will enable 'DEBUG_VM_PGTABLE = y'
> on platforms that subscribe ARCH_HAS_DEBUG_VM_PGTABLE.
Isn’t that only for compiling testing? Who is booting such a beast and make sure everything working as expected?
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Anshuman Khandual @ 2020-01-28 7:03 UTC (permalink / raw)
To: Qian Cai, Christophe Leroy
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Linux-MM, Dave Hansen,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390, x86,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Jason Gunthorpe, Gerald Schaefer, linux-snps-arc, Ingo Molnar,
Kees Cook, Masahiro Yamada, Mark Brown, Kirill A . Shutemov,
Dan Williams, Vlastimil Babka, linux-arm-kernel,
Sri Krishna chowdary, Ard Biesheuvel, Greg Kroah-Hartman,
linux-mips, Ralf Baechle, linux-kernel, Paul Burton,
Mike Rapoport, Vineet Gupta, Martin Schwidefsky, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <F90DA0AA-4D27-4346-8D8D-D9A7871E2C07@lca.pw>
On 01/28/2020 12:06 PM, Qian Cai wrote:
>
>
>> On Jan 28, 2020, at 1:17 AM, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>
>> It is 'default y' so there is no much risk that it is forgotten, at least all test suites run with 'allyes_defconfig' will trigger the test, so I think it is really a good feature.
>
> This thing depends on DEBUG_VM which I don’t see it is selected by any defconfig. Am I missing anything?
>
'allyesconfig' makes 'DEBUG_VM = y' which in turn will enable 'DEBUG_VM_PGTABLE = y'
on platforms that subscribe ARCH_HAS_DEBUG_VM_PGTABLE.
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Qian Cai @ 2020-01-28 6:36 UTC (permalink / raw)
To: Christophe Leroy
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Linux-MM,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390, x86,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Jason Gunthorpe, Gerald Schaefer, linux-snps-arc, Ingo Molnar,
Kees Cook, Anshuman Khandual, Masahiro Yamada, Mark Brown,
Kirill A . Shutemov, Dan Williams, Vlastimil Babka,
linux-arm-kernel, Sri Krishna chowdary, Ard Biesheuvel,
Greg Kroah-Hartman, Dave Hansen, linux-mips, Ralf Baechle,
linux-kernel, Paul Burton, Mike Rapoport, Vineet Gupta,
Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <115c187b-73ce-30b2-0694-999db1f2183b@c-s.fr>
> On Jan 28, 2020, at 1:17 AM, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
> It is 'default y' so there is no much risk that it is forgotten, at least all test suites run with 'allyes_defconfig' will trigger the test, so I think it is really a good feature.
This thing depends on DEBUG_VM which I don’t see it is selected by any defconfig. Am I missing anything?
^ permalink raw reply
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Christophe Leroy @ 2020-01-28 6:17 UTC (permalink / raw)
To: Qian Cai, Anshuman Khandual
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Linux-MM, Dave Hansen,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390, x86,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Jason Gunthorpe, Gerald Schaefer, linux-snps-arc, Ingo Molnar,
Kees Cook, Masahiro Yamada, Mark Brown, Kirill A . Shutemov,
Dan Williams, Vlastimil Babka, linux-arm-kernel,
Sri Krishna chowdary, Ard Biesheuvel, Greg Kroah-Hartman,
linux-mips, Ralf Baechle, linux-kernel, Paul Burton,
Mike Rapoport, Vineet Gupta, Martin Schwidefsky, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <EE5B936C-121E-4322-A285-603D82D59B3A@lca.pw>
Le 28/01/2020 à 06:48, Qian Cai a écrit :
>
>
>> On Jan 27, 2020, at 11:58 PM, Anshuman Khandual <Anshuman.Khandual@arm.com> wrote:
>>
>> As I had mentioned before, the test attempts to formalize page table helper semantics
>> as expected from generic MM code paths and intend to catch deviations when enabled on
>> a given platform. How else should we test semantics errors otherwise ? There are past
>> examples of usefulness for this procedure on arm64 and on s390. I am wondering how
>> else to prove the usefulness of a debug feature if these references are not enough.
>
> Not saying it will not be useful. As you mentioned it actually found a bug or two in the past. The problem is that there is always a cost to maintain something like this, and nobody knew how things could be broken even for the isolated code you mentioned in the future given how complicated the kernel code base is. I am not so positive that many developers would enable this debug feature and use it on a regular basis from the information you gave so far.
>
> On the other hand, it might just be good at maintaining this thing out of tree by yourself anyway, because if there isn’t going to be used by many developers, few people is going to contribute to this and even noticed when it is broken. What’s the point of getting this merged apart from being getting some meaningless credits?
>
It is 'default y' so there is no much risk that it is forgotten, at
least all test suites run with 'allyes_defconfig' will trigger the test,
so I think it is really a good feature.
Christophe
^ permalink raw reply
* [PATCH v6 10/10] drivers/oprofile: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:14 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring for CAP_PERFMON privileged process. Providing
the access under CAP_PERFMON capability singly, without the rest of
CAP_SYS_ADMIN credentials, excludes chances to misuse the credentials and
makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to the monitoring remains open
for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
monitoring is discouraged with respect to CAP_PERFMON capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
drivers/oprofile/event_buffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/oprofile/event_buffer.c b/drivers/oprofile/event_buffer.c
index 12ea4a4ad607..6c9edc8bbc95 100644
--- a/drivers/oprofile/event_buffer.c
+++ b/drivers/oprofile/event_buffer.c
@@ -113,7 +113,7 @@ static int event_buffer_open(struct inode *inode, struct file *file)
{
int err = -EPERM;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EPERM;
if (test_and_set_bit_lock(0, &buffer_opened))
--
2.20.1
^ permalink raw reply related
* [PATCH v6 09/10] drivers/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:13 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to the monitoring remains open
for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
monitoring is discouraged with respect to CAP_PERFMON capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
drivers/perf/arm_spe_pmu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index 4e4984a55cd1..5dff81bc3324 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -274,7 +274,7 @@ static u64 arm_spe_event_to_pmscr(struct perf_event *event)
if (!attr->exclude_kernel)
reg |= BIT(SYS_PMSCR_EL1_E1SPE_SHIFT);
- if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && capable(CAP_SYS_ADMIN))
+ if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && perfmon_capable())
reg |= BIT(SYS_PMSCR_EL1_CX_SHIFT);
return reg;
@@ -700,7 +700,7 @@ static int arm_spe_pmu_event_init(struct perf_event *event)
return -EOPNOTSUPP;
reg = arm_spe_event_to_pmscr(event);
- if (!capable(CAP_SYS_ADMIN) &&
+ if (!perfmon_capable() &&
(reg & (BIT(SYS_PMSCR_EL1_PA_SHIFT) |
BIT(SYS_PMSCR_EL1_CX_SHIFT) |
BIT(SYS_PMSCR_EL1_PCT_SHIFT))))
--
2.20.1
^ permalink raw reply related
* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Christophe Leroy @ 2020-01-28 6:13 UTC (permalink / raw)
To: Qian Cai, Anshuman Khandual
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Linux-MM, Dave Hansen,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390, x86,
Russell King - ARM Linux, Matthew Wilcox, Steven Price,
Jason Gunthorpe, Gerald Schaefer, linux-snps-arc, Ingo Molnar,
Kees Cook, Masahiro Yamada, Mark Brown, Kirill A . Shutemov,
Dan Williams, Vlastimil Babka, linux-arm-kernel,
Sri Krishna chowdary, Ard Biesheuvel, Greg Kroah-Hartman,
linux-mips, Ralf Baechle, linux-kernel, Paul Burton,
Mike Rapoport, Vineet Gupta, Martin Schwidefsky, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <016A776F-EFD9-4D2B-A3A9-788008617D95@lca.pw>
Le 28/01/2020 à 04:33, Qian Cai a écrit :
>
>
>> On Jan 27, 2020, at 10:06 PM, Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>
>>
>>
>> On 01/28/2020 07:41 AM, Qian Cai wrote:
>>>
>>>
>>>> On Jan 27, 2020, at 8:28 PM, Anshuman Khandual <Anshuman.Khandual@arm.com> wrote:
>>>>
>>>> This adds tests which will validate architecture page table helpers and
>>>> other accessors in their compliance with expected generic MM semantics.
>>>> This will help various architectures in validating changes to existing
>>>> page table helpers or addition of new ones.
>>>>
>>>> This test covers basic page table entry transformations including but not
>>>> limited to old, young, dirty, clean, write, write protect etc at various
>>>> level along with populating intermediate entries with next page table page
>>>> and validating them.
>>>>
>>>> Test page table pages are allocated from system memory with required size
>>>> and alignments. The mapped pfns at page table levels are derived from a
>>>> real pfn representing a valid kernel text symbol. This test gets called
>>>> right after page_alloc_init_late().
>>>>
>>>> This gets build and run when CONFIG_DEBUG_VM_PGTABLE is selected along with
>>>> CONFIG_VM_DEBUG. Architectures willing to subscribe this test also need to
>>>> select CONFIG_ARCH_HAS_DEBUG_VM_PGTABLE which for now is limited to x86 and
>>>> arm64. Going forward, other architectures too can enable this after fixing
>>>> build or runtime problems (if any) with their page table helpers.
>>
>> Hello Qian,
>>
>>>
>>> What’s the value of this block of new code? It only supports x86 and arm64
>>> which are supposed to be good now.
>>
>> We have been over the usefulness of this code many times before as the patch is
>> already in it's V12. Currently it is enabled on arm64, x86 (except PAE), arc and
>> ppc32. There are build time or runtime problems with other archs which prevent
>
> I am not sure if I care too much about arc and ppc32 which are pretty much legacy
> platforms.
>
>> enablement of this test (for the moment) but then the goal is to integrate all
>> of them going forward. The test not only validates platform's adherence to the
>> expected semantics from generic MM but also helps in keeping it that way during
>> code changes in future as well.
>
> Another option maybe to get some decent arches on board first before merging this
> thing, so it have more changes to catch regressions for developers who might run this.
>
ppc32 an indecent / legacy platform ? Are you kidying ?
Powerquicc II PRO for instance is fully supported by the manufacturer
and widely used in many small networking devices.
Christophe
^ permalink raw reply
* [PATCH v6 08/10] parisc/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:12 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to the monitoring remains open
for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
monitoring is discouraged with respect to CAP_PERFMON capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
arch/parisc/kernel/perf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/parisc/kernel/perf.c b/arch/parisc/kernel/perf.c
index 676683641d00..c4208d027794 100644
--- a/arch/parisc/kernel/perf.c
+++ b/arch/parisc/kernel/perf.c
@@ -300,7 +300,7 @@ static ssize_t perf_write(struct file *file, const char __user *buf,
else
return -EFAULT;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
if (count != sizeof(uint32_t))
--
2.20.1
^ permalink raw reply related
* [PATCH v6 07/10] powerpc/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:12 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to the monitoring remains open
for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
monitoring is discouraged with respect to CAP_PERFMON capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
arch/powerpc/perf/imc-pmu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index cb50a9e1fd2d..e837717492e4 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -898,7 +898,7 @@ static int thread_imc_event_init(struct perf_event *event)
if (event->attr.type != event->pmu->type)
return -ENOENT;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
/* Sampling not supported */
@@ -1307,7 +1307,7 @@ static int trace_imc_event_init(struct perf_event *event)
if (event->attr.type != event->pmu->type)
return -ENOENT;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
/* Return if this is a couting event */
--
2.20.1
^ permalink raw reply related
* [PATCH v6 06/10] trace/bpf_trace: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:11 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to bpf_trace monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to bpf_trace monitoring remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for
secure bpf_trace monitoring is discouraged with respect to CAP_PERFMON
capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
kernel/trace/bpf_trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index e5ef4ae9edb5..334f1d71ebb1 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1395,7 +1395,7 @@ int perf_event_query_prog_array(struct perf_event *event, void __user *info)
u32 *ids, prog_cnt, ids_len;
int ret;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EPERM;
if (event->attr.type != PERF_TYPE_TRACEPOINT)
return -EINVAL;
--
2.20.1
^ permalink raw reply related
* [PATCH v6 05/10] drm/i915/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:10 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to i915_perf monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to i915_events subsystem remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for
secure i915_events monitoring is discouraged with respect to CAP_PERFMON
capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
drivers/gpu/drm/i915/i915_perf.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 2ae14bc14931..d89347861b7d 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -3375,10 +3375,10 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf,
/* Similar to perf's kernel.perf_paranoid_cpu sysctl option
* we check a dev.i915.perf_stream_paranoid sysctl option
* to determine if it's ok to access system wide OA counters
- * without CAP_SYS_ADMIN privileges.
+ * without CAP_PERFMON or CAP_SYS_ADMIN privileges.
*/
if (privileged_op &&
- i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
+ i915_perf_stream_paranoid && !perfmon_capable()) {
DRM_DEBUG("Insufficient privileges to open i915 perf stream\n");
ret = -EACCES;
goto err_ctx;
@@ -3571,9 +3571,8 @@ static int read_properties_unlocked(struct i915_perf *perf,
} else
oa_freq_hz = 0;
- if (oa_freq_hz > i915_oa_max_sample_rate &&
- !capable(CAP_SYS_ADMIN)) {
- DRM_DEBUG("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without root privileges\n",
+ if (oa_freq_hz > i915_oa_max_sample_rate && !perfmon_capable()) {
+ DRM_DEBUG("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without CAP_PERFMON or CAP_SYS_ADMIN privileges\n",
i915_oa_max_sample_rate);
return -EACCES;
}
@@ -3994,7 +3993,7 @@ int i915_perf_add_config_ioctl(struct drm_device *dev, void *data,
return -EINVAL;
}
- if (i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
+ if (i915_perf_stream_paranoid && !perfmon_capable()) {
DRM_DEBUG("Insufficient privileges to add i915 OA config\n");
return -EACCES;
}
@@ -4141,7 +4140,7 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data,
return -ENOTSUPP;
}
- if (i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
+ if (i915_perf_stream_paranoid && !perfmon_capable()) {
DRM_DEBUG("Insufficient privileges to remove i915 OA config\n");
return -EACCES;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v6 04/10] perf tool: extend Perf tool with CAP_PERFMON capability support
From: Alexey Budankov @ 2020-01-28 6:09 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Extend error messages to mention CAP_PERFMON capability as an option
to substitute CAP_SYS_ADMIN capability for secure system performance
monitoring and observability operations. Make perf_event_paranoid_check()
and __cmd_ftrace() to be aware of CAP_PERFMON capability.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to perf_events subsystem remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for
secure perf_events monitoring is discouraged with respect to CAP_PERFMON
capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/builtin-ftrace.c | 5 +++--
tools/perf/design.txt | 3 ++-
tools/perf/util/cap.h | 4 ++++
tools/perf/util/evsel.c | 10 +++++-----
tools/perf/util/util.c | 1 +
5 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index d5adc417a4ca..55eda54240fb 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -284,10 +284,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
.events = POLLIN,
};
- if (!perf_cap__capable(CAP_SYS_ADMIN)) {
+ if (!(perf_cap__capable(CAP_PERFMON) ||
+ perf_cap__capable(CAP_SYS_ADMIN))) {
pr_err("ftrace only works for %s!\n",
#ifdef HAVE_LIBCAP_SUPPORT
- "users with the SYS_ADMIN capability"
+ "users with the CAP_PERFMON or CAP_SYS_ADMIN capability"
#else
"root"
#endif
diff --git a/tools/perf/design.txt b/tools/perf/design.txt
index 0453ba26cdbd..a42fab308ff6 100644
--- a/tools/perf/design.txt
+++ b/tools/perf/design.txt
@@ -258,7 +258,8 @@ gets schedule to. Per task counters can be created by any user, for
their own tasks.
A 'pid == -1' and 'cpu == x' counter is a per CPU counter that counts
-all events on CPU-x. Per CPU counters need CAP_SYS_ADMIN privilege.
+all events on CPU-x. Per CPU counters need CAP_PERFMON or CAP_SYS_ADMIN
+privilege.
The 'flags' parameter is currently unused and must be zero.
diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h
index 051dc590ceee..ae52878c0b2e 100644
--- a/tools/perf/util/cap.h
+++ b/tools/perf/util/cap.h
@@ -29,4 +29,8 @@ static inline bool perf_cap__capable(int cap __maybe_unused)
#define CAP_SYSLOG 34
#endif
+#ifndef CAP_PERFMON
+#define CAP_PERFMON 38
+#endif
+
#endif /* __PERF_CAP_H */
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a69e64236120..a35f17723dd3 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2491,14 +2491,14 @@ int perf_evsel__open_strerror(struct evsel *evsel, struct target *target,
"You may not have permission to collect %sstats.\n\n"
"Consider tweaking /proc/sys/kernel/perf_event_paranoid,\n"
"which controls use of the performance events system by\n"
- "unprivileged users (without CAP_SYS_ADMIN).\n\n"
+ "unprivileged users (without CAP_PERFMON or CAP_SYS_ADMIN).\n\n"
"The current value is %d:\n\n"
" -1: Allow use of (almost) all events by all users\n"
" Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK\n"
- ">= 0: Disallow ftrace function tracepoint by users without CAP_SYS_ADMIN\n"
- " Disallow raw tracepoint access by users without CAP_SYS_ADMIN\n"
- ">= 1: Disallow CPU event access by users without CAP_SYS_ADMIN\n"
- ">= 2: Disallow kernel profiling by users without CAP_SYS_ADMIN\n\n"
+ ">= 0: Disallow ftrace function tracepoint by users without CAP_PERFMON or CAP_SYS_ADMIN\n"
+ " Disallow raw tracepoint access by users without CAP_SYS_PERFMON or CAP_SYS_ADMIN\n"
+ ">= 1: Disallow CPU event access by users without CAP_PERFMON or CAP_SYS_ADMIN\n"
+ ">= 2: Disallow kernel profiling by users without CAP_PERFMON or CAP_SYS_ADMIN\n\n"
"To make this setting permanent, edit /etc/sysctl.conf too, e.g.:\n\n"
" kernel.perf_event_paranoid = -1\n" ,
target->system_wide ? "system-wide " : "",
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 969ae560dad9..51cf3071db74 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -272,6 +272,7 @@ int perf_event_paranoid(void)
bool perf_event_paranoid_check(int max_level)
{
return perf_cap__capable(CAP_SYS_ADMIN) ||
+ perf_cap__capable(CAP_PERFMON) ||
perf_event_paranoid() <= max_level;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v6 03/10] perf/core: open access to probes for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:09 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring via kprobes and uprobes and eBPF tracing for
CAP_PERFMON privileged process. Providing the access under CAP_PERFMON
capability singly, without the rest of CAP_SYS_ADMIN credentials, excludes
chances to misuse the credentials and makes operation more secure.
perf kprobes and uprobes are used by ftrace and eBPF. perf probe uses
ftrace to define new kprobe events, and those events are treated as
tracepoint events. eBPF defines new probes via perf_event_open interface
and then the probes are used in eBPF tracing.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process or
program be granted only those privileges (e.g., capabilities) necessary to
accomplish its legitimate function, and only for the time that such privileges
are actually required)
For backward compatibility reasons access to perf_events subsystem remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for
secure perf_events monitoring is discouraged with respect to CAP_PERFMON
capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
kernel/events/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index d956c81bd310..c6453320ffea 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -9088,7 +9088,7 @@ static int perf_kprobe_event_init(struct perf_event *event)
if (event->attr.type != perf_kprobe.type)
return -ENOENT;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
/*
@@ -9148,7 +9148,7 @@ static int perf_uprobe_event_init(struct perf_event *event)
if (event->attr.type != perf_uprobe.type)
return -ENOENT;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
/*
--
2.20.1
^ permalink raw reply related
* [PATCH v6 02/10] perf/core: open access to the core for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:08 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring of kernel code, cpus, tracepoints and namespaces
data for a CAP_PERFMON privileged process. Providing the access under
CAP_PERFMON capability singly, without the rest of CAP_SYS_ADMIN credentials,
excludes chances to misuse the credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process or
program be granted only those privileges (e.g., capabilities) necessary to
accomplish its legitimate function, and only for the time that such privileges
are actually required)
For backward compatibility reasons access to perf_events subsystem remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
perf_events monitoring is discouraged with respect to CAP_PERFMON capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
include/linux/perf_event.h | 6 +++---
kernel/events/core.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 6d4c22aee384..730469babcc2 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1285,7 +1285,7 @@ static inline int perf_is_paranoid(void)
static inline int perf_allow_kernel(struct perf_event_attr *attr)
{
- if (sysctl_perf_event_paranoid > 1 && !capable(CAP_SYS_ADMIN))
+ if (sysctl_perf_event_paranoid > 1 && !perfmon_capable())
return -EACCES;
return security_perf_event_open(attr, PERF_SECURITY_KERNEL);
@@ -1293,7 +1293,7 @@ static inline int perf_allow_kernel(struct perf_event_attr *attr)
static inline int perf_allow_cpu(struct perf_event_attr *attr)
{
- if (sysctl_perf_event_paranoid > 0 && !capable(CAP_SYS_ADMIN))
+ if (sysctl_perf_event_paranoid > 0 && !perfmon_capable())
return -EACCES;
return security_perf_event_open(attr, PERF_SECURITY_CPU);
@@ -1301,7 +1301,7 @@ static inline int perf_allow_cpu(struct perf_event_attr *attr)
static inline int perf_allow_tracepoint(struct perf_event_attr *attr)
{
- if (sysctl_perf_event_paranoid > -1 && !capable(CAP_SYS_ADMIN))
+ if (sysctl_perf_event_paranoid > -1 && !perfmon_capable())
return -EPERM;
return security_perf_event_open(attr, PERF_SECURITY_TRACEPOINT);
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 2173c23c25b4..d956c81bd310 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11186,7 +11186,7 @@ SYSCALL_DEFINE5(perf_event_open,
}
if (attr.namespaces) {
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v6 01/10] capabilities: introduce CAP_PERFMON to kernel and user space
From: Alexey Budankov @ 2020-01-28 6:07 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Introduce CAP_PERFMON capability designed to secure system performance
monitoring and observability operations so that CAP_PERFMON would assist
CAP_SYS_ADMIN capability in its governing role for performance monitoring
and observability subsystems.
CAP_PERFMON hardens system security and integrity during system performance
monitoring and observability operations by decreasing attack surface that
is available to a CAP_SYS_ADMIN privileged process [2]. Providing the access
to system performance monitoring and observability operations under CAP_PERFMON
capability singly, without the rest of CAP_SYS_ADMIN credentials, excludes
chances to misuse the credentials and makes the operation more secure.
Thus, CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e: 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
CAP_PERFMON meets the demand to secure system performance monitoring and
observability operations for adoption in security sensitive, restricted,
multiuser production environments (e.g. HPC clusters, cloud and virtual compute
environments), where root or CAP_SYS_ADMIN credentials are not available to
mass users of a system, and securely unblocks applicability and scalability
of system performance monitoring and observability operations beyond root
and CAP_SYS_ADMIN process use cases.
CAP_PERFMON takes over CAP_SYS_ADMIN credentials related to system performance
monitoring and observability operations and balances amount of CAP_SYS_ADMIN
credentials following the recommendations in the capabilities man page [1]
for CAP_SYS_ADMIN: "Note: this capability is overloaded; see Notes to kernel
developers, below." For backward compatibility reasons access to system
performance monitoring and observability subsystems of the kernel remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN capability
usage for secure system performance monitoring and observability operations
is discouraged with respect to the designed CAP_PERFMON capability.
Although the software running under CAP_PERFMON can not ensure avoidance
of related hardware issues, the software can still mitigate these issues
following the official embargoed hardware issues mitigation procedure [2].
The bugs in the software itself can be fixed following the standard kernel
development process [3] to maintain and harden security of system performance
monitoring and observability operations.
[1] http://man7.org/linux/man-pages/man7/capabilities.7.html
[2] https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
[3] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
include/linux/capability.h | 4 ++++
include/uapi/linux/capability.h | 8 +++++++-
security/selinux/include/classmap.h | 4 ++--
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/linux/capability.h b/include/linux/capability.h
index ecce0f43c73a..027d7e4a853b 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -251,6 +251,10 @@ extern bool privileged_wrt_inode_uidgid(struct user_namespace *ns, const struct
extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
extern bool file_ns_capable(const struct file *file, struct user_namespace *ns, int cap);
extern bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns);
+static inline bool perfmon_capable(void)
+{
+ return capable(CAP_PERFMON) || capable(CAP_SYS_ADMIN);
+}
/* audit system wants to get cap info from files as well */
extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data *cpu_caps);
diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
index 240fdb9a60f6..8b416e5f3afa 100644
--- a/include/uapi/linux/capability.h
+++ b/include/uapi/linux/capability.h
@@ -366,8 +366,14 @@ struct vfs_ns_cap_data {
#define CAP_AUDIT_READ 37
+/*
+ * Allow system performance and observability privileged operations
+ * using perf_events, i915_perf and other kernel subsystems
+ */
+
+#define CAP_PERFMON 38
-#define CAP_LAST_CAP CAP_AUDIT_READ
+#define CAP_LAST_CAP CAP_PERFMON
#define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 7db24855e12d..c599b0c2b0e7 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -27,9 +27,9 @@
"audit_control", "setfcap"
#define COMMON_CAP2_PERMS "mac_override", "mac_admin", "syslog", \
- "wake_alarm", "block_suspend", "audit_read"
+ "wake_alarm", "block_suspend", "audit_read", "perfmon"
-#if CAP_LAST_CAP > CAP_AUDIT_READ
+#if CAP_LAST_CAP > CAP_PERFMON
#error New capability defined, please update COMMON_CAP2_PERMS.
#endif
--
2.20.1
^ permalink raw reply related
* [PATCH v6 00/10] Introduce CAP_PERFMON to secure system performance monitoring and observability
From: Alexey Budankov @ 2020-01-28 5:52 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: Song Liu, Andi Kleen, linux-parisc@vger.kernel.org,
Alexander Shishkin, linuxppc-dev@lists.ozlabs.org,
intel-gfx@lists.freedesktop.org, Igor Lubashev, linux-kernel,
Stephane Eranian, linux-perf-users@vger.kernel.org,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org,
oprofile-list, Lionel Landwerlin, Namhyung Kim, Jiri Olsa,
linux-arm-kernel
Currently access to perf_events, i915_perf and other performance monitoring and
observability subsystems of the kernel is open only for a privileged process [1]
with CAP_SYS_ADMIN capability enabled in the process effective set [2].
This patch set introduces CAP_PERFMON capability designed to secure system
performance monitoring and observability operations so that CAP_PERFMON would
assist CAP_SYS_ADMIN capability in its governing role for performance monitoring
and observability subsystems of the kernel.
CAP_PERFMON intends to harden system security and integrity during system
performance monitoring and observability operations by decreasing attack surface
that is available to a CAP_SYS_ADMIN privileged process [2]. Providing the access
to system performance monitoring and observability operations under CAP_PERFMON
capability singly, without the rest of CAP_SYS_ADMIN credentials, excludes chances
to misuse the credentials and makes the operation more secure. Thus, CAP_PERFMON
implements the principal of least privilege for performance monitoring and
observability operations (POSIX IEEE 1003.1e: 2.2.2.39 principle of least
privilege: A security design principle that states that a process or program be
granted only those privileges (e.g., capabilities) necessary to accomplish its
legitimate function, and only for the time that such privileges are actually
required)
CAP_PERFMON intends to meet the demand to secure system performance monitoring
and observability operations for adoption in security sensitive, restricted,
multiuser production environments (e.g. HPC clusters, cloud and virtual compute
environments), where root or CAP_SYS_ADMIN credentials are not available to mass
users of a system, and securely unblock applicability and scalability of system
performance monitoring and observability operations beyond root or CAP_SYS_ADMIN
process use cases.
CAP_PERFMON intends to take over CAP_SYS_ADMIN credentials related to system
performance monitoring and observability operations and balance amount of
CAP_SYS_ADMIN credentials following the recommendations in the capabilities man
page [2] for CAP_SYS_ADMIN: "Note: this capability is overloaded; see Notes to
kernel developers, below." For backward compatibility reasons access to system
performance monitoring and observability subsystems of the kernel remains open
for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN capability usage for
secure system performance monitoring and observability operations is discouraged
with respect to the designed CAP_PERFMON capability.
Possible alternative solution to this system security hardening, capabilities
balancing task of making performance monitoring and observability operations
more accessible could be to use the existing CAP_SYS_PTRACE capability to govern
system performance monitoring and observability subsystems. However CAP_SYS_PTRACE
capability still provides users with more credentials than are required for secure
performance monitoring and observability operations and this excess is avoided by
the designed CAP_PERFMON capability.
Although software running under CAP_PERFMON can not ensure avoidance of related
hardware issues, the software can still mitigate those issues following the official
embargoed hardware issues mitigation procedure [3]. The bugs in the software itself
can be fixed following the standard kernel development process [4] to maintain and
harden security of system performance monitoring and observability operations.
Finally, the patch set is shaped in the way that simplifies backtracking procedure
of possible induced issues [5] as much as possible.
The patch set is for tip perf/core repository:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip perf/core
sha1: 56ee04aa63285d6bc8a995a26e2441ae3d419bcd
---
Changes in v6:
- avoided noaudit checks in perfmon_capable() to explicitly advertise CAP_PERFMON
usage thru audit logs to secure system performance monitoring and observability
Changes in v5:
- renamed CAP_SYS_PERFMON to CAP_PERFMON
- extended perfmon_capable() with noaudit checks
Changes in v4:
- converted perfmon_capable() into an inline function
- made perf_events kprobes, uprobes, hw breakpoints and namespaces data available
to CAP_SYS_PERFMON privileged processes
- applied perfmon_capable() to drivers/perf and drivers/oprofile
- extended __cmd_ftrace() with support of CAP_SYS_PERFMON
Changes in v3:
- implemented perfmon_capable() macros aggregating required capabilities checks
Changes in v2:
- made perf_events trace points available to CAP_SYS_PERFMON privileged processes
- made perf_event_paranoid_check() treat CAP_SYS_PERFMON equally to CAP_SYS_ADMIN
- applied CAP_SYS_PERFMON to i915_perf, bpf_trace, powerpc and parisc system
performance monitoring and observability related subsystems
---
Alexey Budankov (10):
capabilities: introduce CAP_PERFMON to kernel and user space
perf/core: open access to the core for CAP_PERFMON privileged process
perf/core: open access to probes for CAP_PERFMON privileged process
perf tool: extend Perf tool with CAP_PERFMON capability support
drm/i915/perf: open access for CAP_PERFMON privileged process
trace/bpf_trace: open access for CAP_PERFMON privileged process
powerpc/perf: open access for CAP_PERFMON privileged process
parisc/perf: open access for CAP_PERFMON privileged process
drivers/perf: open access for CAP_PERFMON privileged process
drivers/oprofile: open access for CAP_PERFMON privileged process
arch/parisc/kernel/perf.c | 2 +-
arch/powerpc/perf/imc-pmu.c | 4 ++--
drivers/gpu/drm/i915/i915_perf.c | 13 ++++++-------
drivers/oprofile/event_buffer.c | 2 +-
drivers/perf/arm_spe_pmu.c | 4 ++--
include/linux/capability.h | 4 ++++
include/linux/perf_event.h | 6 +++---
include/uapi/linux/capability.h | 8 +++++++-
kernel/events/core.c | 6 +++---
kernel/trace/bpf_trace.c | 2 +-
security/selinux/include/classmap.h | 4 ++--
tools/perf/builtin-ftrace.c | 5 +++--
tools/perf/design.txt | 3 ++-
tools/perf/util/cap.h | 4 ++++
tools/perf/util/evsel.c | 10 +++++-----
tools/perf/util/util.c | 1 +
16 files changed, 47 insertions(+), 31 deletions(-)
---
Testing and validation (Intel Skylake, 8 cores, Fedora 29, 5.5.0-rc3+, x86_64):
libcap library [6], [7], [8] and Perf tool can be used to apply CAP_PERFMON
capability for secure system performance monitoring and observability beyond the
scope permitted by the system wide perf_event_paranoid kernel setting [9] and
below are the steps for evaluation:
- patch, build and boot the kernel
- patch, build Perf tool e.g. to /home/user/perf
...
# git clone git://git.kernel.org/pub/scm/libs/libcap/libcap.git libcap
# pushd libcap
# patch libcap/include/uapi/linux/capabilities.h with [PATCH 1]
# make
# pushd progs
# ./setcap "cap_perfmon,cap_sys_ptrace,cap_syslog=ep" /home/user/perf
# ./setcap -v "cap_perfmon,cap_sys_ptrace,cap_syslog=ep" /home/user/perf
/home/user/perf: OK
# ./getcap /home/user/perf
/home/user/perf = cap_sys_ptrace,cap_syslog,cap_perfmon+ep
# echo 2 > /proc/sys/kernel/perf_event_paranoid
# cat /proc/sys/kernel/perf_event_paranoid
2
...
$ /home/user/perf top
... works as expected ...
$ cat /proc/`pidof perf`/status
Name: perf
Umask: 0002
State: S (sleeping)
Tgid: 2958
Ngid: 0
Pid: 2958
PPid: 9847
TracerPid: 0
Uid: 500 500 500 500
Gid: 500 500 500 500
FDSize: 256
...
CapInh: 0000000000000000
CapPrm: 0000004400080000
CapEff: 0000004400080000 => 01000100 00000000 00001000 00000000 00000000
cap_perfmon,cap_sys_ptrace,cap_syslog
CapBnd: 0000007fffffffff
CapAmb: 0000000000000000
NoNewPrivs: 0
Seccomp: 0
Speculation_Store_Bypass: thread vulnerable
Cpus_allowed: ff
Cpus_allowed_list: 0-7
...
Usage of cap_perfmon effectively avoids unused credentials excess:
- with cap_sys_admin:
CapEff: 0000007fffffffff => 01111111 11111111 11111111 11111111 11111111
- with cap_perfmon:
CapEff: 0000004400080000 => 01000100 00000000 00001000 00000000 00000000
38 34 19
perfmon syslog sys_ptrace
---
[1] https://www.kernel.org/doc/html/latest/admin-guide/perf-security.html
[2] http://man7.org/linux/man-pages/man7/capabilities.7.html
[3] https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
[4] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html
[5] https://www.kernel.org/doc/html/latest/process/management-style.html#decisions
[6] http://man7.org/linux/man-pages/man8/setcap.8.html
[7] https://git.kernel.org/pub/scm/libs/libcap/libcap.git
[8] https://sites.google.com/site/fullycapable/, posix_1003.1e-990310.pdf
[9] http://man7.org/linux/man-pages/man2/perf_event_open.2.html
--
2.20.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox