[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e1b62234-9b8a-e7c2-2946-5ef9f6f23a08@cloudflare.com>
Date: Mon, 13 Jun 2022 15:52:38 -0500
From: Frederick Lawler <fred@...udflare.com>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-aio@...ck.org, linux-fsdevel@...r.kernel.org,
linux-cachefs@...hat.com, linux-cifs@...r.kernel.org,
samba-technical@...ts.samba.org, linux-mm@...ck.org,
linux-nfs@...r.kernel.org, linux-unionfs@...r.kernel.org,
linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
keyrings@...r.kernel.org, selinux@...r.kernel.org,
serge@...lyn.com, amir73il@...il.com, kernel-team@...udflare.com,
Jeff Moyer <jmoyer@...hat.com>,
Paul Moore <paul@...l-moore.com>
Subject: Re: [PATCH v3] cred: Propagate security_prepare_creds() error code
Hi Eric,
On 6/13/22 12:04 PM, Eric W. Biederman wrote:
> Frederick Lawler <fred@...udflare.com> writes:
>
>> While experimenting with the security_prepare_creds() LSM hook, we
>> noticed that our EPERM error code was not propagated up the callstack.
>> Instead ENOMEM is always returned. As a result, some tools may send a
>> confusing error message to the user:
>>
>> $ unshare -rU
>> unshare: unshare failed: Cannot allocate memory
>>
>> A user would think that the system didn't have enough memory, when
>> instead the action was denied.
>>
>> This problem occurs because prepare_creds() and prepare_kernel_cred()
>> return NULL when security_prepare_creds() returns an error code. Later,
>> functions calling prepare_creds() and prepare_kernel_cred() return
>> ENOMEM because they assume that a NULL meant there was no memory
>> allocated.
>>
>> Fix this by propagating an error code from security_prepare_creds() up
>> the callstack.
>
> Why would it make sense for security_prepare_creds to return an error
> code other than ENOMEM?
> > That seems a bit of a violation of what that function is supposed to do
>
The API allows LSM authors to decide what error code is returned from
the cred_prepare hook. security_task_alloc() is a similar hook, and has
its return code propagated.
I'm proposing we follow security_task_allocs() pattern, and add
visibility for failure cases in prepare_creds().
> I have probably missed a very interesting discussion where that was
> mentioned but I don't see link to the discussion or anything explaining
> why we want to do that in this change.
>
AFAIK, this is the start of the discussion.
> Eric
>
Powered by blists - more mailing lists