Re: [PATCH] lsm: handle the NULL buffer case in lsm_fill_user_ctx()

["Serge E. Hallyn" <serge@hallyn.com>]

On Fri, Mar 15, 2024 at 09:08:47AM -0700, Casey Schaufler wrote:
> On 3/15/2024 8:02 AM, Serge E. Hallyn wrote:
> > On Wed, Mar 13, 2024 at 10:22:03PM -0400, Paul Moore wrote:
> >> Passing a NULL buffer into the lsm_get_self_attr() syscall is a valid
> >> way to quickly determine the minimum size of the buffer needed to for
> >> the syscall to return all of the LSM attributes to the caller.
> >> Unfortunately we/I broke that behavior in commit d7cf3412a9f6
> >> ("lsm: consolidate buffer size handling into lsm_fill_user_ctx()")
> >> such that it returned an error to the caller; this patch restores the
> >> original desired behavior of using the NULL buffer as a quick way to
> >> correctly size the attribute buffer.
> >>
> >> Cc: stable@vger.kernel.org
> >> Fixes: d7cf3412a9f6 ("lsm: consolidate buffer size handling into lsm_fill_user_ctx()")
> >> Signed-off-by: Paul Moore <paul@paul-moore.com>
> >> ---
> >>  security/security.c | 8 +++++++-
> >>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/security/security.c b/security/security.c
> >> index 5b2e0a15377d..7e118858b545 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -780,7 +780,9 @@ static int lsm_superblock_alloc(struct super_block *sb)
> >>   * @id: LSM id
> >>   * @flags: LSM defined flags
> >>   *
> >> - * Fill all of the fields in a userspace lsm_ctx structure.
> >> + * Fill all of the fields in a userspace lsm_ctx structure.  If @uctx is NULL
> >> + * simply calculate the required size to output via @utc_len and return
> >> + * success.
> >>   *
> >>   * Returns 0 on success, -E2BIG if userspace buffer is not large enough,
> >>   * -EFAULT on a copyout error, -ENOMEM if memory can't be allocated.
> >> @@ -799,6 +801,10 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len,
> >>  		goto out;
> >>  	}
> >>  
> >> +	/* no buffer - return success/0 and set @uctx_len to the req size */
> >> +	if (!uctx)
> >> +		goto out;
> > If the user just passes in *uctx_len=0, then they will get -E2BIG
> > but still will get the length in *uctx_len.
> 
> Yes.
> 
> > To use it this new way, they have to first set *uctx_len to a
> > value larger than nctx_len could possibly be, else they'll...
> > still get -E2BIG.
> 
> Not sure I understand the problem. A return of 0 or E2BIG gets the
> caller the size. 

The problem is that there are two ways of doing the same thing, with
different behavior.  People are bound to get it wrong at some point,
and it's more corner cases to try and maintain (once we start).