Re: [PATCH v4 1/9] capabilities: introduce CAP_SYS_PERFMON to kernel and user space
On Wed, Dec 18, 2019 at 12:24:28PM +0300, Alexey Budankov wrote: > > Introduce CAP_SYS_PERFMON capability devoted to secure system performance > monitoring and observability operations so that CAP_SYS_PERFMON would > assist CAP_SYS_ADMIN capability in its governing role for perf_events, > i915_perf and other subsystems of the kernel. > > CAP_SYS_PERFMON intends to harden system security and integrity during > system performance monitoring and observability operations by decreasing > attack surface that is available to CAP_SYS_ADMIN privileged processes. > > CAP_SYS_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 in accordance with the recommendations > provided in the man page for CAP_SYS_ADMIN [1]: "Note: this capability > is overloaded; see Notes to kernel developers, below." > > [1] http://man7.org/linux/man-pages/man7/capabilities.7.html > > Signed-off-by: Alexey Budankov Acked-by: Serge Hallyn
Re: [PATCH 1/3] powerpc: bare minimum checkpoint/restart implementation
Quoting Nathan Lynch (n...@pobox.com): Nathan Lynch n...@pobox.com wrote: Oren Laadan wrote: Nathan Lynch wrote: What doesn't work: * restarting a 32-bit task from a 64-bit task and vice versa Is there a test to bail if we attempt to checkpoint such tasks ? No, but I'll add one if it looks too hard to fix for the next round. Unfortunately, adding a check for this is hard. The point of no return in the restart path is cr_read_mm, which tears down current's address space. cr_read_mm runs way before cr_read_cpu, which is the only restart method I've implemented for powerpc so far. So, checking for this condition in cr_read_cpu is too late if I want restart(2) to return an error and leave the caller's memory map intact. (And I do want this: restart should be as robust as execve.) Well okay then, cr_read_head_arch seems to be the right place in the restart sequence for the architecture code to handle this. However, cr_write_head_arch (which produces the buffer that cr_read_head_arch consumes) is not provided a reference to the task to be checkpointed, nor can it assume that it's operating on current. I need a reference to a task before I can determine whether it's running in 32- or 64-bit mode, or using the FPU, Altivec, SPE, whatever. In any case, mixing 32- and 64-bit tasks across restart is something I eventually want to support, not reject. But the problem I've outlined applies to FPU state and vector extensions (VMX, SPE), as well as sanity-checking debug register (DABR) contents. We'll need to be able to error out gracefully from restart when a checkpoint image specifies a feature unsupported by the current kernel or hardware. But I don't see how to do it with the current architecture. Am I missing something? I suspect I can guess the response to this suggestion, but how about we accept that if sys_restart() fails due to something like this, the task is lost and can't exit gracefully? The end-user can then run a user-space program to run through the checkpoint image ahead of time (if they want) and verify whether restart can be expected to succeed on the current hardware and os version? If a minor difference is detected, the user-space program might simply rewrite (a copy of) the checkpoint image into one which can in fact succeed. -serge ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/3] powerpc: bare minimum checkpoint/restart implementation
Quoting Benjamin Herrenschmidt (b...@kernel.crashing.org): On Wed, 2009-02-04 at 18:44 -0500, Oren Laadan wrote: * Anything that is decided at compiled time should probably go to the arch- dependent header. * Anything that can change at boot time (e.g., for x86 that would include the capabilities of the FPU), or even run time (is there any ?) should be described to the letter (in fine print) in 'struct cr_hdr_cpu' and friends. I think we should avoid compile time completely. We mostly try to have kernels running on everything anyway, and there's no reason not to be able to move a snapshot to a different CPU if it's not using a feature of the CPU that is different. Absolutely, but the accepted way to handle that so far is that if you want to run an incompatible checkpoint image on a new cpu, a userspace program will rewrite the image to be correct for the target cpu. But what you list below seems more usable than trying to encapsulate that info in some hokey version number system. Nathan, what about you start the structure with a 64 bit bitmask that indicates what records are present followed by concatenated records ? IE. The main state (pt_regs) wouldn't change, but then, you could have a list of things: - FPRs - old style VSX - VSRs - Freescale SPE state - DABR - BookE IAC/DACs - tbd... Then, when resuming a snapshot, we can use some bit masks trickery indicating the validity on a given target. IE. If CPU has no VSX and original program uses VSX then you can't resume. But if CPU has VSR you can.. etc... We can keep it trivial at fist, especting the same features, and try to be smart later. Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/3] powerpc: bare minimum checkpoint/restart implementation
Quoting Benjamin Herrenschmidt (b...@kernel.crashing.org):
+struct cr_hdr_cpu {
+ struct pt_regs pt_regs;
+ /* relevant fields from thread_struct */
+ double fpr[32][TS_FPRWIDTH];
+ unsigned int fpscr;
+ int fpexc_mode;
+ /* unsigned int align_ctl; this is never updated? */
+ unsigned long dabr;
+};
Is there some version or other identification somewhere ? If not there
should be. ie, we're going to add things here. For example, what about
the vector registers ? Also, some CPUs will have more HW debug registers
than just the DABR (we plan to add support for all the BookE architected
IACs and DACs for example), etc...
The arch-independent checkpoint header does have kernel
maj:min:rev:patch info. We expect to have to do more,
assuming that the .config can change the arch-dependent
cpu header (i.e. perhaps TS_FPRWIDTH could be changed).
-serge
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/3] powerpc: bare minimum checkpoint/restart implementation
Quoting Oren Laadan (or...@cs.columbia.edu):
+static void cr_hdr_init(struct cr_hdr *hdr, __s16 type, __s16 len, __u32
parent)
+{
+ hdr-type = type;
+ hdr-len = len;
+ hdr-parent = parent;
+}
+
This function is rather generic and useful to non-arch-dependent and other
architectures code. Perhaps put in a separate patch ?
BTW I disagree here - looking through the patch I found
the use of this fn to be very distracting. To replace 3
simple in-line assignments, I have to verify the order of
4 weird-looking arguments, and actually first try to
remember what 'cr_hdr_init' is supposed to be and do?
That's not meant to be a complaint, just explanation :)
I prefer dropping its use altogether. Since I believe
most of the functions calling it in this patch shouldn't
exist anyway (just writing 0xdeadbeef into the file), it
all the more should just go away.
-serge
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 6/8] cleanup do_init_bootmem()
Quoting Dave Hansen ([EMAIL PROTECTED]): I'm debating whether this is worth it. It makes this a bit more clean looking, but doesn't seriously enhance readability. But, I do think it helps a bit. Thoughts? Absolutely. do_init_bootmem_node() is *still* a bit largish, but far better broken out. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
