Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Sean Christopherson @ 2019-03-19 20:09 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: x86, linux-sgx, akpm, dave.hansen, nhorman, npmccallum,
	serge.ayoun, shay.katz-zamir, haitao.huang, andriy.shevchenko,
	tglx, kai.svahn, bp, josh, luto, kai.huang, rientjes,
	James Morris, Serge E . Hallyn, linux-security-module
In-Reply-To: <20190317211456.13927-18-jarkko.sakkinen@linux.intel.com>

On Sun, Mar 17, 2019 at 11:14:46PM +0200, Jarkko Sakkinen wrote:
> In order to provide a mechanism for devilering provisoning rights:
> 
> 1. Add a new file to the securityfs file called sgx/provision that works
>    as a token for allowing an enclave to have the provisioning privileges.
> 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the
>    following data structure:
> 
>    struct sgx_enclave_set_attribute {
>            __u64 addr;
>            __u64 token_fd;
>    };
> 
> A daemon could sit on top of sgx/provision and send a file descriptor of
> this file to a process that needs to be able to provision enclaves.
> 
> The way this API is used is more or less straight-forward. Lets assume that
> dev_fd is a handle to /dev/sgx and prov_fd is a handle to sgx/provision.
> You would allow SGX_IOC_ENCLAVE_CREATE to initialize an enclave with the
> PROVISIONKEY attribute by
> 
> params.addr = <enclave address>;
> params.token_fd = prov_fd;
> 
> ioctl(dev_fd, SGX_IOC_ENCLAVE_SET_ATTRIBUTE, &params);
> 
> Cc: James Morris <jmorris@namei.org>
> Cc: Serge E. Hallyn <serge@hallyn.com>
> Cc: linux-security-module@vger.kernel.org
> Suggested-by: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  arch/x86/include/uapi/asm/sgx.h        | 13 +++++++
>  arch/x86/kernel/cpu/sgx/driver/ioctl.c | 43 +++++++++++++++++++++++
>  arch/x86/kernel/cpu/sgx/driver/main.c  | 47 ++++++++++++++++++++++++++
>  3 files changed, 103 insertions(+)
> 
> diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
> index aadf9c76e360..150a784db395 100644
> --- a/arch/x86/include/uapi/asm/sgx.h
> +++ b/arch/x86/include/uapi/asm/sgx.h
> @@ -16,6 +16,8 @@
>  	_IOW(SGX_MAGIC, 0x01, struct sgx_enclave_add_page)
>  #define SGX_IOC_ENCLAVE_INIT \
>  	_IOW(SGX_MAGIC, 0x02, struct sgx_enclave_init)
> +#define SGX_IOC_ENCLAVE_SET_ATTRIBUTE \
> +	_IOW(SGX_MAGIC, 0x03, struct sgx_enclave_set_attribute)
>  
>  /* IOCTL return values */
>  #define SGX_POWER_LOST_ENCLAVE		0x40000000
> @@ -56,4 +58,15 @@ struct sgx_enclave_init {
>  	__u64	sigstruct;
>  };
>  
> +/**
> + * struct sgx_enclave_set_attribute - parameter structure for the
> + *				      %SGX_IOC_ENCLAVE_INIT ioctl
> + * @addr:		address within the ELRANGE
> + * @attribute_fd:	file handle of the attribute file in the securityfs
> + */
> +struct sgx_enclave_set_attribute {
> +	__u64	addr;
> +	__u64	attribute_fd;
> +};
> +
>  #endif /* _UAPI_ASM_X86_SGX_H */
> diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> index 4b9a91b53b50..5d85bd3f7876 100644
> --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> @@ -759,6 +759,46 @@ static long sgx_ioc_enclave_init(struct file *filep, unsigned int cmd,
>  	return ret;
>  }
>  
> +/**
> + * sgx_ioc_enclave_set_attribute - handler for %SGX_IOC_ENCLAVE_SET_ATTRIBUTE
> + * @filep:	open file to /dev/sgx
> + * @cmd:	the command value
> + * @arg:	pointer to a struct sgx_enclave_set_attribute instance
> + *
> + * Sets an attribute matching the attribute file that is pointed by the
> + * parameter structure field attribute_fd.

With the @data change (see below), this becomes something like:

 * Allow the enclave to request the attribute managed by the SGX security file
 * pointed at by the parameter structure field attribute_fd.

> + *
> + * Return: 0 on success, -errno otherwise
> + */
> +static long sgx_ioc_enclave_set_attribute(struct file *filep, unsigned int cmd,
> +					  unsigned long arg)
> +{
> +	struct sgx_enclave_set_attribute *params = (void *)arg;
> +	struct file *attribute_file;
> +	struct sgx_encl *encl;
> +	int ret;
> +
> +	attribute_file = fget(params->attribute_fd);
> +	if (!attribute_file->f_op)

This should be:

	if (!attribute_file)
		return -EINVAL;

> +		return -EINVAL;
> +
> +	if (attribute_file->f_op != &sgx_fs_provision_fops) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ret = sgx_encl_get(params->addr, &encl);
> +	if (ret)
> +		goto out;
> +
> +	encl->allowed_attributes |= SGX_ATTR_PROVISIONKEY;

A cleanr approach would be to pass SGX_ATTR_PROVISIONKEY via @data to
securityfs_create_file().  Then you don't need to define dummy file_ops
for each file, i.e. a generic sgx_sec_fs_ops would suffice for the above
check.  And you don't have this weird hardcoding of the provision bit.

E.g.:

	if (attribute_file->f_op != &sgx_sec_fs_fops) {
		ret = -EINVAL;
		goto out;
	}

	ret = sgx_encl_get(params->addr, &encl);
	if (ret)
		goto out;

	encl->allowed_attributes |= (u64)attribute_file->private_data;

Since SGX doesn't support 32-bit builds we don't even need to worry about
the (very distant) future where SGX defines bits in the 63:32 range.


> +	kref_put(&encl->refcount, sgx_encl_release);
> +
> +out:
> +	fput(attribute_file);
> +	return ret;
> +}
> +
>  typedef long (*sgx_ioc_t)(struct file *filep, unsigned int cmd,
>  			  unsigned long arg);
>  
> @@ -778,6 +818,9 @@ long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
>  	case SGX_IOC_ENCLAVE_INIT:
>  		handler = sgx_ioc_enclave_init;
>  		break;
> +	case SGX_IOC_ENCLAVE_SET_ATTRIBUTE:
> +		handler = sgx_ioc_enclave_set_attribute;
> +		break;
>  	default:
>  		return -ENOIOCTLCMD;
>  	}
> diff --git a/arch/x86/kernel/cpu/sgx/driver/main.c b/arch/x86/kernel/cpu/sgx/driver/main.c
> index 16f36cd0af04..9a5360dcad98 100644
> --- a/arch/x86/kernel/cpu/sgx/driver/main.c
> +++ b/arch/x86/kernel/cpu/sgx/driver/main.c
> @@ -22,6 +22,11 @@ u64 sgx_attributes_reserved_mask;
>  u64 sgx_xfrm_reserved_mask = ~0x3;
>  u32 sgx_xsave_size_tbl[64];
>  
> +const struct file_operations sgx_fs_provision_fops;
> +
> +static struct dentry *sgx_fs;
> +static struct dentry *sgx_fs_provision;
> +
>  #ifdef CONFIG_COMPAT
>  static long sgx_compat_ioctl(struct file *filep, unsigned int cmd,
>  			      unsigned long arg)
> @@ -147,6 +152,40 @@ static struct sgx_dev_ctx *sgxm_dev_ctx_alloc(struct device *parent)
>  	return ctx;
>  }
>  
> +static int sgx_fs_init(struct device *dev)
> +{
> +	int ret;
> +
> +	sgx_fs = securityfs_create_dir(dev_name(dev), NULL);
> +	if (IS_ERR(sgx_fs)) {
> +		ret = PTR_ERR(sgx_fs);
> +		goto err_sgx_fs;
> +	}
> +
> +	sgx_fs_provision = securityfs_create_file("provision", 0600, sgx_fs,
> +						  NULL, &sgx_fs_provision_fops);

Per above, pass SGX_ATTR_PROVISIONKEY instead of NULL.

> +	if (IS_ERR(sgx_fs)) {
> +		ret = PTR_ERR(sgx_fs_provision);
> +		goto err_sgx_fs_provision;
> +	}
> +
> +	return 0;
> +
> +err_sgx_fs_provision:
> +	securityfs_remove(sgx_fs);
> +	sgx_fs_provision = NULL;
> +
> +err_sgx_fs:
> +	sgx_fs = NULL;
> +	return ret;
> +}
> +
> +static void sgx_fs_remove(void)
> +{
> +	securityfs_remove(sgx_fs_provision);
> +	securityfs_remove(sgx_fs);
> +}
> +
>  static int sgx_dev_init(struct device *parent)
>  {
>  	struct sgx_dev_ctx *sgx_dev;
> @@ -190,6 +229,10 @@ static int sgx_dev_init(struct device *parent)
>  	if (!sgx_encl_wq)
>  		return -ENOMEM;
>  
> +	ret = sgx_fs_init(&sgx_dev->ctrl_dev);
> +	if (ret)
> +		goto err_fs_init;
> +
>  	ret = cdev_device_add(&sgx_dev->ctrl_cdev, &sgx_dev->ctrl_dev);
>  	if (ret)
>  		goto err_device_add;
> @@ -197,6 +240,9 @@ static int sgx_dev_init(struct device *parent)
>  	return 0;
>  
>  err_device_add:
> +	sgx_fs_remove();
> +
> +err_fs_init:
>  	destroy_workqueue(sgx_encl_wq);
>  	return ret;
>  }
> @@ -220,6 +266,7 @@ static int sgx_drv_remove(struct platform_device *pdev)
>  {
>  	struct sgx_dev_ctx *ctx = dev_get_drvdata(&pdev->dev);
>  
> +	sgx_fs_remove();
>  	cdev_device_del(&ctx->ctrl_cdev, &ctx->ctrl_dev);
>  	destroy_workqueue(sgx_encl_wq);
>  
> -- 
> 2.19.1
> 

^ permalink raw reply

* Re: [PATCH] device_cgroup: fix RCU imbalance in error case
From: Tejun Heo @ 2019-03-19 17:47 UTC (permalink / raw)
  To: Jann Horn
  Cc: James Morris, Serge E. Hallyn, Li Zefan, Johannes Weiner,
	linux-security-module, linux-kernel, Aristeu Rozanski,
	Serge E . Hallyn, Michal Hocko, cgroups
In-Reply-To: <20190319013659.86199-1-jannh@google.com>

On Tue, Mar 19, 2019 at 02:36:59AM +0100, Jann Horn wrote:
> When dev_exception_add() returns an error (due to a failed memory
> allocation), make sure that we move the RCU preemption count back to where
> it was before we were called. We dropped the RCU read lock inside the loop
> body, so we can't just "break".
> 
> sparse complains about this, too:
> 
> $ make -s C=2 security/device_cgroup.o
> ./include/linux/rcupdate.h:647:9: warning: context imbalance in
> 'propagate_exception' - unexpected unlock
> 
> Fixes: d591fb56618f ("device_cgroup: simplify cgroup tree walk in propagate_exception()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>

Applied to cgroup/for-5.1-fixes.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
From: Eric Biggers @ 2019-03-19 17:09 UTC (permalink / raw)
  To: Geert Uytterhoeven, Kees Cook
  Cc: Herbert Xu, linux-security-module, Linux ARM,
	Linux Crypto Mailing List, Linux Kernel Mailing List
In-Reply-To: <CAMuHMdWhx0-kspDyLOgTp+9udiBpkMHULSGyMg9uPPV16=FWQg@mail.gmail.com>

On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> When running the sha1-asm crypto selftest on arm with
> CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> 
>     usercopy: Kernel memory overwrite attempt detected to spans
> multiple pages (offset 0, size 42)!
>     ------------[ cut here ]------------
>     kernel BUG at mm/usercopy.c:102!
>     Internal error: Oops - BUG: 0 [#1] SMP ARM
>     Modules linked in:
>     CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
>     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
>     PC is at usercopy_abort+0x68/0x90
>     LR is at usercopy_abort+0x68/0x90
>     pc : [<c030fd60>]    lr : [<c030fd60>]    psr: 60000013
>     sp : ea54bc60  ip : 00000010  fp : cccccccd
>     r10: 00000000  r9 : c0e0ce04  r8 : ea54d009
>     r7 : ea54d00a  r6 : 00000000  r5 : 0000002a  r4 : c09d1120
>     r3 : dd6cd422  r2 : dd6cd422  r1 : 2abb4000  r0 : 0000005f
>     Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
>     Control: 30c5387d  Table: 40003000  DAC: fffffffd
>     Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
>     Stack: (0xea54bc60 to 0xea54c000)
>     bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> 00000000 c0310060
>     bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> ea54cfe0 ea538c00
>     bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> 0000003a c0427a30
>     bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> 0000002a c081cf80
>     bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> c0e0a408 eb074240
>     bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> ebef7480 eb074200
>     bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> c084061c c0428c38
>     bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> 00000002 eabe4e80
>     bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> 00000006 c09d544c
>     bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> eb074200 00000000
>     bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> 00000000 c081cf70
>     bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> c0e4ee80 443e9884
>     bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> fefefefe fefefefe
>     be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> ea4f7a00 c03062bc
>     be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> 00000002 eabe4e40
>     be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> eb074200 ea538c00
>     be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> c04dace8 00000006
>     be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> ea4f71a8 c0429420
>     bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> 00000400 ffffffff
>     bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> c0e5a2a0 c0e5a2a0
>     bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> c0e5a2a0 c0269470
>     bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> ea537280 0000000e
>     bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> c09c9ed0 ea54bf5c
>     bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> c09c9ed0 ea537200
>     bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> ea4f71a8 c0426524
>     bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> 00000000 00000000
>     bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> 00000000 00000000
>     bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
>     bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> 00000000 00000000
>     [<c030fd60>] (usercopy_abort) from [<c0310060>]
> (__check_object_size+0x2d8/0x448)
>     [<c0310060>] (__check_object_size) from [<c0427a30>]
> (build_test_sglist+0x268/0x2d8)
>     [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> (test_hash_vec_cfg+0x110/0x694)
>     [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> (__alg_test_hash+0x158/0x1b8)
>     [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
>     [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
>     [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
>     [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
>     [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
>     Exception stack(0xea54bfb0 to 0xea54bff8)
>     bfa0:                                     00000000 00000000
> 00000000 00000000
>     bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
>     bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
>     Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
>     ---[ end trace 190b3cf48e720f78 ]---
>     BUG: sleeping function called from invalid context at
> include/linux/percpu-rwsem.h:34
>     in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
>     CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G      D
> 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
>     Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
>     [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
>     [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
>     [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
>     [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
>     [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
>     [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
>     [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
>     [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
>     Exception stack(0xea54bc10 to 0xea54bc58)
>     bc00:                                     0000005f 2abb4000
> dd6cd422 dd6cd422
>     bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> 00000000 cccccccd
>     bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
>     [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
>     [<c030fd60>] (usercopy_abort) from [<c0310060>]
> (__check_object_size+0x2d8/0x448)
>     [<c0310060>] (__check_object_size) from [<c0427a30>]
> (build_test_sglist+0x268/0x2d8)
>     [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> (test_hash_vec_cfg+0x110/0x694)
>     [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> (__alg_test_hash+0x158/0x1b8)
>     [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
>     [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
>     [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
>     [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
>     [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
>     Exception stack(0xea54bfb0 to 0xea54bff8)
>     bfa0:                                     00000000 00000000
> 00000000 00000000
>     bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
>     bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> 
> A similar trace is seen with sha1-ce on arm64:
> 
>     usercopy: Kernel memory overwrite attempt detected to spans
> multiple pages (offset 0, size 42)!
>     ------------[ cut here ]------------
>     kernel BUG at mm/usercopy.c:102!
>     Internal error: Oops - BUG: 0 [#1] SMP
>     Modules linked in:
>     CPU: 1 PID: 33 Comm: cryptomgr_test Not tainted
> 5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
>     Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
>     pstate: 60400005 (nZCv daif +PAN -UAO)
>     pc : usercopy_abort+0x64/0x90
>     lr : usercopy_abort+0x64/0x90
>     sp : ffffff8011eb38d0
>     x29: ffffff8011eb38e0 x28: 6db6db6db6db6db7
>     x27: ffffffbf00000000 x26: 0000000000000038
>     x25: ffffffc0778fd009 x24: 0000000000000000
>     x23: ffffffc0778fd00a x22: ffffff8010d51000
>     x21: 0000000000000000 x20: 000000000000002a
>     x19: ffffffc0778fcfe0 x18: 000000000000000a
>     x17: 00000000526a1be5 x16: 0000000000000014
>     x15: 000000000009f6c2 x14: 0720072007200720
>     x13: 0720072007200720 x12: 0720072007200720
>     x11: 0720072007200720 x10: 0720072007200720
>     x9 : ffffff80110126c8 x8 : 0000000000000000
>     x7 : ffffff801015700c x6 : 0000000000000000
>     x5 : 0000000000000000 x4 : ffffff8011eb4000
>     x3 : 0000000000000080 x2 : a045404094166600
>     x1 : 0000000000000000 x0 : 000000000000005f
>     Process cryptomgr_test (pid: 33, stack limit = 0x(____ptrval____))
>     Call trace:
>      usercopy_abort+0x64/0x90
>      __check_object_size+0x64/0x464
>      build_test_sglist+0x238/0x2c8
>      test_hash_vec_cfg+0x130/0x660
>      __alg_test_hash+0x1b4/0x1f4
>      alg_test_hash+0x88/0x104
>      alg_test.part.6+0x2a8/0x330
>      alg_test+0x98/0xa0
>      cryptomgr_test+0x24/0x4c
>      kthread+0x120/0x130
>      ret_from_fork+0x10/0x18
>     Code: aa0003e3 b00053e0 91148000 97fc3bf2 (d4210000)
>     ---[ end trace d9f3261d50a7f84f ]---
>     BUG: sleeping function called from invalid context at
> include/linux/percpu-rwsem.h:34
>     in_atomic(): 0, irqs_disabled(): 128, pid: 33, name: cryptomgr_test
>     INFO: lockdep is turned off.
>     irq event stamp: 262
>     hardirqs last  enabled at (261): [<ffffff8010157050>]
> console_unlock+0x554/0x560
>     hardirqs last disabled at (262): [<ffffff8010081a28>]
> do_debug_exception+0x48/0x13c
>     softirqs last  enabled at (258): [<ffffff8010081ee4>]
> __do_softirq+0x18c/0x4a0
>     softirqs last disabled at (245): [<ffffff80100f3e10>] irq_exit+0xa4/0x100
>     CPU: 1 PID: 33 Comm: cryptomgr_test Tainted: G      D
> 5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
>     Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
>     Call trace:
>      dump_backtrace+0x0/0x118
>      show_stack+0x14/0x1c
>      dump_stack+0xc8/0x118
>      ___might_sleep+0x24c/0x25c
>      __might_sleep+0x70/0x80
>      exit_signals+0x48/0x278
>      do_exit+0x10c/0xa30
>      die+0x1f4/0x208
>      bug_handler+0x4c/0x78
>      brk_handler+0x15c/0x188
>      do_debug_exception+0xd4/0x13c
>      el1_dbg+0x18/0xbc
>      usercopy_abort+0x64/0x90
>      __check_object_size+0x64/0x464
>      build_test_sglist+0x238/0x2c8
>      test_hash_vec_cfg+0x130/0x660
>      __alg_test_hash+0x1b4/0x1f4
>      alg_test_hash+0x88/0x104
>      alg_test.part.6+0x2a8/0x330
>      alg_test+0x98/0xa0
>      cryptomgr_test+0x24/0x4c
>      kthread+0x120/0x130
>      ret_from_fork+0x10/0x18
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Well, this must happen with the new (in 5.1) crypto self-tests implementation
for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y.  I don't
understand why hardened usercopy considers it a bug though, as there's no buffer
overflow.  The crypto tests use copy_from_iter() to copy data into a 2-page
buffer that was allocated with __get_free_pages():

	__get_free_pages(GFP_KERNEL, 1)

... where 1 means an order-1 allocation.

If it copies to offset=4064 len=42, for example, then hardened usercopy
considers it a bug even though the buffer is 8192 bytes long.  Why?

It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
with ITER_KVEC.

- Eric

^ permalink raw reply

* Re: mount.nfs: Protocol error after upgrade to linux/master
From: Casey Schaufler @ 2019-03-19 15:03 UTC (permalink / raw)
  To: Tetsuo Handa, Kees Cook
  Cc: Jakub Kicinski, linux-security-module, Trond Myklebust,
	open list:NFS, SUNRPC, AND..., Anna Schumaker, LKML
In-Reply-To: <2bf23acd-22c4-a260-7648-845887a409d5@i-love.sakura.ne.jp>

On 3/19/2019 3:56 AM, Tetsuo Handa wrote:
> Since Kees Cook seems to be busy now, here is my version...
>
>  From 885553e4793d9af2d4e9e99c7d137b0ec7b5f8ad Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Tue, 19 Mar 2019 19:52:31 +0900
> Subject: [PATCH] LSM: Revive CONFIG_DEFAULT_SECURITY_* for "make oldconfig"
>
> Commit 70b62c25665f636c ("LoadPin: Initialize as ordered LSM") removed
> CONFIG_DEFAULT_SECURITY_{SELINUX,SMACK,TOMOYO,APPARMOR,DAC} from
> security/Kconfig and changed CONFIG_LSM to provide a fixed ordering as a
> default value. That commit expected that existing users (upgrading from
> Linux 5.0 and earlier) will edit CONFIG_LSM value in accordance with
> their CONFIG_DEFAULT_SECURITY_* choice in their old kernel configs. But
> since users might forget to edit CONFIG_LSM value, this patch revives
> the choice (only for providing the default value for CONFIG_LSM) in order
> to make sure that CONFIG_LSM reflects CONFIG_DEFAULT_SECURITY_* from their
> old kernel configs.
>
> Reported-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>   security/Kconfig | 36 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/security/Kconfig b/security/Kconfig
> index 1d6463f..743e594 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -239,9 +239,43 @@ source "security/safesetid/Kconfig"
>   
>   source "security/integrity/Kconfig"
>   
> +choice
> +	prompt "Default security module [superseded by 'Ordered list of enabled LSMs' below]"
> +	default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
> +	default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
> +	default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
> +	default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
> +	default DEFAULT_SECURITY_DAC
> +
> +	help
> +	  This choice is there only for converting CONFIG_DEFAULT_SECURITY in old
> +	  kernel config to CONFIG_LSM in new kernel config. Don't change this choice
> +	  unless you are creating a fresh kernel config, for this choice will be
> +	  ignored after CONFIG_LSM is once defined.
> +
> +	config DEFAULT_SECURITY_SELINUX
> +		bool "SELinux" if SECURITY_SELINUX=y
> +
> +	config DEFAULT_SECURITY_SMACK
> +		bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y
> +
> +	config DEFAULT_SECURITY_TOMOYO
> +		bool "TOMOYO" if SECURITY_TOMOYO=y
> +
> +	config DEFAULT_SECURITY_APPARMOR
> +		bool "AppArmor" if SECURITY_APPARMOR=y
> +	config DEFAULT_SECURITY_DAC
> +		bool "Unix Discretionary Access Controls"
> +
> +endchoice
> +
>   config LSM
>   	string "Ordered list of enabled LSMs"
> -	default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> +	default "yama,loadpin,safesetid,integrity,selinux" if DEFAULT_SECURITY_SELINUX
> +	default "yama,loadpin,safesetid,integrity,smack" if DEFAULT_SECURITY_SMACK
> +	default "yama,loadpin,safesetid,integrity,tomoyo" if DEFAULT_SECURITY_TOMOYO
> +	default "yama,loadpin,safesetid,integrity,apparmor" if DEFAULT_SECURITY_APPARMOR
> +	default "yama,loadpin,safesetid,integrity"
>   	help
>   	  A comma-separated list of LSMs, in initialization order.
>   	  Any LSMs left off this list will be ignored. This can be

^ permalink raw reply

* crypto: Kernel memory overwrite attempt detected to spans multiple pages
From: Geert Uytterhoeven @ 2019-03-19 11:54 UTC (permalink / raw)
  To: Kees Cook, Herbert Xu
  Cc: linux-security-module, Linux ARM, Linux Crypto Mailing List,
	Linux Kernel Mailing List

When running the sha1-asm crypto selftest on arm with
CONFIG_HARDENED_USERCOPY_PAGESPAN=y:

    usercopy: Kernel memory overwrite attempt detected to spans
multiple pages (offset 0, size 42)!
    ------------[ cut here ]------------
    kernel BUG at mm/usercopy.c:102!
    Internal error: Oops - BUG: 0 [#1] SMP ARM
    Modules linked in:
    CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
    Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
    PC is at usercopy_abort+0x68/0x90
    LR is at usercopy_abort+0x68/0x90
    pc : [<c030fd60>]    lr : [<c030fd60>]    psr: 60000013
    sp : ea54bc60  ip : 00000010  fp : cccccccd
    r10: 00000000  r9 : c0e0ce04  r8 : ea54d009
    r7 : ea54d00a  r6 : 00000000  r5 : 0000002a  r4 : c09d1120
    r3 : dd6cd422  r2 : dd6cd422  r1 : 2abb4000  r0 : 0000005f
    Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
    Control: 30c5387d  Table: 40003000  DAC: fffffffd
    Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
    Stack: (0xea54bc60 to 0xea54c000)
    bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
00000000 c0310060
    bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
ea54cfe0 ea538c00
    bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
0000003a c0427a30
    bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
0000002a c081cf80
    bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
c0e0a408 eb074240
    bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
ebef7480 eb074200
    bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
c084061c c0428c38
    bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
00000002 eabe4e80
    bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
00000006 c09d544c
    bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
eb074200 00000000
    bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
00000000 c081cf70
    bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
c0e4ee80 443e9884
    bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
fefefefe fefefefe
    be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
ea4f7a00 c03062bc
    be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
00000002 eabe4e40
    be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
eb074200 ea538c00
    be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
c04dace8 00000006
    be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
ea4f71a8 c0429420
    bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
00000400 ffffffff
    bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
c0e5a2a0 c0e5a2a0
    bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
c0e5a2a0 c0269470
    bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
ea537280 0000000e
    bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
c09c9ed0 ea54bf5c
    bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
c09c9ed0 ea537200
    bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
ea4f71a8 c0426524
    bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
00000000 00000000
    bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
00000000 00000000
    bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
    bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
00000000 00000000
    [<c030fd60>] (usercopy_abort) from [<c0310060>]
(__check_object_size+0x2d8/0x448)
    [<c0310060>] (__check_object_size) from [<c0427a30>]
(build_test_sglist+0x268/0x2d8)
    [<c0427a30>] (build_test_sglist) from [<c0428c38>]
(test_hash_vec_cfg+0x110/0x694)
    [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
(__alg_test_hash+0x158/0x1b8)
    [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
    [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
    [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
    [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
    [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
    Exception stack(0xea54bfb0 to 0xea54bff8)
    bfa0:                                     00000000 00000000
00000000 00000000
    bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
    bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
    Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
    ---[ end trace 190b3cf48e720f78 ]---
    BUG: sleeping function called from invalid context at
include/linux/percpu-rwsem.h:34
    in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
    CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G      D
5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
    Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
    [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
    [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
    [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
    [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
    [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
    [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
    [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
    [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
    Exception stack(0xea54bc10 to 0xea54bc58)
    bc00:                                     0000005f 2abb4000
dd6cd422 dd6cd422
    bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
00000000 cccccccd
    bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
    [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
    [<c030fd60>] (usercopy_abort) from [<c0310060>]
(__check_object_size+0x2d8/0x448)
    [<c0310060>] (__check_object_size) from [<c0427a30>]
(build_test_sglist+0x268/0x2d8)
    [<c0427a30>] (build_test_sglist) from [<c0428c38>]
(test_hash_vec_cfg+0x110/0x694)
    [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
(__alg_test_hash+0x158/0x1b8)
    [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
    [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
    [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
    [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
    [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
    Exception stack(0xea54bfb0 to 0xea54bff8)
    bfa0:                                     00000000 00000000
00000000 00000000
    bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
    bfe0: 00000000 00000000 00000000 00000000 00000013 00000000

A similar trace is seen with sha1-ce on arm64:

    usercopy: Kernel memory overwrite attempt detected to spans
multiple pages (offset 0, size 42)!
    ------------[ cut here ]------------
    kernel BUG at mm/usercopy.c:102!
    Internal error: Oops - BUG: 0 [#1] SMP
    Modules linked in:
    CPU: 1 PID: 33 Comm: cryptomgr_test Not tainted
5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
    Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
    pstate: 60400005 (nZCv daif +PAN -UAO)
    pc : usercopy_abort+0x64/0x90
    lr : usercopy_abort+0x64/0x90
    sp : ffffff8011eb38d0
    x29: ffffff8011eb38e0 x28: 6db6db6db6db6db7
    x27: ffffffbf00000000 x26: 0000000000000038
    x25: ffffffc0778fd009 x24: 0000000000000000
    x23: ffffffc0778fd00a x22: ffffff8010d51000
    x21: 0000000000000000 x20: 000000000000002a
    x19: ffffffc0778fcfe0 x18: 000000000000000a
    x17: 00000000526a1be5 x16: 0000000000000014
    x15: 000000000009f6c2 x14: 0720072007200720
    x13: 0720072007200720 x12: 0720072007200720
    x11: 0720072007200720 x10: 0720072007200720
    x9 : ffffff80110126c8 x8 : 0000000000000000
    x7 : ffffff801015700c x6 : 0000000000000000
    x5 : 0000000000000000 x4 : ffffff8011eb4000
    x3 : 0000000000000080 x2 : a045404094166600
    x1 : 0000000000000000 x0 : 000000000000005f
    Process cryptomgr_test (pid: 33, stack limit = 0x(____ptrval____))
    Call trace:
     usercopy_abort+0x64/0x90
     __check_object_size+0x64/0x464
     build_test_sglist+0x238/0x2c8
     test_hash_vec_cfg+0x130/0x660
     __alg_test_hash+0x1b4/0x1f4
     alg_test_hash+0x88/0x104
     alg_test.part.6+0x2a8/0x330
     alg_test+0x98/0xa0
     cryptomgr_test+0x24/0x4c
     kthread+0x120/0x130
     ret_from_fork+0x10/0x18
    Code: aa0003e3 b00053e0 91148000 97fc3bf2 (d4210000)
    ---[ end trace d9f3261d50a7f84f ]---
    BUG: sleeping function called from invalid context at
include/linux/percpu-rwsem.h:34
    in_atomic(): 0, irqs_disabled(): 128, pid: 33, name: cryptomgr_test
    INFO: lockdep is turned off.
    irq event stamp: 262
    hardirqs last  enabled at (261): [<ffffff8010157050>]
console_unlock+0x554/0x560
    hardirqs last disabled at (262): [<ffffff8010081a28>]
do_debug_exception+0x48/0x13c
    softirqs last  enabled at (258): [<ffffff8010081ee4>]
__do_softirq+0x18c/0x4a0
    softirqs last disabled at (245): [<ffffff80100f3e10>] irq_exit+0xa4/0x100
    CPU: 1 PID: 33 Comm: cryptomgr_test Tainted: G      D
5.1.0-rc1-salvator-x-01109-gbeb7d6376ecfbf07-dirty #352
    Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
    Call trace:
     dump_backtrace+0x0/0x118
     show_stack+0x14/0x1c
     dump_stack+0xc8/0x118
     ___might_sleep+0x24c/0x25c
     __might_sleep+0x70/0x80
     exit_signals+0x48/0x278
     do_exit+0x10c/0xa30
     die+0x1f4/0x208
     bug_handler+0x4c/0x78
     brk_handler+0x15c/0x188
     do_debug_exception+0xd4/0x13c
     el1_dbg+0x18/0xbc
     usercopy_abort+0x64/0x90
     __check_object_size+0x64/0x464
     build_test_sglist+0x238/0x2c8
     test_hash_vec_cfg+0x130/0x660
     __alg_test_hash+0x1b4/0x1f4
     alg_test_hash+0x88/0x104
     alg_test.part.6+0x2a8/0x330
     alg_test+0x98/0xa0
     cryptomgr_test+0x24/0x4c
     kthread+0x120/0x130
     ret_from_fork+0x10/0x18

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: mount.nfs: Protocol error after upgrade to linux/master
From: Tetsuo Handa @ 2019-03-19 10:56 UTC (permalink / raw)
  To: Casey Schaufler, Kees Cook
  Cc: Jakub Kicinski, linux-security-module, Trond Myklebust,
	open list:NFS, SUNRPC, AND..., Anna Schumaker, LKML
In-Reply-To: <bee1a774-461a-2535-a640-bbe65291e909@schaufler-ca.com>

Since Kees Cook seems to be busy now, here is my version...

From 885553e4793d9af2d4e9e99c7d137b0ec7b5f8ad Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 19 Mar 2019 19:52:31 +0900
Subject: [PATCH] LSM: Revive CONFIG_DEFAULT_SECURITY_* for "make oldconfig"

Commit 70b62c25665f636c ("LoadPin: Initialize as ordered LSM") removed
CONFIG_DEFAULT_SECURITY_{SELINUX,SMACK,TOMOYO,APPARMOR,DAC} from
security/Kconfig and changed CONFIG_LSM to provide a fixed ordering as a
default value. That commit expected that existing users (upgrading from
Linux 5.0 and earlier) will edit CONFIG_LSM value in accordance with
their CONFIG_DEFAULT_SECURITY_* choice in their old kernel configs. But
since users might forget to edit CONFIG_LSM value, this patch revives
the choice (only for providing the default value for CONFIG_LSM) in order
to make sure that CONFIG_LSM reflects CONFIG_DEFAULT_SECURITY_* from their
old kernel configs.

Reported-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 security/Kconfig | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/security/Kconfig b/security/Kconfig
index 1d6463f..743e594 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -239,9 +239,43 @@ source "security/safesetid/Kconfig"
 
 source "security/integrity/Kconfig"
 
+choice
+	prompt "Default security module [superseded by 'Ordered list of enabled LSMs' below]"
+	default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
+	default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
+	default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
+	default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
+	default DEFAULT_SECURITY_DAC
+
+	help
+	  This choice is there only for converting CONFIG_DEFAULT_SECURITY in old
+	  kernel config to CONFIG_LSM in new kernel config. Don't change this choice
+	  unless you are creating a fresh kernel config, for this choice will be
+	  ignored after CONFIG_LSM is once defined.
+
+	config DEFAULT_SECURITY_SELINUX
+		bool "SELinux" if SECURITY_SELINUX=y
+
+	config DEFAULT_SECURITY_SMACK
+		bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y
+
+	config DEFAULT_SECURITY_TOMOYO
+		bool "TOMOYO" if SECURITY_TOMOYO=y
+
+	config DEFAULT_SECURITY_APPARMOR
+		bool "AppArmor" if SECURITY_APPARMOR=y
+	config DEFAULT_SECURITY_DAC
+		bool "Unix Discretionary Access Controls"
+
+endchoice
+
 config LSM
 	string "Ordered list of enabled LSMs"
-	default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
+	default "yama,loadpin,safesetid,integrity,selinux" if DEFAULT_SECURITY_SELINUX
+	default "yama,loadpin,safesetid,integrity,smack" if DEFAULT_SECURITY_SMACK
+	default "yama,loadpin,safesetid,integrity,tomoyo" if DEFAULT_SECURITY_TOMOYO
+	default "yama,loadpin,safesetid,integrity,apparmor" if DEFAULT_SECURITY_APPARMOR
+	default "yama,loadpin,safesetid,integrity"
 	help
 	  A comma-separated list of LSMs, in initialization order.
 	  Any LSMs left off this list will be ignored. This can be
-- 
1.8.3.1



^ permalink raw reply related

* Re: [PATCH] device_cgroup: fix RCU imbalance in error case
From: Michal Hocko @ 2019-03-19  8:33 UTC (permalink / raw)
  To: Jann Horn
  Cc: James Morris, Serge E. Hallyn, Tejun Heo, Li Zefan,
	Johannes Weiner, linux-security-module, linux-kernel,
	Aristeu Rozanski, Serge E . Hallyn, cgroups
In-Reply-To: <20190319013659.86199-1-jannh@google.com>

On Tue 19-03-19 02:36:59, Jann Horn wrote:
> When dev_exception_add() returns an error (due to a failed memory
> allocation), make sure that we move the RCU preemption count back to where
> it was before we were called. We dropped the RCU read lock inside the loop
> body, so we can't just "break".
> 
> sparse complains about this, too:
> 
> $ make -s C=2 security/device_cgroup.o
> ./include/linux/rcupdate.h:647:9: warning: context imbalance in
> 'propagate_exception' - unexpected unlock
> 
> Fixes: d591fb56618f ("device_cgroup: simplify cgroup tree walk in propagate_exception()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>

FWIW looks good to me.
Acked-by: Michal Hocko <mhocko@suse.com>

Thanks

> ---
> Compile-tested only.
> 
> I'm not entirely sure who's supposed to be the maintainer for this thing.
> The sign-offs on the commits to this file come from Tejun, but MAINTAINERS
> claims it's part of security/, so I'm just sending this to both the
> security folks and the cgroup folks, you can figure out whose tree you want
> to take this through. :P
> If the cgroup folks feel responsible for this file, maybe you could fix up
> MAINTAINERS?
> 
>  security/device_cgroup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/device_cgroup.c b/security/device_cgroup.c
> index cd97929fac66..dc28914fa72e 100644
> --- a/security/device_cgroup.c
> +++ b/security/device_cgroup.c
> @@ -560,7 +560,7 @@ static int propagate_exception(struct dev_cgroup *devcg_root,
>  		    devcg->behavior == DEVCG_DEFAULT_ALLOW) {
>  			rc = dev_exception_add(devcg, ex);
>  			if (rc)
> -				break;
> +				return rc;
>  		} else {
>  			/*
>  			 * in the other possible cases:
> -- 
> 2.21.0.225.g810b269d1ac-goog

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* [PATCH] device_cgroup: fix RCU imbalance in error case
From: Jann Horn @ 2019-03-19  1:36 UTC (permalink / raw)
  To: James Morris, Serge E. Hallyn, Tejun Heo, Li Zefan,
	Johannes Weiner, jannh
  Cc: linux-security-module, linux-kernel, Aristeu Rozanski,
	Serge E . Hallyn, Michal Hocko, cgroups

When dev_exception_add() returns an error (due to a failed memory
allocation), make sure that we move the RCU preemption count back to where
it was before we were called. We dropped the RCU read lock inside the loop
body, so we can't just "break".

sparse complains about this, too:

$ make -s C=2 security/device_cgroup.o
./include/linux/rcupdate.h:647:9: warning: context imbalance in
'propagate_exception' - unexpected unlock

Fixes: d591fb56618f ("device_cgroup: simplify cgroup tree walk in propagate_exception()")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
Compile-tested only.

I'm not entirely sure who's supposed to be the maintainer for this thing.
The sign-offs on the commits to this file come from Tejun, but MAINTAINERS
claims it's part of security/, so I'm just sending this to both the
security folks and the cgroup folks, you can figure out whose tree you want
to take this through. :P
If the cgroup folks feel responsible for this file, maybe you could fix up
MAINTAINERS?

 security/device_cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/device_cgroup.c b/security/device_cgroup.c
index cd97929fac66..dc28914fa72e 100644
--- a/security/device_cgroup.c
+++ b/security/device_cgroup.c
@@ -560,7 +560,7 @@ static int propagate_exception(struct dev_cgroup *devcg_root,
 		    devcg->behavior == DEVCG_DEFAULT_ALLOW) {
 			rc = dev_exception_add(devcg, ex);
 			if (rc)
-				break;
+				return rc;
 		} else {
 			/*
 			 * in the other possible cases:
-- 
2.21.0.225.g810b269d1ac-goog


^ permalink raw reply related

* Re: [PATCH v10, RESEND 5/6] KEYS: trusted: explicitly use tpm_chip structure from tpm_default_chip()
From: Dan Williams @ 2019-03-18 22:35 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: Jarkko Sakkinen, zohar, david.safford, monty.wiseman,
	matthewgarrett, linux-integrity, linux-security-module, keyrings,
	Linux Kernel Mailing List, silviu.vlasceanu, linux-nvdimm
In-Reply-To: <20190206162452.7749-6-roberto.sassu@huawei.com>

On Wed, Feb 6, 2019 at 10:30 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> When crypto agility support will be added to the TPM driver, users of the
> driver have to retrieve the allocated banks from chip->allocated_banks and
> use this information to prepare the array of tpm_digest structures to be
> passed to tpm_pcr_extend().
>
> This patch retrieves a tpm_chip pointer from tpm_default_chip() so that the
> pointer can be used to prepare the array of tpm_digest structures.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  security/keys/trusted.c | 38 ++++++++++++++++++++++++--------------
>  1 file changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index 4d98f4f87236..5b852263eae1 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -34,6 +34,7 @@
>
>  static const char hmac_alg[] = "hmac(sha1)";
>  static const char hash_alg[] = "sha1";
> +static struct tpm_chip *chip;
>
>  struct sdesc {
>         struct shash_desc shash;
> @@ -362,7 +363,7 @@ int trusted_tpm_send(unsigned char *cmd, size_t buflen)
>         int rc;
>
>         dump_tpm_buf(cmd);
> -       rc = tpm_send(NULL, cmd, buflen);
> +       rc = tpm_send(chip, cmd, buflen);
>         dump_tpm_buf(cmd);
>         if (rc > 0)
>                 /* Can't return positive return codes values to keyctl */
> @@ -384,10 +385,10 @@ static int pcrlock(const int pcrnum)
>
>         if (!capable(CAP_SYS_ADMIN))
>                 return -EPERM;
> -       ret = tpm_get_random(NULL, hash, SHA1_DIGEST_SIZE);
> +       ret = tpm_get_random(chip, hash, SHA1_DIGEST_SIZE);
>         if (ret != SHA1_DIGEST_SIZE)
>                 return ret;
> -       return tpm_pcr_extend(NULL, pcrnum, hash) ? -EINVAL : 0;
> +       return tpm_pcr_extend(chip, pcrnum, hash) ? -EINVAL : 0;
>  }
>
>  /*
> @@ -400,7 +401,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
>         unsigned char ononce[TPM_NONCE_SIZE];
>         int ret;
>
> -       ret = tpm_get_random(NULL, ononce, TPM_NONCE_SIZE);
> +       ret = tpm_get_random(chip, ononce, TPM_NONCE_SIZE);
>         if (ret != TPM_NONCE_SIZE)
>                 return ret;
>
> @@ -496,7 +497,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
>         if (ret < 0)
>                 goto out;
>
> -       ret = tpm_get_random(NULL, td->nonceodd, TPM_NONCE_SIZE);
> +       ret = tpm_get_random(chip, td->nonceodd, TPM_NONCE_SIZE);
>         if (ret != TPM_NONCE_SIZE)
>                 goto out;
>         ordinal = htonl(TPM_ORD_SEAL);
> @@ -606,7 +607,7 @@ static int tpm_unseal(struct tpm_buf *tb,
>
>         ordinal = htonl(TPM_ORD_UNSEAL);
>         keyhndl = htonl(SRKHANDLE);
> -       ret = tpm_get_random(NULL, nonceodd, TPM_NONCE_SIZE);
> +       ret = tpm_get_random(chip, nonceodd, TPM_NONCE_SIZE);
>         if (ret != TPM_NONCE_SIZE) {
>                 pr_info("trusted_key: tpm_get_random failed (%d)\n", ret);
>                 return ret;
> @@ -751,7 +752,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>         int i;
>         int tpm2;
>
> -       tpm2 = tpm_is_tpm2(NULL);
> +       tpm2 = tpm_is_tpm2(chip);
>         if (tpm2 < 0)
>                 return tpm2;
>
> @@ -920,7 +921,7 @@ static struct trusted_key_options *trusted_options_alloc(void)
>         struct trusted_key_options *options;
>         int tpm2;
>
> -       tpm2 = tpm_is_tpm2(NULL);
> +       tpm2 = tpm_is_tpm2(chip);
>         if (tpm2 < 0)
>                 return NULL;
>
> @@ -970,7 +971,7 @@ static int trusted_instantiate(struct key *key,
>         size_t key_len;
>         int tpm2;
>
> -       tpm2 = tpm_is_tpm2(NULL);
> +       tpm2 = tpm_is_tpm2(chip);
>         if (tpm2 < 0)
>                 return tpm2;
>
> @@ -1011,7 +1012,7 @@ static int trusted_instantiate(struct key *key,
>         switch (key_cmd) {
>         case Opt_load:
>                 if (tpm2)
> -                       ret = tpm_unseal_trusted(NULL, payload, options);
> +                       ret = tpm_unseal_trusted(chip, payload, options);
>                 else
>                         ret = key_unseal(payload, options);
>                 dump_payload(payload);
> @@ -1021,13 +1022,13 @@ static int trusted_instantiate(struct key *key,
>                 break;
>         case Opt_new:
>                 key_len = payload->key_len;
> -               ret = tpm_get_random(NULL, payload->key, key_len);
> +               ret = tpm_get_random(chip, payload->key, key_len);
>                 if (ret != key_len) {
>                         pr_info("trusted_key: key_create failed (%d)\n", ret);
>                         goto out;
>                 }
>                 if (tpm2)
> -                       ret = tpm_seal_trusted(NULL, payload, options);
> +                       ret = tpm_seal_trusted(chip, payload, options);
>                 else
>                         ret = key_seal(payload, options);
>                 if (ret < 0)
> @@ -1225,17 +1226,26 @@ static int __init init_trusted(void)
>  {
>         int ret;
>
> +       chip = tpm_default_chip();
> +       if (!chip)
> +               return -ENOENT;

This change causes a regression loading the encrypted_keys module on
systems that don't have a tpm.

Module init functions should not have hardware dependencies.

The effect is that the libnvdimm module, which is an encrypted_keys
user, fails to load, but up until this change encrypted_keys did not
have a hard dependency on TPM presence.

^ permalink raw reply

* Re: [PATCH 09/27] hibernate: Disable when the kernel is locked down
From: Alan Cox @ 2019-03-18 18:55 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: jmorris, LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <CACdnJuutrCYOoppScar53nfNnsrE6Oq36ayRGnJRLgh202aasw@mail.gmail.com>

> Suse have a solution for this that I'd like to see pushed again, but
> from a practical perspective enterprise distributions have been
> shipping this for some time without significant obvious customer
> complaint.

Probably because their IT department hasn't noticed 8)

Alan

^ permalink raw reply

* Re: [PATCH RESEND] KEYS: remove CONFIG_KEYS_COMPAT
From: Eric Biggers @ 2019-03-18 17:28 UTC (permalink / raw)
  To: keyrings, David Howells; +Cc: linux-security-module
In-Reply-To: <20190207233548.GC125156@gmail.com>

On Thu, Feb 07, 2019 at 03:35:49PM -0800, Eric Biggers wrote:
> On Wed, Nov 14, 2018 at 04:52:38PM -0800, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > KEYS_COMPAT now always takes the value of COMPAT && KEYS.  But the
> > security/keys/ directory is only compiled if KEYS is enabled, so in
> > practice KEYS_COMPAT is the same as COMPAT.  Therefore, remove the
> > unnecessary KEYS_COMPAT and just use COMPAT directly.
> > 
> > (Also remove an outdated comment from compat.c.)
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >  security/keys/Kconfig    | 4 ----
> >  security/keys/Makefile   | 2 +-
> >  security/keys/compat.c   | 5 -----
> >  security/keys/internal.h | 4 ++--
> >  4 files changed, 3 insertions(+), 12 deletions(-)
> > 
> > diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> > index 6462e6654ccf4..e115d691d9776 100644
> > --- a/security/keys/Kconfig
> > +++ b/security/keys/Kconfig
> > @@ -20,10 +20,6 @@ config KEYS
> >  
> >  	  If you are unsure as to whether this is required, answer N.
> >  
> > -config KEYS_COMPAT
> > -	def_bool y
> > -	depends on COMPAT && KEYS
> > -
> >  config PERSISTENT_KEYRINGS
> >  	bool "Enable register of persistent per-UID keyrings"
> >  	depends on KEYS
> > diff --git a/security/keys/Makefile b/security/keys/Makefile
> > index 9cef54064f608..c694458d9a46c 100644
> > --- a/security/keys/Makefile
> > +++ b/security/keys/Makefile
> > @@ -17,7 +17,7 @@ obj-y := \
> >  	request_key_auth.o \
> >  	user_defined.o
> >  compat-obj-$(CONFIG_KEY_DH_OPERATIONS) += compat_dh.o
> > -obj-$(CONFIG_KEYS_COMPAT) += compat.o $(compat-obj-y)
> > +obj-$(CONFIG_COMPAT) += compat.o $(compat-obj-y)
> >  obj-$(CONFIG_PROC_FS) += proc.o
> >  obj-$(CONFIG_SYSCTL) += sysctl.o
> >  obj-$(CONFIG_PERSISTENT_KEYRINGS) += persistent.o
> > diff --git a/security/keys/compat.c b/security/keys/compat.c
> > index 9482df601dc33..f22527e88e3d5 100644
> > --- a/security/keys/compat.c
> > +++ b/security/keys/compat.c
> > @@ -50,11 +50,6 @@ static long compat_keyctl_instantiate_key_iov(
> >  
> >  /*
> >   * The key control system call, 32-bit compatibility version for 64-bit archs
> > - *
> > - * This should only be called if the 64-bit arch uses weird pointers in 32-bit
> > - * mode or doesn't guarantee that the top 32-bits of the argument registers on
> > - * taking a 32-bit syscall are zero.  If you can, you should call sys_keyctl()
> > - * directly.
> >   */
> >  COMPAT_SYSCALL_DEFINE5(keyctl, u32, option,
> >  		       u32, arg2, u32, arg3, u32, arg4, u32, arg5)
> > diff --git a/security/keys/internal.h b/security/keys/internal.h
> > index 74cb0ff42fedb..d1836e5d670cb 100644
> > --- a/security/keys/internal.h
> > +++ b/security/keys/internal.h
> > @@ -272,7 +272,7 @@ extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *,
> >  			      size_t, struct keyctl_kdf_params __user *);
> >  extern long __keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *,
> >  				size_t, struct keyctl_kdf_params *);
> > -#ifdef CONFIG_KEYS_COMPAT
> > +#ifdef CONFIG_COMPAT
> >  extern long compat_keyctl_dh_compute(struct keyctl_dh_params __user *params,
> >  				char __user *buffer, size_t buflen,
> >  				struct compat_keyctl_kdf_params __user *kdf);
> > @@ -287,7 +287,7 @@ static inline long keyctl_dh_compute(struct keyctl_dh_params __user *params,
> >  	return -EOPNOTSUPP;
> >  }
> >  
> > -#ifdef CONFIG_KEYS_COMPAT
> > +#ifdef CONFIG_COMPAT
> >  static inline long compat_keyctl_dh_compute(
> >  				struct keyctl_dh_params __user *params,
> >  				char __user *buffer, size_t buflen,
> > -- 
> > 2.19.1.930.g4563a0d9d0-goog
> > 
> 
> Ping.  David, are you planning to apply this?
> 
> - Eric

Ping.

^ permalink raw reply

* [PATCH v19 17/27] x86/sgx: Add provisioning
From: Jarkko Sakkinen @ 2019-03-17 21:14 UTC (permalink / raw)
  To: x86, linux-sgx
  Cc: akpm, dave.hansen, sean.j.christopherson, nhorman, npmccallum,
	serge.ayoun, shay.katz-zamir, haitao.huang, andriy.shevchenko,
	tglx, kai.svahn, bp, josh, luto, kai.huang, rientjes,
	Jarkko Sakkinen, James Morris, Serge E . Hallyn,
	linux-security-module
In-Reply-To: <20190317211456.13927-1-jarkko.sakkinen@linux.intel.com>

In order to provide a mechanism for devilering provisoning rights:

1. Add a new file to the securityfs file called sgx/provision that works
   as a token for allowing an enclave to have the provisioning privileges.
2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the
   following data structure:

   struct sgx_enclave_set_attribute {
           __u64 addr;
           __u64 token_fd;
   };

A daemon could sit on top of sgx/provision and send a file descriptor of
this file to a process that needs to be able to provision enclaves.

The way this API is used is more or less straight-forward. Lets assume that
dev_fd is a handle to /dev/sgx and prov_fd is a handle to sgx/provision.
You would allow SGX_IOC_ENCLAVE_CREATE to initialize an enclave with the
PROVISIONKEY attribute by

params.addr = <enclave address>;
params.token_fd = prov_fd;

ioctl(dev_fd, SGX_IOC_ENCLAVE_SET_ATTRIBUTE, &params);

Cc: James Morris <jmorris@namei.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: linux-security-module@vger.kernel.org
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 arch/x86/include/uapi/asm/sgx.h        | 13 +++++++
 arch/x86/kernel/cpu/sgx/driver/ioctl.c | 43 +++++++++++++++++++++++
 arch/x86/kernel/cpu/sgx/driver/main.c  | 47 ++++++++++++++++++++++++++
 3 files changed, 103 insertions(+)

diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
index aadf9c76e360..150a784db395 100644
--- a/arch/x86/include/uapi/asm/sgx.h
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -16,6 +16,8 @@
 	_IOW(SGX_MAGIC, 0x01, struct sgx_enclave_add_page)
 #define SGX_IOC_ENCLAVE_INIT \
 	_IOW(SGX_MAGIC, 0x02, struct sgx_enclave_init)
+#define SGX_IOC_ENCLAVE_SET_ATTRIBUTE \
+	_IOW(SGX_MAGIC, 0x03, struct sgx_enclave_set_attribute)
 
 /* IOCTL return values */
 #define SGX_POWER_LOST_ENCLAVE		0x40000000
@@ -56,4 +58,15 @@ struct sgx_enclave_init {
 	__u64	sigstruct;
 };
 
+/**
+ * struct sgx_enclave_set_attribute - parameter structure for the
+ *				      %SGX_IOC_ENCLAVE_INIT ioctl
+ * @addr:		address within the ELRANGE
+ * @attribute_fd:	file handle of the attribute file in the securityfs
+ */
+struct sgx_enclave_set_attribute {
+	__u64	addr;
+	__u64	attribute_fd;
+};
+
 #endif /* _UAPI_ASM_X86_SGX_H */
diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
index 4b9a91b53b50..5d85bd3f7876 100644
--- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
@@ -759,6 +759,46 @@ static long sgx_ioc_enclave_init(struct file *filep, unsigned int cmd,
 	return ret;
 }
 
+/**
+ * sgx_ioc_enclave_set_attribute - handler for %SGX_IOC_ENCLAVE_SET_ATTRIBUTE
+ * @filep:	open file to /dev/sgx
+ * @cmd:	the command value
+ * @arg:	pointer to a struct sgx_enclave_set_attribute instance
+ *
+ * Sets an attribute matching the attribute file that is pointed by the
+ * parameter structure field attribute_fd.
+ *
+ * Return: 0 on success, -errno otherwise
+ */
+static long sgx_ioc_enclave_set_attribute(struct file *filep, unsigned int cmd,
+					  unsigned long arg)
+{
+	struct sgx_enclave_set_attribute *params = (void *)arg;
+	struct file *attribute_file;
+	struct sgx_encl *encl;
+	int ret;
+
+	attribute_file = fget(params->attribute_fd);
+	if (!attribute_file->f_op)
+		return -EINVAL;
+
+	if (attribute_file->f_op != &sgx_fs_provision_fops) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = sgx_encl_get(params->addr, &encl);
+	if (ret)
+		goto out;
+
+	encl->allowed_attributes |= SGX_ATTR_PROVISIONKEY;
+	kref_put(&encl->refcount, sgx_encl_release);
+
+out:
+	fput(attribute_file);
+	return ret;
+}
+
 typedef long (*sgx_ioc_t)(struct file *filep, unsigned int cmd,
 			  unsigned long arg);
 
@@ -778,6 +818,9 @@ long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 	case SGX_IOC_ENCLAVE_INIT:
 		handler = sgx_ioc_enclave_init;
 		break;
+	case SGX_IOC_ENCLAVE_SET_ATTRIBUTE:
+		handler = sgx_ioc_enclave_set_attribute;
+		break;
 	default:
 		return -ENOIOCTLCMD;
 	}
diff --git a/arch/x86/kernel/cpu/sgx/driver/main.c b/arch/x86/kernel/cpu/sgx/driver/main.c
index 16f36cd0af04..9a5360dcad98 100644
--- a/arch/x86/kernel/cpu/sgx/driver/main.c
+++ b/arch/x86/kernel/cpu/sgx/driver/main.c
@@ -22,6 +22,11 @@ u64 sgx_attributes_reserved_mask;
 u64 sgx_xfrm_reserved_mask = ~0x3;
 u32 sgx_xsave_size_tbl[64];
 
+const struct file_operations sgx_fs_provision_fops;
+
+static struct dentry *sgx_fs;
+static struct dentry *sgx_fs_provision;
+
 #ifdef CONFIG_COMPAT
 static long sgx_compat_ioctl(struct file *filep, unsigned int cmd,
 			      unsigned long arg)
@@ -147,6 +152,40 @@ static struct sgx_dev_ctx *sgxm_dev_ctx_alloc(struct device *parent)
 	return ctx;
 }
 
+static int sgx_fs_init(struct device *dev)
+{
+	int ret;
+
+	sgx_fs = securityfs_create_dir(dev_name(dev), NULL);
+	if (IS_ERR(sgx_fs)) {
+		ret = PTR_ERR(sgx_fs);
+		goto err_sgx_fs;
+	}
+
+	sgx_fs_provision = securityfs_create_file("provision", 0600, sgx_fs,
+						  NULL, &sgx_fs_provision_fops);
+	if (IS_ERR(sgx_fs)) {
+		ret = PTR_ERR(sgx_fs_provision);
+		goto err_sgx_fs_provision;
+	}
+
+	return 0;
+
+err_sgx_fs_provision:
+	securityfs_remove(sgx_fs);
+	sgx_fs_provision = NULL;
+
+err_sgx_fs:
+	sgx_fs = NULL;
+	return ret;
+}
+
+static void sgx_fs_remove(void)
+{
+	securityfs_remove(sgx_fs_provision);
+	securityfs_remove(sgx_fs);
+}
+
 static int sgx_dev_init(struct device *parent)
 {
 	struct sgx_dev_ctx *sgx_dev;
@@ -190,6 +229,10 @@ static int sgx_dev_init(struct device *parent)
 	if (!sgx_encl_wq)
 		return -ENOMEM;
 
+	ret = sgx_fs_init(&sgx_dev->ctrl_dev);
+	if (ret)
+		goto err_fs_init;
+
 	ret = cdev_device_add(&sgx_dev->ctrl_cdev, &sgx_dev->ctrl_dev);
 	if (ret)
 		goto err_device_add;
@@ -197,6 +240,9 @@ static int sgx_dev_init(struct device *parent)
 	return 0;
 
 err_device_add:
+	sgx_fs_remove();
+
+err_fs_init:
 	destroy_workqueue(sgx_encl_wq);
 	return ret;
 }
@@ -220,6 +266,7 @@ static int sgx_drv_remove(struct platform_device *pdev)
 {
 	struct sgx_dev_ctx *ctx = dev_get_drvdata(&pdev->dev);
 
+	sgx_fs_remove();
 	cdev_device_del(&ctx->ctrl_cdev, &ctx->ctrl_dev);
 	destroy_workqueue(sgx_encl_wq);
 
-- 
2.19.1


^ permalink raw reply related

* Re: mount.nfs: Protocol error after upgrade to linux/master
From: Casey Schaufler @ 2019-03-17  1:02 UTC (permalink / raw)
  To: Tetsuo Handa, Kees Cook
  Cc: Jakub Kicinski, linux-security-module, Trond Myklebust,
	open list:NFS, SUNRPC, AND..., Anna Schumaker, LKML
In-Reply-To: <f23d0fad-dc72-0e53-cac6-31abfd12a050@I-love.SAKURA.ne.jp>

On 3/16/2019 1:08 AM, Tetsuo Handa wrote:
> On 2019/03/16 14:38, Kees Cook wrote:
>>   config LSM
>>          string "Ordered list of enabled LSMs"
>> +       default "yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor" if DEFAULT_SECURITY_SMACK
>> +       default "yama,loadpin,safesetid,integrity,tomoyo,selinux,smack,apparmor" if DEFAULT_SECURITY_TOMOYO
>> +       default "yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo" if DEFAULT_SECURITY_APPARMOR
>>          default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
>>          help
>>            A comma-separated list of LSMs, in initialization order.
>>
>> (I don't see a way to include an earlier config string in a new
>> default.) Thoughts?
>>
> Hmm, DEFAULT_SECURITY_TOMOYO no longer works because TOMOYO will be
> always enabled as long as CONFIG_SECURITY_TOMOYO=y. Maybe
>
>   config LSM
>          string "Ordered list of enabled LSMs"
> -       default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> +       default "yama,loadpin,safesetid,integrity,selinux" if DEFAULT_SECURITY_SELINUX
> +       default "yama,loadpin,safesetid,integrity,smack" if DEFAULT_SECURITY_SMACK
> +       default "yama,loadpin,safesetid,integrity,tomoyo" if DEFAULT_SECURITY_TOMOYO
> +       default "yama,loadpin,safesetid,integrity,apparmor" if DEFAULT_SECURITY_APPARMOR
> +       default "yama,loadpin,safesetid,integrity" if DEFAULT_SECURITY_DAC
>          help
>            A comma-separated list of LSMs, in initialization order.
>
> (i.e. include only up to one major LSM as default choice, and allow manually including
> multiple major LSMs at both kernel build time and kernel boot time) is better?

I think this looks pretty good.


^ permalink raw reply

* [PATCH ghak109 V1] audit: link integrity evm_write_xattrs record to syscall event
From: Richard Guy Briggs @ 2019-03-16 12:10 UTC (permalink / raw)
  To: linux-integrity, linux-security-module, Linux-Audit Mailing List,
	LKML
  Cc: Paul Moore, sgrubb, omosnace, eparis, serge, zohar, mjg59,
	Richard Guy Briggs

In commit fa516b66a1bf ("EVM: Allow runtime modification of the set of
verified xattrs"), the call to audit_log_start() is missing a context to
link it to an audit event. Since this event is in user context, add
the process' syscall context to the record.

In addition, the orphaned keyword "locked" appears in the record.
Normalize this by changing it to "xattr=(locked)".

Please see the github issue
https://github.com/linux-audit/audit-kernel/issues/109

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 security/integrity/evm/evm_secfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index 015aea8fdf1e..4171d174e9da 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -192,7 +192,8 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
 	if (count > XATTR_NAME_MAX)
 		return -E2BIG;
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_INTEGRITY_EVM_XATTR);
+	ab = audit_log_start(audit_context(), GFP_KERNEL,
+			     AUDIT_INTEGRITY_EVM_XATTR);
 	if (!ab)
 		return -ENOMEM;
 
@@ -222,7 +223,7 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
 		inode_lock(inode);
 		err = simple_setattr(evm_xattrs, &newattrs);
 		inode_unlock(inode);
-		audit_log_format(ab, "locked");
+		audit_log_format(ab, "xattr=(locked)");
 		if (!err)
 			err = count;
 		goto out;
-- 
1.8.3.1


^ permalink raw reply related

* Re: mount.nfs: Protocol error after upgrade to linux/master
From: Tetsuo Handa @ 2019-03-16  8:08 UTC (permalink / raw)
  To: Kees Cook
  Cc: Jakub Kicinski, linux-security-module, Trond Myklebust,
	open list:NFS, SUNRPC, AND..., Anna Schumaker, LKML
In-Reply-To: <CAGXu5jL-e7bDEXfpUuT-HyFarR-8T0djXChR0cuzTwFQPQ0tOw@mail.gmail.com>

On 2019/03/16 14:38, Kees Cook wrote:
>  config LSM
>         string "Ordered list of enabled LSMs"
> +       default "yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor" if DEFAULT_SECURITY_SMACK
> +       default "yama,loadpin,safesetid,integrity,tomoyo,selinux,smack,apparmor" if DEFAULT_SECURITY_TOMOYO
> +       default "yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo" if DEFAULT_SECURITY_APPARMOR
>         default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
>         help
>           A comma-separated list of LSMs, in initialization order.
> 
> (I don't see a way to include an earlier config string in a new
> default.) Thoughts?
>

Hmm, DEFAULT_SECURITY_TOMOYO no longer works because TOMOYO will be
always enabled as long as CONFIG_SECURITY_TOMOYO=y. Maybe

 config LSM
        string "Ordered list of enabled LSMs"
-       default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
+       default "yama,loadpin,safesetid,integrity,selinux" if DEFAULT_SECURITY_SELINUX
+       default "yama,loadpin,safesetid,integrity,smack" if DEFAULT_SECURITY_SMACK
+       default "yama,loadpin,safesetid,integrity,tomoyo" if DEFAULT_SECURITY_TOMOYO
+       default "yama,loadpin,safesetid,integrity,apparmor" if DEFAULT_SECURITY_APPARMOR
+       default "yama,loadpin,safesetid,integrity" if DEFAULT_SECURITY_DAC
        help
          A comma-separated list of LSMs, in initialization order.

(i.e. include only up to one major LSM as default choice, and allow manually including
multiple major LSMs at both kernel build time and kernel boot time) is better?


^ permalink raw reply

* Re: mount.nfs: Protocol error after upgrade to linux/master
From: Kees Cook @ 2019-03-16  5:38 UTC (permalink / raw)
  To: Jakub Kicinski, linux-security-module
  Cc: Trond Myklebust, open list:NFS, SUNRPC, AND..., Anna Schumaker,
	LKML
In-Reply-To: <CAGXu5jKHRPuaALQcPXvpDeuVgZR+EeRqo9Qj4j3kYatb3HUSSA@mail.gmail.com>

On Fri, Mar 15, 2019 at 10:24 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Mar 15, 2019 at 4:54 PM Jakub Kicinski
> <jakub.kicinski@netronome.com> wrote:
> >
> > On Fri, 15 Mar 2019 12:01:05 -0700, Jakub Kicinski wrote:
> > > On Fri, 15 Mar 2019 11:05:55 -0700, Jakub Kicinski wrote:
> > > > Hi,
> > > >
> > > > I just upgraded from:
> > > >
> > > > commit a3b1933d34d5bb26d7503752e3528315a9e28339 (net)
> > > > Merge: c6873d18cb4a 24319258660a
> > > > Author: David S. Miller <davem@davemloft.net>
> > > > Date:   Mon Mar 11 16:22:49 2019 -0700
> > > >
> > > > to
> > > >
> > > > commit 3b319ee220a8795406852a897299dbdfc1b09911
> > > > Merge: 9352ca585b2a b6e88119f1ed
> > > > Author: Linus Torvalds <torvalds@linux-foundation.org>
> > > > Date:   Thu Mar 14 10:48:14 2019 -0700
> > > >
> > > > and I'm seeing:
> > > >
> > > > # mount /home/
> > > > mount.nfs: Protocol error
> > > >
> > > > No errors in dmesg, please let me know if it's a known problem or what
> > > > other info could be of use.
> > >
> > > Hm.. I tried to bisect but reverting to that commit doesn't help.
> > >
> > > Looks like the server responds with:
> > >
> > >   ICMP parameter problem - octet 22, length 80
> > >
> > > pointing at some IP options (type 134)...
> >
> > Okay, figured it out, it's the commit 13e735c0e953 ("LSM: Introduce
> > CONFIG_LSM") and all the related changes in security/
> >
> > I did olddefconfig and it changed my security module from apparmor to
> > smack silently.  smack must be slapping those IP options on by default.
> >
> > Pretty awful user experience, and a non-zero chance that users who
> > upgrade their kernels will miss this and end up with the wrong security
> > module...
>
> I wonder if we can add some kind of logic to Kconfig to retain the old
> CONFIG_DEFAULT_SECURITY and include it as the first legacy-major LSM
> listed in CONFIG_LSM?
>
> Like, but the old selector back in, but mark is as "soon to be
> entirely replaced with CONFIG_LSM" and then make CONFIG_LSM's default
> be "yama,loadpin,safesetid,integrity,$(CONFIG_DEFAULT_SECURITY),selinux,smack,tomoyo,apparmor"
> ? Duplicates are ignored...

This would initialize a default order from the earlier Kconfig items:

diff --git a/security/Kconfig b/security/Kconfig
index 1d6463fb1450..e3813b5c6824 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -239,8 +239,40 @@ source "security/safesetid/Kconfig"

 source "security/integrity/Kconfig"

+choice
+       prompt "First legacy-major LSM to be initialized"
+       default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
+       default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
+       default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
+       default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
+       default DEFAULT_SECURITY_DAC
+
+       help
+         Select the legacy-major security module that will be initialize
+         first. Overridden by non-default CONFIG_LSM.
+
+       config DEFAULT_SECURITY_SELINUX
+               bool "SELinux" if SECURITY_SELINUX=y
+
+       config DEFAULT_SECURITY_SMACK
+               bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y
+
+       config DEFAULT_SECURITY_TOMOYO
+               bool "TOMOYO" if SECURITY_TOMOYO=y
+
+       config DEFAULT_SECURITY_APPARMOR
+               bool "AppArmor" if SECURITY_APPARMOR=y
+
+       config DEFAULT_SECURITY_DAC
+               bool "Unix Discretionary Access Controls"
+
+endchoice
+
 config LSM
        string "Ordered list of enabled LSMs"
+       default
"yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor" if
DEFAULT_SECURITY_SMACK
+       default
"yama,loadpin,safesetid,integrity,tomoyo,selinux,smack,apparmor" if
DEFAULT_SECURITY_TOMOYO
+       default
"yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo" if
DEFAULT_SECURITY_APPARMOR
        default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
        help
          A comma-separated list of LSMs, in initialization order.

(I don't see a way to include an earlier config string in a new
default.) Thoughts?

-- 
Kees Cook

^ permalink raw reply related

* Re: mount.nfs: Protocol error after upgrade to linux/master
From: Kees Cook @ 2019-03-16  5:24 UTC (permalink / raw)
  To: Jakub Kicinski, linux-security-module
  Cc: Trond Myklebust, open list:NFS, SUNRPC, AND..., Anna Schumaker,
	LKML
In-Reply-To: <20190315165440.53b9db3c@cakuba.netronome.com>

On Fri, Mar 15, 2019 at 4:54 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Fri, 15 Mar 2019 12:01:05 -0700, Jakub Kicinski wrote:
> > On Fri, 15 Mar 2019 11:05:55 -0700, Jakub Kicinski wrote:
> > > Hi,
> > >
> > > I just upgraded from:
> > >
> > > commit a3b1933d34d5bb26d7503752e3528315a9e28339 (net)
> > > Merge: c6873d18cb4a 24319258660a
> > > Author: David S. Miller <davem@davemloft.net>
> > > Date:   Mon Mar 11 16:22:49 2019 -0700
> > >
> > > to
> > >
> > > commit 3b319ee220a8795406852a897299dbdfc1b09911
> > > Merge: 9352ca585b2a b6e88119f1ed
> > > Author: Linus Torvalds <torvalds@linux-foundation.org>
> > > Date:   Thu Mar 14 10:48:14 2019 -0700
> > >
> > > and I'm seeing:
> > >
> > > # mount /home/
> > > mount.nfs: Protocol error
> > >
> > > No errors in dmesg, please let me know if it's a known problem or what
> > > other info could be of use.
> >
> > Hm.. I tried to bisect but reverting to that commit doesn't help.
> >
> > Looks like the server responds with:
> >
> >   ICMP parameter problem - octet 22, length 80
> >
> > pointing at some IP options (type 134)...
>
> Okay, figured it out, it's the commit 13e735c0e953 ("LSM: Introduce
> CONFIG_LSM") and all the related changes in security/
>
> I did olddefconfig and it changed my security module from apparmor to
> smack silently.  smack must be slapping those IP options on by default.
>
> Pretty awful user experience, and a non-zero chance that users who
> upgrade their kernels will miss this and end up with the wrong security
> module...

I wonder if we can add some kind of logic to Kconfig to retain the old
CONFIG_DEFAULT_SECURITY and include it as the first legacy-major LSM
listed in CONFIG_LSM?

Like, but the old selector back in, but mark is as "soon to be
entirely replaced with CONFIG_LSM" and then make CONFIG_LSM's default
be "yama,loadpin,safesetid,integrity,$(CONFIG_DEFAULT_SECURITY),selinux,smack,tomoyo,apparmor"
? Duplicates are ignored...

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH] security: inode: fix a missing check for securityfs_create_file
From: Tetsuo Handa @ 2019-03-15 22:34 UTC (permalink / raw)
  To: Kangjie Lu
  Cc: pakki001, James Morris, Serge E. Hallyn, linux-security-module,
	linux-kernel
In-Reply-To: <20190315210025.17832-1-kjlu@umn.edu>

On 2019/03/16 6:00, Kangjie Lu wrote:
> securityfs_create_file  may fail. The fix checks its status and
> returns the error code upstream if it fails.

Failure in __init functions of vmlinux means that the system failed
before the global /sbin/init process starts. There is little value
with continuing the boot process. Calling panic() or BUG_ON() will
be OK, for the userspace will be get confused by lack of that file
even if we continued without securityfs entry in /proc/filesystems .

> 
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> 
> ---
> Return the exact error code upstream.
> ---
>  security/inode.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/security/inode.c b/security/inode.c
> index b7772a9b315e..667f8b15027d 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -339,6 +339,11 @@ static int __init securityfs_init(void)
>  #ifdef CONFIG_SECURITY
>  	lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL,
>  						&lsm_ops);
> +	if (IS_ERR(lsm_dentry)) {
> +		unregister_filesystem(&fs_type);
> +		sysfs_remove_mount_point(kernel_kobj, "security");
> +		return PTR_ERR(lsm_dentry);
> +	}
>  #endif
>  	return 0;
>  }
> 


^ permalink raw reply

* [PATCH] security: inode: fix a missing check for securityfs_create_file
From: Kangjie Lu @ 2019-03-15 21:00 UTC (permalink / raw)
  To: kjlu
  Cc: pakki001, James Morris, Serge E. Hallyn, linux-security-module,
	linux-kernel
In-Reply-To: <alpine.LRH.2.21.1903160608080.2905@namei.org>

securityfs_create_file  may fail. The fix checks its status and
returns the error code upstream if it fails.

Signed-off-by: Kangjie Lu <kjlu@umn.edu>

---
Return the exact error code upstream.
---
 security/inode.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/security/inode.c b/security/inode.c
index b7772a9b315e..667f8b15027d 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -339,6 +339,11 @@ static int __init securityfs_init(void)
 #ifdef CONFIG_SECURITY
 	lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL,
 						&lsm_ops);
+	if (IS_ERR(lsm_dentry)) {
+		unregister_filesystem(&fs_type);
+		sysfs_remove_mount_point(kernel_kobj, "security");
+		return PTR_ERR(lsm_dentry);
+	}
 #endif
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH] security: inode: fix a missing check for securityfs_create_file
From: James Morris @ 2019-03-15 19:08 UTC (permalink / raw)
  To: Kangjie Lu
  Cc: pakki001, Serge E. Hallyn, linux-security-module, linux-kernel,
	Casey Schaufler
In-Reply-To: <20190315040901.31551-1-kjlu@umn.edu>

On Thu, 14 Mar 2019, Kangjie Lu wrote:

> securityfs_create_file  may fail. The fix checks its status and
> returns EFAULT upstream if it fails.
> 
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> ---
>  security/inode.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/security/inode.c b/security/inode.c
> index b7772a9b315e..11d9a6bc2161 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -339,6 +339,11 @@ static int __init securityfs_init(void)
>  #ifdef CONFIG_SECURITY
>  	lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL,
>  						&lsm_ops);
> +	if (IS_ERR(lsm_dentry)) {
> +		unregister_filesystem(&fs_type);
> +		sysfs_remove_mount_point(kernel_kobj, "security");
> +		return -EFAULT;
> +	}
>  #endif
>  	return 0;
>  }
> 

Good catch, but you should propagate the error returned from 
securityfs_create_file().


-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply

* Re: Add support for TCG2 log format on UEFI systems
From: Jarkko Sakkinen @ 2019-03-15 11:47 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Peter Huewe, Jason Gunthorpe, Roberto Sassu,
	linux-efi, LSM List, Linux Kernel Mailing List,
	Thiébaud Weksteen
In-Reply-To: <CACdnJuuWMg6c1GJHR3EMobaZ1CwquNudjcgxwXNWczE=3DbQTg@mail.gmail.com>

On Thu, Mar 14, 2019 at 02:04:02PM -0700, Matthew Garrett wrote:
> On Thu, Mar 14, 2019 at 2:35 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> > > Identical to V4, but based on tpmdd-next
> >
> > This is not found /sys/kernel/security/tpm0/ascii_bios_measurements
> 
> That's expected - the existing kernel TCG2 log code doesn't expose
> ascii_bios_measurements, only binary_bios_measurements. This patchset
> doesn't change that.

Oops, I meant to point out that the binary_bios_measurents is not found
(i.e. ascii was a typo). The whole tpm0 directory is missing.

I'll try to pinpoint next week where things might go wrong on my system.

/Jarkko

^ permalink raw reply

* [PATCH] security: inode: fix a missing check for securityfs_create_file
From: Kangjie Lu @ 2019-03-15  4:09 UTC (permalink / raw)
  To: kjlu
  Cc: pakki001, James Morris, Serge E. Hallyn, linux-security-module,
	linux-kernel

securityfs_create_file  may fail. The fix checks its status and
returns EFAULT upstream if it fails.

Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
 security/inode.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/security/inode.c b/security/inode.c
index b7772a9b315e..11d9a6bc2161 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -339,6 +339,11 @@ static int __init securityfs_init(void)
 #ifdef CONFIG_SECURITY
 	lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL,
 						&lsm_ops);
+	if (IS_ERR(lsm_dentry)) {
+		unregister_filesystem(&fs_type);
+		sysfs_remove_mount_point(kernel_kobj, "security");
+		return -EFAULT;
+	}
 #endif
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related

* Re: Add support for TCG2 log format on UEFI systems
From: Matthew Garrett @ 2019-03-14 21:04 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-integrity, Peter Huewe, Jason Gunthorpe, Roberto Sassu,
	linux-efi, LSM List, Linux Kernel Mailing List,
	Thiébaud Weksteen
In-Reply-To: <20190314093515.GA11165@linux.intel.com>

On Thu, Mar 14, 2019 at 2:35 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Wed, Feb 27, 2019 at 12:26:54PM -0800, Matthew Garrett wrote:
> > Identical to V4, but based on tpmdd-next
>
> This is not found /sys/kernel/security/tpm0/ascii_bios_measurements

That's expected - the existing kernel TCG2 log code doesn't expose
ascii_bios_measurements, only binary_bios_measurements. This patchset
doesn't change that.

^ permalink raw reply

* Re: [PATCH 1/1] Smack: Create smack_rule cache to optimize memory usage
From: Casey Schaufler @ 2019-03-14 17:07 UTC (permalink / raw)
  To: Vishal Goel, linux-security-module, linux-kernel
  Cc: pankaj.m, a.sahrawat, casey
In-Reply-To: <1552554393-11691-1-git-send-email-vishal.goel@samsung.com>

On 3/14/2019 2:06 AM, Vishal Goel wrote:
> This patch allows for small memory optimization by creating the
> kmem cache for "struct smack_rule" instead of using kzalloc.
> For adding new smack rule, kzalloc is used to allocate the memory
> for "struct smack_rule". kzalloc will always allocate 32 or 64 bytes
> for 1 structure depending upon the kzalloc cache sizes available in
> system. Although the size of structure is 20 bytes only, resulting
> in memory wastage per object in the default pool.
>
> For e.g., if there are 20000 rules, then it will save 240KB(20000*12)
> which is crucial for small memory targets.
>
> Signed-off-by: Vishal Goel <vishal.goel@samsung.com>
> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>

I will take this for 5.2.
Thank you. Keep up the good work.

> ---
>   security/smack/smack.h     |  1 +
>   security/smack/smack_lsm.c | 12 ++++++++++--
>   security/smack/smackfs.c   |  2 +-
>   3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 6a71fc7..a5d7461 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -354,6 +354,7 @@ int smk_tskacc(struct task_smack *, struct smack_known *,
>   
>   #define SMACK_HASH_SLOTS 16
>   extern struct hlist_head smack_known_hash[SMACK_HASH_SLOTS];
> +extern struct kmem_cache *smack_rule_cache;
>   
>   /*
>    * Is the directory transmuting?
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 319add3..16b6cf5 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -56,6 +56,7 @@
>   static LIST_HEAD(smk_ipv6_port_list);
>   #endif
>   static struct kmem_cache *smack_inode_cache;
> +struct kmem_cache *smack_rule_cache;
>   int smack_enabled;
>   
>   static const match_table_t smk_mount_tokens = {
> @@ -349,7 +350,7 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead,
>   	int rc = 0;
>   
>   	list_for_each_entry_rcu(orp, ohead, list) {
> -		nrp = kzalloc(sizeof(struct smack_rule), gfp);
> +		nrp = kmem_cache_zalloc(smack_rule_cache, gfp);
>   		if (nrp == NULL) {
>   			rc = -ENOMEM;
>   			break;
> @@ -1995,7 +1996,7 @@ static void smack_cred_free(struct cred *cred)
>   	list_for_each_safe(l, n, &tsp->smk_rules) {
>   		rp = list_entry(l, struct smack_rule, list);
>   		list_del(&rp->list);
> -		kfree(rp);
> +		kmem_cache_free(smack_rule_cache, rp);
>   	}
>   	kfree(tsp);
>   }
> @@ -4788,10 +4789,17 @@ static __init int smack_init(void)
>   	if (!smack_inode_cache)
>   		return -ENOMEM;
>   
> +	smack_rule_cache = KMEM_CACHE(smack_rule, 0);
> +	if (!smack_rule_cache) {
> +		kmem_cache_destroy(smack_inode_cache);
> +		return -ENOMEM;
> +	}
> +
>   	tsp = new_task_smack(&smack_known_floor, &smack_known_floor,
>   				GFP_KERNEL);
>   	if (tsp == NULL) {
>   		kmem_cache_destroy(smack_inode_cache);
> +		kmem_cache_destroy(smack_rule_cache);
>   		return -ENOMEM;
>   	}
>   
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 2a8a1f5..d8a0e25 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -236,7 +236,7 @@ static int smk_set_access(struct smack_parsed_rule *srp,
>   	}
>   
>   	if (found == 0) {
> -		sp = kzalloc(sizeof(*sp), GFP_KERNEL);
> +		sp = kmem_cache_zalloc(smack_rule_cache, GFP_KERNEL);
>   		if (sp == NULL) {
>   			rc = -ENOMEM;
>   			goto out;

^ permalink raw reply

* [PATCH 35/38] vfs: Convert securityfs to fs_context
From: David Howells @ 2019-03-14 16:13 UTC (permalink / raw)
  To: viro; +Cc: linux-security-module, dhowells, linux-fsdevel, linux-kernel
In-Reply-To: <155257972443.13720.11743171471060355965.stgit@warthog.procyon.org.uk>

Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-security-module@vger.kernel.org
---

 security/inode.c |   21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/security/inode.c b/security/inode.c
index b7772a9b315e..4fecbc8b072a 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -16,6 +16,7 @@
 #include <linux/sysfs.h>
 #include <linux/kobject.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
 #include <linux/mount.h>
 #include <linux/pagemap.h>
 #include <linux/init.h>
@@ -40,7 +41,7 @@ static const struct super_operations securityfs_super_operations = {
 	.evict_inode	= securityfs_evict_inode,
 };
 
-static int fill_super(struct super_block *sb, void *data, int silent)
+static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	static const struct tree_descr files[] = {{""}};
 	int error;
@@ -54,17 +55,25 @@ static int fill_super(struct super_block *sb, void *data, int silent)
 	return 0;
 }
 
-static struct dentry *get_sb(struct file_system_type *fs_type,
-		  int flags, const char *dev_name,
-		  void *data)
+static int securityfs_get_tree(struct fs_context *fc)
 {
-	return mount_single(fs_type, flags, data, fill_super);
+	return vfs_get_super(fc, vfs_get_single_super, securityfs_fill_super);
+}
+
+static const struct fs_context_operations securityfs_context_ops = {
+	.get_tree	= securityfs_get_tree,
+};
+
+static int securityfs_init_fs_context(struct fs_context *fc)
+{
+	fc->ops = &securityfs_context_ops;
+	return 0;
 }
 
 static struct file_system_type fs_type = {
 	.owner =	THIS_MODULE,
 	.name =		"securityfs",
-	.mount =	get_sb,
+	.init_fs_context = securityfs_init_fs_context,
 	.kill_sb =	kill_litter_super,
 };
 


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox