Re: kernel BUG at kernel/cred.c:434!

From: Stephen Smalley
Date: Thu Apr 18 2019 - 09:43:10 EST


On 4/17/19 12:42 PM, Casey Schaufler wrote:
On 4/17/2019 9:27 AM, Oleg Nesterov wrote:
On 04/17, Paul Moore wrote:
On Wed, Apr 17, 2019 at 10:57 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
On 04/17, Paul Moore wrote:
I'm tempted to simply return an error in selinux_setprocattr() if
the task's credentials are not the same as its real_cred;
What about other modules? I have no idea what smack_setprocattr() is,
but it too does prepare_creds/commit creds.

it seems that the simplest workaround should simply add the additional
cred == real_cred into proc_pid_attr_write().
Yes, that is simple, but I worry about what other LSMs might want to
do. While I believe failing if the effective creds are not the same
as the real_creds is okay for SELinux (possibly Smack too), I worry
about what other LSMs may want to do. After all,
proc_pid_attr_write() doesn't change the the creds itself, that is
something the specific LSMs do.
Yes, but if proc_pid_attr_write() is called with cred != real_cred then
something is already wrong?

In fact, I think that something is already wrong if it is not called by
user-space directly. Too late to ask, but why is this /proc/self/attr/
magic not implemented via syscall(s) ?

Shell scripts, for one thing. It's a straightforward and appropriate
use of the /proc interface. System calls would require additional change
to existing programs, whereas using the /proc interface allows a good
deal to be done in the containing scripts.

It is fairly awkward/fragile to use these interfaces from shell scripts due to limitations of the interface (e.g. no partial writes, thereby breaking if the shell splits up the data into multiple write() calls) and due to the fact that most of the attributes (except for current) are typically reset/cleared upon exec to prevent undue caller/callee influence. It works well enough for echo -n "somelabel" > /proc/self/attr/current but not much else, and even that doesn't work without the -n due to multiple write() calls.

Just for the record, this functionality was originally implemented via separate system calls at least for SELinux (in its original kernel patch), then multiplexed through the single LSM security system call upon porting to LSM (before merging to mainline), but viro and others strongly favored using /proc as the interface for getting/setting any new process attributes. Understandable, but it does impose some limitations, including a dependency on having a writable proc mount in the namespace of any process that needs to set any of these attributes,