Re: [PATCH v3] cred: Propagate security_prepare_creds() error code

[Frederick Lawler]

Hi Eric,

On 6/13/22 12:04 PM, Eric W. Biederman wrote:
> Frederick Lawler <fred@xxxxxxxxxxxxxx> 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