On 7/21/2023 3:28 PM, Paul Moore wrote:
> On Tue, Jul 11, 2023 at 11:36 AM Mickaël Salaün <mic@xxxxxxxxxxx> wrote:
>> On 29/06/2023 21:55, Casey Schaufler wrote:
>>
> ...
>>> +/**
>>> + * security_setselfattr - Set an LSM attribute on the current process.
>>> + * @attr: which attribute to set
>>> + * @ctx: the user-space source for the information
>>> + * @size: the size of the data
>>> + * @flags: reserved for future use, must be 0
>>> + *
>>> + * Set an LSM attribute for the current process. The LSM, attribute
>>> + * and new value are included in @ctx.
>>> + *
>>> + * Returns 0 on success, -EINVAL if the input is inconsistent, -EFAULT
>>> + * if the user buffer is inaccessible or an LSM specific failure.
>>> + */
>>> +int security_setselfattr(unsigned int attr, struct lsm_ctx __user *ctx,
>>> + size_t size, u32 flags)
>>> +{
>>> + struct security_hook_list *hp;
>>> + struct lsm_ctx lctx;
>>> +
>>> + if (flags)
>>> + return -EINVAL;
>>> + if (size < sizeof(*ctx))
>>> + return -EINVAL;
>>> + if (copy_from_user(&lctx, ctx, sizeof(*ctx)))
>> I'd suggest to handle all the user space copy here and pass a kernel
>> pointer to each LSM hook calls (and handle kmalloc and kfree here, if
>> needed).
> Agreed. I thought I mentioned something like that at one point, maybe
> not. In general we should do whatever user/kernel copying and sanity
> checking in the LSM layer that we can; there will be somethings we
> can't check, but those that we can, we should.
That is in direct conflict with the "thin LSM" concept. My recollection,
and it could be wrong, was that you wanted the user space copy in the
LSM specific code. Maybe I'm wrong. I will move it into the infrastructure.
It will make the code simpler.
