[PATCH v2 15/25] LSM: Specify which LSM to display

Casey Schaufler casey at schaufler-ca.com
Wed Jun 19 17:00:24 UTC 2019


On 6/18/2019 10:28 PM, Kees Cook wrote:
> On Tue, Jun 18, 2019 at 04:05:41PM -0700, Casey Schaufler wrote:
>> Create a new entry "display" in /proc/.../attr for controlling
>> which LSM security information is displayed for a process.
>> The name of an active LSM that supplies hooks for human readable
>> data may be written to "display" to set the value. The name of
>> the LSM currently in use can be read from "display".
>> At this point there can only be one LSM capable of display
>> active.
>>
>> This affects /proc/.../attr/current and SO_PEERSEC.
>>
>> Signed-off-by: Casey Schaufler <casey at schaufler-ca.com>
>> ---
>>  fs/proc/base.c      |   1 +
>>  security/security.c | 108 +++++++++++++++++++++++++++++++++++---------
>>  2 files changed, 88 insertions(+), 21 deletions(-)
>>
>> diff --git a/fs/proc/base.c b/fs/proc/base.c
>> index ddef482f1334..7bf70e041315 100644
>> --- a/fs/proc/base.c
>> +++ b/fs/proc/base.c
>> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
>>  	ATTR(NULL, "fscreate",		0666),
>>  	ATTR(NULL, "keycreate",		0666),
>>  	ATTR(NULL, "sockcreate",	0666),
>> +	ATTR(NULL, "display",		0666),
>>  #ifdef CONFIG_SECURITY_SMACK
>>  	DIR("smack",			0555,
>>  	    proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
>> diff --git a/security/security.c b/security/security.c
>> index 46f6cf21d33c..9cfdc664239e 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache;
>>  static struct kmem_cache *lsm_inode_cache;
>>  
>>  char *lsm_names;
>> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
>> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
>> +	.lbs_task = sizeof(int),
>> +};
> This needs some comments. I know what's happening here only because I
> understand very well how the blob sizing works. :) Perhaps:
>
> .lbs_task = sizeof(int), /* storage for selected "display" LSM */

Point.

