> "Winkler, Tomas" <tomas.winkler@xxxxxxxxx> writes: > > >> The user space API is achieved via a number of synchronous IOCTLs. > >> > >> * RPMB_IOC_VER_CMD - simple versioning API > >> * RPMB_IOC_CAP_CMD - query of underlying capabilities > >> * RPMB_IOC_PKEY_CMD - one time programming of access key > >> * RPMB_IOC_COUNTER_CMD - query the write counter > >> * RPMB_IOC_WBLOCKS_CMD - write blocks to device > >> * RPMB_IOC_RBLOCKS_CMD - read blocks from device > >> > >> The keys used for programming and writing blocks to the device are > >> key_serial_t handles as provided by the keyctl() interface. > >> > >> [AJB: here there are two key differences between this and the > >> original proposal. The first is the dropping of the sequence of > >> preformated frames in favour of explicit actions. The second is the > >> introduction of key_serial_t and the keyring API for referencing the > >> key to use] > > > > Putting it gently I'm not sure this is good idea, from the security point of > view. > > The key has to be possession of the one that signs the frames as they are, > it doesn't mean it is linux kernel keyring, it can be other party on different > system. > > With this approach you will make the other usecases not applicable. It > > is less then trivial to move key securely from one system to another. > > OK I can understand the desire for such a use-case but it does constrain the > interface on the kernel with access to the hardware to purely providing a > pipe to the raw hardware while also having to expose the details of the HW > to userspace. This is the use case in Android. The key is in the "trusty" which different os running in a virtual environment. The file storage abstraction is implemented there. I'm not sure the point of constraining the kernel, can you please elaborate on that. Also doesn't this break down after a PROGRAM_KEY event as > the key will have had to traverse into the "untrusted" kernel? This is one in a life event of the card happening on the manufacturing floor, maybe even not performed on Linux. > I wonder if virtio-rpmb may be of help here? You could wrap up up the front- > end in the security domain that has the keys although I don't know how easy > it would be for a backend to work with real hardware? I'm open to see any proposal, not sure I can wrap may head about it right now. Anyway I was about to send the new round of my code, but let's come to common ground first. Thanks Tomas > > > We all wished it can be abstracted more but the frames has to come > already signed, and the key material is inside the frame. > > Thanks > > Tomas > > > > > >> > >> Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx> > >> Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx> > >> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx> > >> Cc: Arnd Bergmann <arnd.bergmann@xxxxxxxxxx> > >> Cc: Ilias Apalodimas <ilias.apalodimas@xxxxxxxxxx> > >> Cc: Tomas Winkler <tomas.winkler@xxxxxxxxx> > >> Cc: Alexander Usyskin <alexander.usyskin@xxxxxxxxx> > >> Cc: Avri Altman <avri.altman@xxxxxxxxxxx> > >> --- > >> .../userspace-api/ioctl/ioctl-number.rst | 1 + > >> MAINTAINERS | 1 + > >> drivers/char/rpmb/Kconfig | 7 + > >> drivers/char/rpmb/Makefile | 1 + > >> drivers/char/rpmb/cdev.c | 246 ++++++++++++++++++ > >> drivers/char/rpmb/core.c | 10 +- > >> drivers/char/rpmb/rpmb-cdev.h | 17 ++ > >> include/linux/rpmb.h | 10 + > >> include/uapi/linux/rpmb.h | 68 +++++ > >> 9 files changed, 357 insertions(+), 4 deletions(-) create mode > >> 100644 drivers/char/rpmb/cdev.c create mode 100644 > >> drivers/char/rpmb/rpmb- cdev.h create mode 100644 > >> include/uapi/linux/rpmb.h > >> > >> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst > >> b/Documentation/userspace-api/ioctl/ioctl-number.rst > >> index a4c75a28c839..0ff2d4d81bb0 100644 > >> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst > >> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst > >> @@ -344,6 +344,7 @@ Code Seq# Include File > >> Comments > >> 0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux- > >> remoteproc@xxxxxxxxxxxxxxx> > >> 0xB6 all linux/fpga-dfl.h > >> 0xB7 all uapi/linux/remoteproc_cdev.h <mailto:linux- > >> remoteproc@xxxxxxxxxxxxxxx> > >> +0xB8 80-8F uapi/linux/rpmb.h <mailto:linux- > >> mmc@xxxxxxxxxxxxxxx> > >> 0xC0 00-0F linux/usb/iowarrior.h > >> 0xCA 00-0F uapi/misc/cxl.h > >> 0xCA 10-2F uapi/misc/ocxl.h > >> diff --git a/MAINTAINERS b/MAINTAINERS index > >> 076f3983526c..c60b41b6e6bd 100644 > >> --- a/MAINTAINERS > >> +++ b/MAINTAINERS > >> @@ -15374,6 +15374,7 @@ M: ? > >> L: linux-kernel@xxxxxxxxxxxxxxx > >> S: Supported > >> F: drivers/char/rpmb/* > >> +F: include/uapi/linux/rpmb.h > >> F: include/linux/rpmb.h > >> > >> RTL2830 MEDIA DRIVER > >> diff --git a/drivers/char/rpmb/Kconfig b/drivers/char/rpmb/Kconfig > >> index 431c2823cf70..9068664a399a 100644 > >> --- a/drivers/char/rpmb/Kconfig > >> +++ b/drivers/char/rpmb/Kconfig > >> @@ -9,3 +9,10 @@ config RPMB > >> access RPMB partition. > >> > >> If unsure, select N. > >> + > >> +config RPMB_INTF_DEV > >> + bool "RPMB character device interface /dev/rpmbN" > >> + depends on RPMB && KEYS > >> + help > >> + Say yes here if you want to access RPMB from user space > >> + via character device interface /dev/rpmb%d > >> diff --git a/drivers/char/rpmb/Makefile b/drivers/char/rpmb/Makefile > >> index 24d4752a9a53..f54b3f30514b 100644 > >> --- a/drivers/char/rpmb/Makefile > >> +++ b/drivers/char/rpmb/Makefile > >> @@ -3,5 +3,6 @@ > >> > >> obj-$(CONFIG_RPMB) += rpmb.o > >> rpmb-objs += core.o > >> +rpmb-$(CONFIG_RPMB_INTF_DEV) += cdev.o > >> > >> ccflags-y += -D__CHECK_ENDIAN__ > >> diff --git a/drivers/char/rpmb/cdev.c b/drivers/char/rpmb/cdev.c new > >> file mode 100644 index 000000000000..55f66720fd03 > >> --- /dev/null > >> +++ b/drivers/char/rpmb/cdev.c > >> @@ -0,0 +1,246 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Copyright(c) 2015 - 2019 Intel Corporation. > >> + */ > >> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > >> + > >> +#include <linux/fs.h> > >> +#include <linux/uaccess.h> > >> +#include <linux/compat.h> > >> +#include <linux/slab.h> > >> +#include <linux/capability.h> > >> + > >> +#include <linux/rpmb.h> > >> + > >> +#include "rpmb-cdev.h" > >> + > >> +static dev_t rpmb_devt; > >> +#define RPMB_MAX_DEVS MINORMASK > >> + > >> +#define RPMB_DEV_OPEN 0 /** single open bit (position) */ > >> + > >> +/** > >> + * rpmb_open - the open function > >> + * > >> + * @inode: pointer to inode structure > >> + * @fp: pointer to file structure > >> + * > >> + * Return: 0 on success, <0 on error */ static int rpmb_open(struct > >> +inode *inode, struct file *fp) { > >> + struct rpmb_dev *rdev; > >> + > >> + rdev = container_of(inode->i_cdev, struct rpmb_dev, cdev); > >> + if (!rdev) > >> + return -ENODEV; > >> + > >> + /* the rpmb is single open! */ > >> + if (test_and_set_bit(RPMB_DEV_OPEN, &rdev->status)) > >> + return -EBUSY; > >> + > >> + mutex_lock(&rdev->lock); > >> + > >> + fp->private_data = rdev; > >> + > >> + mutex_unlock(&rdev->lock); > >> + > >> + return nonseekable_open(inode, fp); } > >> + > >> +/** > >> + * rpmb_release - the cdev release function > >> + * > >> + * @inode: pointer to inode structure > >> + * @fp: pointer to file structure > >> + * > >> + * Return: 0 always. > >> + */ > >> +static int rpmb_release(struct inode *inode, struct file *fp) { > >> + struct rpmb_dev *rdev = fp->private_data; > >> + > >> + clear_bit(RPMB_DEV_OPEN, &rdev->status); > >> + > >> + return 0; > >> +} > >> + > >> +static long rpmb_ioctl_ver_cmd(struct rpmb_dev *rdev, > >> + struct rpmb_ioc_ver_cmd __user *ptr) { > >> + struct rpmb_ioc_ver_cmd ver = { > >> + .api_version = RPMB_API_VERSION, > >> + }; > >> + > >> + return copy_to_user(ptr, &ver, sizeof(ver)) ? -EFAULT : 0; } > >> + > >> +static long rpmb_ioctl_cap_cmd(struct rpmb_dev *rdev, > >> + struct rpmb_ioc_cap_cmd __user *ptr) { > >> + struct rpmb_ioc_cap_cmd cap; > >> + > >> + cap.target = rdev->target; > >> + cap.block_size = rdev->ops->block_size; > >> + cap.wr_cnt_max = rdev->ops->wr_cnt_max; > >> + cap.rd_cnt_max = rdev->ops->rd_cnt_max; > >> + cap.auth_method = rdev->ops->auth_method; > >> + cap.capacity = rpmb_get_capacity(rdev); > >> + cap.reserved = 0; > >> + > >> + return copy_to_user(ptr, &cap, sizeof(cap)) ? -EFAULT : 0; } > >> + > >> +static long rpmb_ioctl_pkey_cmd(struct rpmb_dev *rdev, key_serial_t > >> +__user *k) { > >> + key_serial_t keyid; > >> + > >> + if (get_user(keyid, k)) > >> + return -EFAULT; > >> + else > >> + return rpmb_program_key(rdev, keyid); } > >> + > >> +static long rpmb_ioctl_counter_cmd(struct rpmb_dev *rdev, int __user > >> +*ptr) { > >> + int count = rpmb_get_write_count(rdev); > >> + > >> + if (count > 0) > >> + return put_user(count, ptr); > >> + else > >> + return count; > >> +} > >> + > >> +static long rpmb_ioctl_wblocks_cmd(struct rpmb_dev *rdev, > >> + struct rpmb_ioc_blocks_cmd __user *ptr) { > >> + struct rpmb_ioc_blocks_cmd wblocks; > >> + int sz; > >> + long ret; > >> + u8 *data; > >> + > >> + if (copy_from_user(&wblocks, ptr, sizeof(struct > >> rpmb_ioc_blocks_cmd))) > >> + return -EFAULT; > >> + > >> + /* Don't write more blocks device supports */ > >> + if (wblocks.count > rdev->ops->wr_cnt_max) > >> + return -EINVAL; > >> + > >> + sz = wblocks.count * 256; > >> + data = kmalloc(sz, GFP_KERNEL); > >> + > >> + if (!data) > >> + return -ENOMEM; > >> + > >> + if (copy_from_user(data, wblocks.data, sz)) > >> + ret = -EFAULT; > >> + else > >> + ret = rpmb_write_blocks(rdev, wblocks.key, wblocks.addr, > >> +wblocks.count, data); > >> + > >> + kfree(data); > >> + return ret; > >> +} > >> + > >> +static long rpmb_ioctl_rblocks_cmd(struct rpmb_dev *rdev, > >> + struct rpmb_ioc_blocks_cmd __user *ptr) { > >> + struct rpmb_ioc_blocks_cmd rblocks; > >> + int sz; > >> + long ret; > >> + u8 *data; > >> + > >> + if (copy_from_user(&rblocks, ptr, sizeof(struct > >> rpmb_ioc_blocks_cmd))) > >> + return -EFAULT; > >> + > >> + if (rblocks.count > rdev->ops->rd_cnt_max) > >> + return -EINVAL; > >> + > >> + sz = rblocks.count * 256; > >> + data = kmalloc(sz, GFP_KERNEL); > >> + > >> + if (!data) > >> + return -ENOMEM; > >> + > >> + ret = rpmb_read_blocks(rdev, rblocks.addr, rblocks.count, data); > >> + > >> + if (ret == 0) > >> + ret = copy_to_user(rblocks.data, data, sz); > >> + > >> + kfree(data); > >> + return ret; > >> +} > >> + > >> +/** > >> + * rpmb_ioctl - rpmb ioctl dispatcher > >> + * > >> + * @fp: a file pointer > >> + * @cmd: ioctl command RPMB_IOC_SEQ_CMD RPMB_IOC_VER_CMD > >> +RPMB_IOC_CAP_CMD > >> + * @arg: ioctl data: rpmb_ioc_ver_cmd rpmb_ioc_cap_cmd > >> pmb_ioc_seq_cmd > >> + * > >> + * Return: 0 on success; < 0 on error */ static long > >> +rpmb_ioctl(struct file *fp, unsigned int cmd, unsigned long > >> +arg) { > >> + struct rpmb_dev *rdev = fp->private_data; > >> + void __user *ptr = (void __user *)arg; > >> + > >> + switch (cmd) { > >> + case RPMB_IOC_VER_CMD: > >> + return rpmb_ioctl_ver_cmd(rdev, ptr); > >> + case RPMB_IOC_CAP_CMD: > >> + return rpmb_ioctl_cap_cmd(rdev, ptr); > >> + case RPMB_IOC_PKEY_CMD: > >> + return rpmb_ioctl_pkey_cmd(rdev, ptr); > >> + case RPMB_IOC_COUNTER_CMD: > >> + return rpmb_ioctl_counter_cmd(rdev, ptr); > >> + case RPMB_IOC_WBLOCKS_CMD: > >> + return rpmb_ioctl_wblocks_cmd(rdev, ptr); > >> + case RPMB_IOC_RBLOCKS_CMD: > >> + return rpmb_ioctl_rblocks_cmd(rdev, ptr); > >> + default: > >> + dev_err(&rdev->dev, "unsupported ioctl 0x%x.\n", cmd); > >> + return -ENOIOCTLCMD; > >> + } > >> +} > >> + > >> +static const struct file_operations rpmb_fops = { > >> + .open = rpmb_open, > >> + .release = rpmb_release, > >> + .unlocked_ioctl = rpmb_ioctl, > >> + .owner = THIS_MODULE, > >> + .llseek = noop_llseek, > >> +}; > >> + > >> +void rpmb_cdev_prepare(struct rpmb_dev *rdev) { > >> + rdev->dev.devt = MKDEV(MAJOR(rpmb_devt), rdev->id); > >> + rdev->cdev.owner = THIS_MODULE; > >> + cdev_init(&rdev->cdev, &rpmb_fops); } > >> + > >> +void rpmb_cdev_add(struct rpmb_dev *rdev) { > >> + cdev_add(&rdev->cdev, rdev->dev.devt, 1); } > >> + > >> +void rpmb_cdev_del(struct rpmb_dev *rdev) { > >> + if (rdev->dev.devt) > >> + cdev_del(&rdev->cdev); > >> +} > >> + > >> +int __init rpmb_cdev_init(void) > >> +{ > >> + int ret; > >> + > >> + ret = alloc_chrdev_region(&rpmb_devt, 0, RPMB_MAX_DEVS, > >> "rpmb"); > >> + if (ret < 0) > >> + pr_err("unable to allocate char dev region\n"); > >> + > >> + return ret; > >> +} > >> + > >> +void __exit rpmb_cdev_exit(void) > >> +{ > >> + unregister_chrdev_region(rpmb_devt, RPMB_MAX_DEVS); } > >> diff --git a/drivers/char/rpmb/core.c b/drivers/char/rpmb/core.c > >> index > >> a2e21c14986a..e26d605e48e1 100644 > >> --- a/drivers/char/rpmb/core.c > >> +++ b/drivers/char/rpmb/core.c > >> @@ -12,6 +12,7 @@ > >> #include <linux/slab.h> > >> > >> #include <linux/rpmb.h> > >> +#include "rpmb-cdev.h" > >> > >> static DEFINE_IDA(rpmb_ida); > >> > >> @@ -277,6 +278,7 @@ int rpmb_dev_unregister(struct rpmb_dev *rdev) > >> return -EINVAL; > >> > >> mutex_lock(&rdev->lock); > >> + rpmb_cdev_del(rdev); > >> device_del(&rdev->dev); > >> mutex_unlock(&rdev->lock); > >> > >> @@ -371,9 +373,6 @@ struct rpmb_dev *rpmb_dev_register(struct > device > >> *dev, u8 target, > >> if (!ops->read_blocks) > >> return ERR_PTR(-EINVAL); > >> > >> - if (ops->type == RPMB_TYPE_ANY || ops->type > > >> RPMB_TYPE_MAX) > >> - return ERR_PTR(-EINVAL); > >> - > >> rdev = kzalloc(sizeof(*rdev), GFP_KERNEL); > >> if (!rdev) > >> return ERR_PTR(-ENOMEM); > >> @@ -396,6 +395,8 @@ struct rpmb_dev *rpmb_dev_register(struct > device > >> *dev, u8 target, > >> if (ret) > >> goto exit; > >> > >> + rpmb_cdev_add(rdev); > >> + > >> dev_dbg(&rdev->dev, "registered device\n"); > >> > >> return rdev; > >> @@ -412,11 +413,12 @@ static int __init rpmb_init(void) { > >> ida_init(&rpmb_ida); > >> class_register(&rpmb_class); > >> - return 0; > >> + return rpmb_cdev_init(); > >> } > >> > >> static void __exit rpmb_exit(void) > >> { > >> + rpmb_cdev_exit(); > >> class_unregister(&rpmb_class); > >> ida_destroy(&rpmb_ida); > >> } > >> diff --git a/drivers/char/rpmb/rpmb-cdev.h b/drivers/char/rpmb/rpmb- > >> cdev.h new file mode 100644 index 000000000000..e59ff0c05e9d > >> --- /dev/null > >> +++ b/drivers/char/rpmb/rpmb-cdev.h > >> @@ -0,0 +1,17 @@ > >> +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */ > >> +/* > >> + * Copyright (C) 2015-2018 Intel Corp. All rights reserved */ > >> +#ifdef CONFIG_RPMB_INTF_DEV int __init rpmb_cdev_init(void); void > >> +__exit rpmb_cdev_exit(void); void rpmb_cdev_prepare(struct > rpmb_dev > >> +*rdev); void rpmb_cdev_add(struct rpmb_dev *rdev); void > >> +rpmb_cdev_del(struct rpmb_dev *rdev); #else static inline int __init > >> +rpmb_cdev_init(void) { return 0; } static inline void __exit > >> +rpmb_cdev_exit(void) {} static inline void rpmb_cdev_prepare(struct > >> +rpmb_dev *rdev) {} static inline void rpmb_cdev_add(struct rpmb_dev > >> +*rdev) {} static inline void rpmb_cdev_del(struct rpmb_dev *rdev) {} > >> +#endif /* > >> CONFIG_RPMB_INTF_DEV > >> +*/ > >> diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h index > >> 718ba7c91ecd..fe44f60efe31 100644 > >> --- a/include/linux/rpmb.h > >> +++ b/include/linux/rpmb.h > >> @@ -8,9 +8,13 @@ > >> > >> #include <linux/types.h> > >> #include <linux/device.h> > >> +#include <linux/cdev.h> > >> +#include <uapi/linux/rpmb.h> > >> #include <linux/kref.h> > >> #include <linux/key.h> > >> > >> +#define RPMB_API_VERSION 0x80000001 > >> + > >> /** > >> * struct rpmb_ops - RPMB ops to be implemented by underlying block > >> device > >> * > >> @@ -51,6 +55,8 @@ struct rpmb_ops { > >> * @dev : device > >> * @id : device id > >> * @target : RPMB target/region within the physical device > >> + * @cdev : character dev > >> + * @status : device status > >> * @ops : operation exported by block layer > >> */ > >> struct rpmb_dev { > >> @@ -58,6 +64,10 @@ struct rpmb_dev { > >> struct device dev; > >> int id; > >> u8 target; > >> +#ifdef CONFIG_RPMB_INTF_DEV > >> + struct cdev cdev; > >> + unsigned long status; > >> +#endif /* CONFIG_RPMB_INTF_DEV */ > >> const struct rpmb_ops *ops; > >> }; > >> > >> diff --git a/include/uapi/linux/rpmb.h b/include/uapi/linux/rpmb.h > >> new file mode 100644 index 000000000000..3957b785cdd5 > >> --- /dev/null > >> +++ b/include/uapi/linux/rpmb.h > >> @@ -0,0 +1,68 @@ > >> +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR > >> +BSD-3-Clause) */ > >> +/* > >> + * Copyright (C) 2015-2018 Intel Corp. All rights reserved > >> + * Copyright (C) 2021 Linaro Ltd > >> + */ > >> +#ifndef _UAPI_LINUX_RPMB_H_ > >> +#define _UAPI_LINUX_RPMB_H_ > >> + > >> +#include <linux/types.h> > >> + > >> +/** > >> + * struct rpmb_ioc_ver_cmd - rpmb api version > >> + * > >> + * @api_version: rpmb API version. > >> + */ > >> +struct rpmb_ioc_ver_cmd { > >> + __u32 api_version; > >> +} __packed; > >> + > >> +enum rpmb_auth_method { > >> + RPMB_HMAC_ALGO_SHA_256 = 0, > >> +}; > >> + > >> +/** > >> + * struct rpmb_ioc_cap_cmd - rpmb capabilities > >> + * > >> + * @target: rpmb target/region within RPMB partition. > >> + * @capacity: storage capacity (in units of 128K) > >> + * @block_size: storage data block size (in units of 256B) > >> + * @wr_cnt_max: maximal number of block that can be written in a > >> +single > >> request. > >> + * @rd_cnt_max: maximal number of block that can be read in a single > >> request. > >> + * @auth_method: authentication method: currently always > >> HMAC_SHA_256 > >> + * @reserved: reserved to align to 4 bytes. > >> + */ > >> +struct rpmb_ioc_cap_cmd { > >> + __u16 target; > >> + __u16 capacity; > >> + __u16 block_size; > >> + __u16 wr_cnt_max; > >> + __u16 rd_cnt_max; > >> + __u16 auth_method; > >> + __u16 reserved; > >> +} __attribute__((packed)); > >> + > >> +/** > >> + * struct rpmb_ioc_blocks_cmd - read/write blocks to/from RPMB > >> + * > >> + * @keyid: key_serial_t of key to use > >> + * @addr: index into device (units of 256B blocks) > >> + * @count: number of 256B blocks > >> + * @data: pointer to data to write/read */ struct > >> +rpmb_ioc_blocks_cmd { > >> + __s32 key; /* key_serial_t */ > >> + __u32 addr; > >> + __u32 count; > >> + __u8 __user *data; > >> +} __attribute__((packed)); > >> + > >> + > >> +#define RPMB_IOC_VER_CMD _IOR(0xB8, 80, struct > rpmb_ioc_ver_cmd) > >> +#define RPMB_IOC_CAP_CMD _IOR(0xB8, 81, struct > rpmb_ioc_cap_cmd) > >> +#define RPMB_IOC_PKEY_CMD _IOW(0xB8, 82, key_serial_t) > >> +#define RPMB_IOC_COUNTER_CMD _IOR(0xB8, 84, int) #define > >> +RPMB_IOC_WBLOCKS_CMD _IOW(0xB8, 85, struct > rpmb_ioc_blocks_cmd) > >> #define > >> +RPMB_IOC_RBLOCKS_CMD _IOR(0xB8, 86, struct > rpmb_ioc_blocks_cmd) > >> + > >> +#endif /* _UAPI_LINUX_RPMB_H_ */ > >> -- > >> 2.20.1 > > > > -- > Alex Bennée
