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

[Stephen Smalley <sds@tycho.nsa.gov>]

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@redhat.com> 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,