Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3319126a-c5b5-c9db-dddb-7ed169ac7e83@schaufler-ca.com>
Date:   Wed, 7 Jun 2023 16:39:55 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH v10 10/11] SELinux: Add selfattr hooks
Content-Language: en-US
To:     Paul Moore <paul@paul-moore.com>,
        linux-security-module@vger.kernel.org
Cc:     jmorris@namei.org, keescook@chromium.org,
        john.johansen@canonical.com, penguin-kernel@i-love.sakura.ne.jp,
        stephen.smalley.work@gmail.com, linux-kernel@vger.kernel.org,
        linux-api@vger.kernel.org, mic@digikod.net,
        selinux@vger.kernel.org, Casey Schaufler <casey@schaufler-ca.com>
References: <20230428203417.159874-11-casey@schaufler-ca.com>
 <b09b591926e0f297fe0a3e9d8fcf3a6a.paul@paul-moore.com>
From:   Casey Schaufler <casey@schaufler-ca.com>
In-Reply-To: <b09b591926e0f297fe0a3e9d8fcf3a6a.paul@paul-moore.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 6/7/2023 3:32 PM, Paul Moore wrote:
> On Apr 28, 2023 Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Add hooks for setselfattr and getselfattr. These hooks are not very
>> different from their setprocattr and getprocattr equivalents, and
>> much of the code is shared.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> Cc: selinux@vger.kernel.org
>> Cc: Paul Moore <paul@paul-moore.com>
>> ---
>>  security/selinux/hooks.c | 154 +++++++++++++++++++++++++++++++--------
>>  1 file changed, 124 insertions(+), 30 deletions(-)
>>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 2ee92d3fb79d..e78b955e04f8 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -6542,6 +6559,81 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
>>  	return error;
>>  }
>>  
>> +static int selinux_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx,
>> +			       size_t *size, u32 flags)
>> +{
>> +	char *value;
>> +	size_t total_len;
>> +	int len;
>> +	int rc;
>> +
>> +	len = selinux_lsm_getattr(attr, current, &value);
>> +	if (len < 0)
>> +		return len;
>> +
>> +	total_len = ALIGN(struct_size(ctx, ctx, len), 8);
>> +
>> +	if (total_len > *size)
>> +		rc = -E2BIG;
> Hmm.  Since we need to calculate the aligned @total_len value in the
> LSM specific code, perhaps it doesn't make sense to also do the
> alignment in lsm_fill_user_ctx().  My apologies, I know I was the one
> who suggested doing the alignment in a common place previously.

Some future user of lsm_fill_user_ctx() may not need to do the
calculation, so I would be inclined to leave it here. I'm thinking
SO_PEERCONTEXT, or the like. But I'll go with whatever gets the
patchset moving forward.

>
>> +	else if (ctx)
>> +		rc = lsm_fill_user_ctx(ctx, value, len, LSM_ID_SELINUX, 0);
>> +	else
>> +		rc = 1;
>> +
>> +	*size = total_len;
>> +	if (rc < 0)
>> +		return rc;
>> +	return 1;
>> +}
> --
> paul-moore.com