Commit 6fba8981 authored by Casey Schaufler's avatar Casey Schaufler Committed by Paul Moore
Browse files

lsm: ensure the correct LSM context releaser 



Add a new lsm_context data structure to hold all the information about a
"security context", including the string, its size and which LSM allocated
the string. The allocation information is necessary because LSMs have
different policies regarding the lifecycle of these strings. SELinux
allocates and destroys them on each use, whereas Smack provides a pointer
to an entry in a list that never goes away.

Update security_release_secctx() to use the lsm_context instead of a
(char *, len) pair. Change its callers to do likewise.  The LSMs
supporting this hook have had comments added to remind the developer
that there is more work to be done.

The BPF security module provides all LSM hooks. While there has yet to
be a known instance of a BPF configuration that uses security contexts,
the possibility is real. In the existing implementation there is
potential for multiple frees in that case.

Cc: linux-integrity@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: audit@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: linux-nfs@vger.kernel.org
Cc: Todd Kjos <tkjos@google.com>
Signed-off-by: default avatarCasey Schaufler <casey@schaufler-ca.com>
[PM: subject tweak]
Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
parent 40384c84

drivers/android/binder.c

+12 −12
Original line number Diff line number Diff line
@@ -3017,8 +3017,7 @@ static void binder_transaction(struct binder_proc *proc, 
	struct binder_context *context = proc->context;

	int t_debug_id = atomic_inc_return(&binder_last_id);

	ktime_t t_start_time = ktime_get();

	char *secctx = NULL;

	u32 secctx_sz = 0;

	struct lsm_context lsmctx;

	struct list_head sgc_head;

	struct list_head pf_head;

	const void __user *user_buffer = (const void __user *)
@@ -3297,7 +3296,8 @@ static void binder_transaction(struct binder_proc *proc, 
		size_t added_size;



		security_cred_getsecid(proc->cred, &secid);

		ret = security_secid_to_secctx(secid, &secctx, &secctx_sz);

		ret = security_secid_to_secctx(secid, &lsmctx.context,

					       &lsmctx.len);

		if (ret) {

			binder_txn_error("%d:%d failed to get security context\n",

				thread->pid, proc->pid);
@@ -3306,7 +3306,7 @@ static void binder_transaction(struct binder_proc *proc, 
			return_error_line = __LINE__;

			goto err_get_secctx_failed;

		}

		added_size = ALIGN(secctx_sz, sizeof(u64));

		added_size = ALIGN(lsmctx.len, sizeof(u64));

		extra_buffers_size += added_size;

		if (extra_buffers_size < added_size) {

			binder_txn_error("%d:%d integer overflow of extra_buffers_size\n",
@@ -3340,23 +3340,23 @@ static void binder_transaction(struct binder_proc *proc, 
		t->buffer = NULL;

		goto err_binder_alloc_buf_failed;

	}

	if (secctx) {

	if (lsmctx.context) {

		int err;

		size_t buf_offset = ALIGN(tr->data_size, sizeof(void *)) +

				    ALIGN(tr->offsets_size, sizeof(void *)) +

				    ALIGN(extra_buffers_size, sizeof(void *)) -

				    ALIGN(secctx_sz, sizeof(u64));

				    ALIGN(lsmctx.len, sizeof(u64));



		t->security_ctx = t->buffer->user_data + buf_offset;

		err = binder_alloc_copy_to_buffer(&target_proc->alloc,

						  t->buffer, buf_offset,

						  secctx, secctx_sz);

						  lsmctx.context, lsmctx.len);

		if (err) {

			t->security_ctx = 0;

			WARN_ON(1);

		}

		security_release_secctx(secctx, secctx_sz);

		secctx = NULL;

		security_release_secctx(&lsmctx);

		lsmctx.context = NULL;

	}

	t->buffer->debug_id = t->debug_id;

	t->buffer->transaction = t;
@@ -3400,7 +3400,7 @@ static void binder_transaction(struct binder_proc *proc, 
	off_end_offset = off_start_offset + tr->offsets_size;

	sg_buf_offset = ALIGN(off_end_offset, sizeof(void *));

	sg_buf_end_offset = sg_buf_offset + extra_buffers_size -

		ALIGN(secctx_sz, sizeof(u64));

		ALIGN(lsmctx.len, sizeof(u64));

	off_min = 0;

	for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;

	     buffer_offset += sizeof(binder_size_t)) {
@@ -3779,8 +3779,8 @@ static void binder_transaction(struct binder_proc *proc, 
	binder_alloc_free_buf(&target_proc->alloc, t->buffer);

err_binder_alloc_buf_failed:

err_bad_extra_size:

	if (secctx)

		security_release_secctx(secctx, secctx_sz);

	if (lsmctx.context)

		security_release_secctx(&lsmctx);

err_get_secctx_failed:

	kfree(tcomplete);

	binder_stats_deleted(BINDER_STAT_TRANSACTION_COMPLETE);

fs/ceph/xattr.c

+5 −1
Original line number Diff line number Diff line
@@ -1446,12 +1446,16 @@ int ceph_security_init_secctx(struct dentry *dentry, umode_t mode, 


void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)

{

#ifdef CONFIG_CEPH_FS_SECURITY_LABEL

	struct lsm_context scaff; /* scaffolding */

#endif

#ifdef CONFIG_CEPH_FS_POSIX_ACL

	posix_acl_release(as_ctx->acl);

	posix_acl_release(as_ctx->default_acl);

#endif

#ifdef CONFIG_CEPH_FS_SECURITY_LABEL

	security_release_secctx(as_ctx->sec_ctx, as_ctx->sec_ctxlen);

	lsmcontext_init(&scaff, as_ctx->sec_ctx, as_ctx->sec_ctxlen, 0);

	security_release_secctx(&scaff);

#endif

#ifdef CONFIG_FS_ENCRYPTION

	kfree(as_ctx->fscrypt_auth);

fs/nfs/nfs4proc.c

+6 −2
Original line number Diff line number Diff line
@@ -138,8 +138,12 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry, 
static inline void

nfs4_label_release_security(struct nfs4_label *label)

{

	if (label)

		security_release_secctx(label->label, label->len);

	struct lsm_context scaff; /* scaffolding */



	if (label) {

		lsmcontext_init(&scaff, label->label, label->len, 0);

		security_release_secctx(&scaff);

	}

}

static inline u32 *nfs4_bitmask(struct nfs_server *server, struct nfs4_label *label)

{

fs/nfsd/nfs4xdr.c

+6 −2
Original line number Diff line number Diff line
@@ -3644,8 +3644,12 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr, 


out:

#ifdef CONFIG_NFSD_V4_SECURITY_LABEL

	if (args.context)

		security_release_secctx(args.context, args.contextlen);

	if (args.context) {

		struct lsm_context scaff; /* scaffolding */



		lsmcontext_init(&scaff, args.context, args.contextlen, 0);

		security_release_secctx(&scaff);

	}

#endif /* CONFIG_NFSD_V4_SECURITY_LABEL */

	kfree(args.acl);

	if (tempfh) {

include/linux/lsm_hook_defs.h

+1 −1
Original line number Diff line number Diff line
@@ -300,7 +300,7 @@ LSM_HOOK(int, -EOPNOTSUPP, secid_to_secctx, u32 secid, char **secdata, 
LSM_HOOK(int, -EOPNOTSUPP, lsmprop_to_secctx, struct lsm_prop *prop,

	 char **secdata, u32 *seclen)

LSM_HOOK(int, 0, secctx_to_secid, const char *secdata, u32 seclen, u32 *secid)

LSM_HOOK(void, LSM_RET_VOID, release_secctx, char *secdata, u32 seclen)

LSM_HOOK(void, LSM_RET_VOID, release_secctx, struct lsm_context *cp)

LSM_HOOK(void, LSM_RET_VOID, inode_invalidate_secctx, struct inode *inode)

LSM_HOOK(int, 0, inode_notifysecctx, struct inode *inode, void *ctx, u32 ctxlen)

LSM_HOOK(int, 0, inode_setsecctx, struct dentry *dentry, void *ctx, u32 ctxlen)