Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Casey Schaufler <casey@schaufler-ca.com>
Cc: casey.schaufler@intel.com, jmorris@namei.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	john.johansen@canonical.com, penguin-kernel@i-love.sakura.ne.jp,
	paul@paul-moore.com, sds@tycho.nsa.gov
Subject: Re: [PATCH v7 15/28] LSM: Specify which LSM to display
Date: Thu, 8 Aug 2019 14:39:10 -0700
Message-ID: <201908081424.21002A3@keescook> (raw)
In-Reply-To: <20190807194410.9762-16-casey@schaufler-ca.com>

On Wed, Aug 07, 2019 at 12:43:57PM -0700, Casey Schaufler wrote:
> @@ -1980,10 +2033,48 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>  			 size_t size)
>  {
>  	struct security_hook_list *hp;
> +	char *term;
> +	char *cp;
> +	int *display = current->security;

So I went down a rat hole looking at setprocattr vs current. It looks
like everything ignores the $pid part of /proc/$pid/attr/$name and only
ever operates on "current". Is that the expected interface here?

> +	int rc = -EINVAL;
> +	int slot = 0;
> +
> +	if (!strcmp(name, "display")) {
> +		if (!capable(CAP_MAC_ADMIN))
> +			return -EPERM;
> +		/*
> +		 * lsm_slot will be 0 if there are no displaying modules.
> +		 */
> +		if (lsm_slot == 0 || size == 0)
> +			return -EINVAL;

...

> +		cp = kzalloc(size + 1, GFP_KERNEL);
> +		if (cp == NULL)
> +			return -ENOMEM;
> +		memcpy(cp, value, size);

Saving one line, the above can be:

		cp = kmemdup_nul(value, size, GFP_KERNEL);
		if (cp == NULL)
			return -ENOMEM;

> +		term = strchr(cp, ' ');
> +		if (term == NULL)
> +			term = strchr(cp, '\n');

"foo\n " will result in "foo\n". I think you want strsep() instead of
the above three lines:

		term = strsep(cp, " \n");

> +		if (term != NULL)
> +			*term = '\0';
> +
> +		for (slot = 0; slot < lsm_slot; slot++)
> +			if (!strcmp(cp, lsm_slotlist[slot]->lsm)) {
> +				*display = lsm_slotlist[slot]->slot;
> +				rc = size;
> +				break;
> +			}
> +
> +		kfree(cp);
> +		return rc;
> +	}
>  
>  	hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>  		if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>  			continue;
> +		if (lsm == NULL && *display != LSMBLOB_INVALID &&
> +		    *display != hp->lsmid->slot)
> +			continue;
>  		return hp->hook.setprocattr(name, value, size);
>  	}
>  	return -EINVAL;

Otherwise, yeah, seems good.

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

  reply index

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-07 19:43 [PATCH v7 00/28] LSM: Module stacking for AppArmor Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 01/28] LSM: Infrastructure management of the superblock Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 02/28] LSM: Infrastructure management of the sock security Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 03/28] LSM: Infrastructure management of the key blob Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 04/28] LSM: Create and manage the lsmblob data structure Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 05/28] LSM: Use lsmblob in security_audit_rule_match Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 06/28] LSM: Use lsmblob in security_kernel_act_as Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 07/28] net: Prepare UDS for security module stacking Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 08/28] LSM: Use lsmblob in security_secctx_to_secid Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 09/28] LSM: Use lsmblob in security_secid_to_secctx Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 10/28] LSM: Use lsmblob in security_ipc_getsecid Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 11/28] LSM: Use lsmblob in security_task_getsecid Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 12/28] LSM: Use lsmblob in security_inode_getsecid Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 13/28] LSM: Use lsmblob in security_cred_getsecid Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 14/28] IMA: Change internal interfaces to use lsmblobs Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 15/28] LSM: Specify which LSM to display Casey Schaufler
2019-08-08 21:39   ` Kees Cook [this message]
2019-08-08 23:38     ` Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 16/28] LSM: Ensure the correct LSM context releaser Casey Schaufler
2019-08-07 19:43 ` [PATCH v7 17/28] LSM: Use lsmcontext in security_secid_to_secctx Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 18/28] LSM: Use lsmcontext in security_dentry_init_security Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 19/28] LSM: Use lsmcontext in security_inode_getsecctx Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 20/28] LSM: security_secid_to_secctx in netlink netfilter Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 21/28] NET: Store LSM netlabel data in a lsmblob Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 22/28] SELinux: Verify LSM display sanity in binder Casey Schaufler
2019-08-08 21:55   ` Kees Cook
2019-08-09  0:56     ` Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 23/28] Audit: Add subj_LSM fields when necessary Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 24/28] Audit: Include object data for all security modules Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 25/28] LSM: Provide an user space interface for the default display Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 26/28] NET: Add SO_PEERCONTEXT for multiple LSMs Casey Schaufler
2019-08-08 22:21   ` Kees Cook
2019-08-09  0:18     ` Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 27/28] LSM: Add /proc attr entry for full LSM context Casey Schaufler
2019-08-08 22:22   ` Kees Cook
2019-08-09  0:23     ` Casey Schaufler
2019-08-07 19:44 ` [PATCH v7 28/28] AppArmor: Remove the exclusive flag Casey Schaufler

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201908081424.21002A3@keescook \
    --to=keescook@chromium.org \
    --cc=casey.schaufler@intel.com \
    --cc=casey@schaufler-ca.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=sds@tycho.nsa.gov \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org linux-security-module@archiver.kernel.org
	public-inbox-index linux-security-module


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/ public-inbox