>>  
>>  /* Boot-time LSM user choice */
>>  static __initdata const char *chosen_lsm_order;
>> @@ -578,6 +580,8 @@ int lsm_inode_alloc(struct inode *inode)
>>   */
>>  static int lsm_task_alloc(struct task_struct *task)
>>  {
>> +	int *display;
>> +
>>  	if (blob_sizes.lbs_task == 0) {
>>  		task->security = NULL;
>>  		return 0;
>> @@ -586,6 +590,10 @@ static int lsm_task_alloc(struct task_struct *task)
>>  	task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
>>  	if (task->security == NULL)
>>  		return -ENOMEM;
>> +
>> +	display = task->security;
>> +	*display = LSMDATA_INVALID;
> Similarly I think a comment here would be nice. "Initialize currently
> selected "display" LSM to unselected" or something.

Point.

> Also: "int" is okay here for now, but if the LSM infrastructure wants to
> do more like this we'll want to convert to a struct of some kind at the
> start of the lbs_task.

We could go whole hog and include a lsm_info pointer (once
the slot number moves there) instead of an int, but I think
it best to leave it as is for now. I don't see a case where
more information would be required, and it would not be a
hard change to make later.

>> +
>>  	return 0;
>>  }
>>  
>> @@ -1574,14 +1582,27 @@ int security_file_open(struct file *file)
>>  
>>  int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
>>  {
>> +	int *odisplay = current->security;
>> +	int *ndisplay;
>>  	int rc = lsm_task_alloc(task);
>>  
>> -	if (rc)
>> +	if (unlikely(rc))
>>  		return rc;
>> +
>>  	rc = call_int_hook(task_alloc, 0, task, clone_flags);
>> -	if (unlikely(rc))
>> +	if (unlikely(rc)) {
>>  		security_task_free(task);
>> -	return rc;
>> +		return rc;
>> +	}
>> +
>> +	ndisplay = task->security;
>> +	if (ndisplay == NULL)
>> +		return 0;
>> +
>> +	if (odisplay != NULL)
> Perhaps merge these into "if (ndisplay && odisplay)" to drop the early
> return 0 check?

Sure. The logic took a few iterations before it got to
what you see here.

>> +		*ndisplay = *odisplay;
>> +
>> +	return 0;
>>  }
>>  
>>  void security_task_free(struct task_struct *task)
>> @@ -1967,10 +1988,28 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
>>  				char **value)
>>  {
>>  	struct security_hook_list *hp;
>> +	int *display = current->security;
>> +
>> +	if (!strcmp(name, "display")) {
>> +		hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx,
>> +				     list) {
>> +			if (*display == LSMDATA_INVALID ||
>> +			    hp->slot == *display) {
>> +				*value = kstrdup(hp->lsm, GFP_KERNEL);
>> +				if (*value)
>> +					return strlen(hp->lsm);
>> +				return -ENOMEM;
>> +			}
>> +		}
>> +		return -EINVAL;
>> +	}
>>  
>>  	hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
>>  		if (lsm != NULL && strcmp(lsm, hp->lsm))
>>  			continue;
>> +		if (lsm == NULL && *display != LSMDATA_INVALID &&
>> +		    *display != hp->slot)
>> +			continue;
>>  		return hp->hook.getprocattr(p, name, value);
>>  	}
>>  	return -EINVAL;
>> @@ -1980,10 +2019,27 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>>  			 size_t size)
>>  {
>>  	struct security_hook_list *hp;
>> +	int *display = current->security;
>> +	int len;
>> +
>> +	if (!strcmp(name, "display")) {
>> +		hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx,
>> +				     list) {
>> +			len = strlen(hp->lsm);
>> +			if (size >= len && !strncmp(value, hp->lsm, len)) {
>> +				*display = hp->slot;
>> +				return size;
>> +			}
>> +		}
>> +		return -EINVAL;
>> +	}
>>  
>>  	hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>>  		if (lsm != NULL && strcmp(lsm, hp->lsm))
>>  			continue;
>> +		if (lsm == NULL && *display != LSMDATA_INVALID &&
>> +		    *display != hp->slot)
>> +			continue;
>>  		return hp->hook.setprocattr(name, value, size);
>>  	}
>>  	return -EINVAL;
>> @@ -2002,38 +2058,41 @@ EXPORT_SYMBOL(security_ismaclabel);
>>  
>>  int security_secid_to_secctx(struct lsmblob *l, char **secdata, u32 *seclen)
>>  {
>> +	int *display = current->security;
>>  	struct security_hook_list *hp;
>> -	int rc;
>>  
>> -	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
>> -		rc = hp->hook.secid_to_secctx(l->secid[hp->slot],
>> -					      secdata, seclen);
>> -		if (rc != 0)
>> -			return rc;
>> -	}
>> +	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list)
>> +		if (*display == LSMDATA_INVALID || *display == hp->slot)
>> +			return hp->hook.secid_to_secctx(l->secid[hp->slot],
>> +							secdata, seclen);
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL(security_secid_to_secctx);
>>  
>>  int security_secctx_to_secid(const char *secdata, u32 seclen, struct lsmblob *l)
>>  {
>> +	int *display = current->security;
>>  	struct security_hook_list *hp;
>> -	int rc;
>>  
>>  	lsmblob_init(l, 0);
>> -	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
>> -		rc = hp->hook.secctx_to_secid(secdata, seclen,
>> -					      &l->secid[hp->slot]);
>> -		if (rc != 0)
>> -			return rc;
>> -	}
>> +	hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list)
>> +		if (*display == LSMDATA_INVALID || *display == hp->slot)
>> +			return hp->hook.secctx_to_secid(secdata, seclen,
>> +							&l->secid[hp->slot]);
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL(security_secctx_to_secid);
>>  
>>  void security_release_secctx(char *secdata, u32 seclen)
>>  {
>> -	call_void_hook(release_secctx, secdata, seclen);
>> +	int *display = current->security;
>> +	struct security_hook_list *hp;
>> +
>> +	hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
>> +		if (*display == LSMDATA_INVALID || *display == hp->slot) {
>> +			hp->hook.release_secctx(secdata, seclen);
>> +			return;
>> +		}
>>  }
>>  EXPORT_SYMBOL(security_release_secctx);
>>  
>> @@ -2158,8 +2217,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
>>  int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>>  				      int __user *optlen, unsigned len)
>>  {
>> -	return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
>> -				optval, optlen, len);
>> +	int *display = current->security;
>> +	struct security_hook_list *hp;
>> +
>> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
>> +			     list)
>> +		if (*display == LSMDATA_INVALID || *display == hp->slot)
>> +			return hp->hook.socket_getpeersec_stream(sock, optval,
>> +								 optlen, len);
>> +	return -ENOPROTOOPT;
>>  }
>>  
>>  int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
>> -- 
>> 2.20.1
>>




More information about the Linux-security-module-archive mailing list