Re: [PATCH v2 11/25] security: Align inode_setattr hook definition with EVM

[Casey Schaufler <casey@schaufler-ca.com>]

On 9/4/2023 2:08 PM, Jarkko Sakkinen wrote:
> On Thu Aug 31, 2023 at 1:41 PM EEST, Roberto Sassu wrote:
>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>
>> Add the idmap parameter to the definition, so that evm_inode_setattr() can
>> be registered as this hook implementation.
>>
>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>  include/linux/lsm_hook_defs.h | 3 ++-
>>  security/security.c           | 2 +-
>>  security/selinux/hooks.c      | 3 ++-
>>  security/smack/smack_lsm.c    | 4 +++-
>>  4 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
>> index 4bdddb52a8fe..fdf075a6b1bb 100644
>> --- a/include/linux/lsm_hook_defs.h
>> +++ b/include/linux/lsm_hook_defs.h
>> @@ -134,7 +134,8 @@ LSM_HOOK(int, 0, inode_readlink, struct dentry *dentry)
>>  LSM_HOOK(int, 0, inode_follow_link, struct dentry *dentry, struct inode *inode,
>>  	 bool rcu)
>>  LSM_HOOK(int, 0, inode_permission, struct inode *inode, int mask)
>> -LSM_HOOK(int, 0, inode_setattr, struct dentry *dentry, struct iattr *attr)
>> +LSM_HOOK(int, 0, inode_setattr, struct mnt_idmap *idmap, struct dentry *dentry,
>> +	 struct iattr *attr)
> LSM_HOOK(int, 0, inode_setattr, struct mnt_idmap *idmap, struct dentry *dentry, struct iattr *attr)
>
> Only 99 characters, i.e. breaking into two lines is not necessary.

We're keeping the LSM code in the ancient 80 character format.
Until we get some fresh, young maintainers involved who can convince
us that line wrapped 80 character terminals are kewl we're sticking
with what we know.

	https://lwn.net/Articles/822168/

>
>>  LSM_HOOK(int, 0, inode_getattr, const struct path *path)
>>  LSM_HOOK(int, 0, inode_setxattr, struct mnt_idmap *idmap,
>>  	 struct dentry *dentry, const char *name, const void *value,
>> diff --git a/security/security.c b/security/security.c
>> index cb6242feb968..2b24d01cf181 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -2117,7 +2117,7 @@ int security_inode_setattr(struct mnt_idmap *idmap,
>>  
>>  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>>  		return 0;
>> -	ret = call_int_hook(inode_setattr, 0, dentry, attr);
>> +	ret = call_int_hook(inode_setattr, 0, idmap, dentry, attr);
>>  	if (ret)
>>  		return ret;
>>  	return evm_inode_setattr(idmap, dentry, attr);
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index ee7c49c2cfd3..bfcc4d9aa5ab 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -3075,7 +3075,8 @@ static int selinux_inode_permission(struct inode *inode, int mask)
>>  	return rc;
>>  }
>>  
>> -static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr)
>> +static int selinux_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>> +				 struct iattr *iattr)
>>  {
>>  	const struct cred *cred = current_cred();
>>  	struct inode *inode = d_backing_inode(dentry);
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index 679156601a10..89f2669d50a9 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -1181,12 +1181,14 @@ static int smack_inode_permission(struct inode *inode, int mask)
>>  
>>  /**
>>   * smack_inode_setattr - Smack check for setting attributes
>> + * @idmap: idmap of the mount
>>   * @dentry: the object
>>   * @iattr: for the force flag
>>   *
>>   * Returns 0 if access is permitted, an error code otherwise
>>   */
>> -static int smack_inode_setattr(struct dentry *dentry, struct iattr *iattr)
>> +static int smack_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>> +			       struct iattr *iattr)
> static int smack_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry, struct iattr *iattr)
>
> Can be still in a single line (100 characters exactly).
>
>
>>  {
>>  	struct smk_audit_info ad;
>>  	int rc;
>> -- 
>> 2.34.1
>
> BR, Jarkko