Frederick Lawler <fred@xxxxxxxxxxxxxx> writes:
Hi Eric,It is not an api. It is an implementation detail of the linux kernel.
On 6/13/22 12:04 PM, Eric W. Biederman wrote:
Frederick Lawler <fred@xxxxxxxxxxxxxx> writes:The API allows LSM authors to decide what error code is returned from the
While experimenting with the security_prepare_creds() LSM hook, weWhy would it make sense for security_prepare_creds to return an error
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.
code other than ENOMEM?
> That seems a bit of a violation of what that function is supposed to do
cred_prepare hook. security_task_alloc() is a similar hook, and has its return
code propagated.
It is a set of convenient functions that do a job.
The general rule is we don't support cases without an in-tree user. I
don't see an in-tree user.
I'm proposing we follow security_task_allocs() pattern, and add visibility for
failure cases in prepare_creds().
I am asking why we would want to. Especially as it is not an API, and I
don't see any good reason for anything but an -ENOMEM failure to be
supported.
Without an in-tree user that cares it is probably better to go the
opposite direction and remove the possibility of return anything but
memory allocation failure. That will make it clearer to implementors
that a general error code is not supported and this is not a location
to implement policy, this is only a hook to allocate state for the LSM.
You were on v3 and had an out of tree piece of code so I assumed someoneI have probably missed a very interesting discussion where that wasAFAIK, this is the start of the discussion.
mentioned but I don't see link to the discussion or anything explaining
why we want to do that in this change.
had at least thought about why you want to implement policy in a piece
of code whose only purpose is to allocate memory to store state.
Eric