Re: [RFC, PATCH 1/1] rpmb: add Replay Protected Memory Block (RPMB) driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Ulf,

On Sat, 22 Jul 2023 at 03:41, Shyam Saini
<shyamsaini@xxxxxxxxxxxxxxxxxxx> wrote:

From: Alex Bennée <alex.bennee@xxxxxxxxxx>

[This is patch 1 from [1] Alex's submission and this RPMB layer was
originally proposed by [2]Thomas Winkler ]

A number of storage technologies support a specialised hardware
partition designed to be resistant to replay attacks. The underlying
HW protocols differ but the operations are common. The RPMB partition
cannot be accessed via standard block layer, but by a set of specific
commands: WRITE, READ, GET_WRITE_COUNTER, and PROGRAM_KEY. Such a
partition provides authenticated and replay protected access, hence
suitable as a secure storage.

The initial aim of this patch is to provide a simple RPMB Driver which
can be accessed by Linux's optee driver to facilitate fast-path for
RPMB access to optee OS(secure OS) during the boot time. [1] Currently,
Optee OS relies on user-tee supplicant to access eMMC RPMB partition.

A TEE device driver can claim the RPMB interface, for example, via
class_interface_register(). The RPMB driver provides a series of
operations for interacting with the device.

I don't quite follow this. More exactly, how will the TEE driver know
what RPMB device it should use?

I don't have complete code to for this yet, but i think OP-TEE driver
should register with RPMB subsystem and then we can have eMMC/UFS/NVMe
specific implementation for RPMB operations.

Linux optee driver can handle RPMB frames and pass it to RPMB subsystem

[1] U-Boot has mmc specific implementation

I think OPTEE-OS has CFG_RPMB_FS_DEV_ID option
CFG_RPMB_FS_DEV_ID=1 for /dev/mmcblk1rpmb, but in case if a
system has multiple RPMB devices such as UFS/eMMC/NVMe, one them
should be declared as secure storage and optee should access that one only.

Sumit, do you have suggestions for this ?



  * program_key - a one time operation for setting up a new device
  * get_capacity - introspect the device capacity
  * get_write_counter - check the write counter
  * write_blocks - write a series of blocks to the RPMB device
  * read_blocks - read a series of blocks from the RPMB device

The detailed operation of implementing the access is left to the TEE
device driver itself.

The framing details and HW specific bits (JDEC vs NVME frames) are
left to the lower level TEE driver to worry about.

Without kernel fast path to RPMB access doesn't work when IMA try to
extend ftpm's PCR registers.

This fast-path would require additional work in linux optee driver and
as well as in MMC driver.

[1] https://lore.kernel.org/lkml/20220405093759.1126835-2-alex.bennee@xxxxxxxxxx/
[2] https://lore.kernel.org/linux-mmc/1478548394-8184-2-git-send-email-tomas.winkler@xxxxxxxxx/
[3] https://optee.readthedocs.io/en/latest/architecture/secure_storage.html

Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
Signed-off-by: Tomas Winkler <tomas.winkler@xxxxxxxxx>
Signed-off-by: Shyam Saini <shyamsaini@xxxxxxxxxxxxxxxxxxx>


[...]

+/**
+ * rpmb_dev_find_device() - return first matching rpmb device
+ * @data: data for the match function
+ * @match: the matching function
+ *
+ * Return: matching rpmb device or NULL on failure
+ */
+static
+struct rpmb_dev *rpmb_dev_find_device(const void *data,
+                                     int (*match)(struct device *dev,
+                                                  const void *data))
+{
+       struct device *dev;
+
+       dev = class_find_device(&rpmb_class, NULL, data, match);
+
+       return dev ? to_rpmb_dev(dev) : NULL;
+}
+
+struct device_with_target {
+       const struct device *dev;
+       u8 target;
+};
+
+static int match_by_parent(struct device *dev, const void *data)
+{
+       const struct device_with_target *d = data;
+       struct rpmb_dev *rdev = to_rpmb_dev(dev);
+
+       return (d->dev && dev->parent == d->dev && rdev->target == d->target);
+}
+
+/**
+ * rpmb_dev_find_by_device() - retrieve rpmb device from the parent device
+ * @parent: parent device of the rpmb device
+ * @target: RPMB target/region within the physical device
+ *
+ * Return: NULL if there is no rpmb device associated with the parent device
+ */
+struct rpmb_dev *rpmb_dev_find_by_device(struct device *parent, u8 target)
+{
+       struct device_with_target t;
+
+       if (!parent)
+               return NULL;
+
+       t.dev = parent;
+       t.target = target;
+
+       return rpmb_dev_find_device(&t, match_by_parent);
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_find_by_device);

Is this what the TEE driver would be calling to find the rpmb device/partition?

yes, that's the idea.

+
+/**
+ * rpmb_dev_unregister() - unregister RPMB partition from the RPMB subsystem
+ * @rdev: the rpmb device to unregister
+ * Return:
+ * *        0 on success
+ * *        -EINVAL on wrong parameters
+ */
+int rpmb_dev_unregister(struct rpmb_dev *rdev)
+{
+       if (!rdev)
+               return -EINVAL;
+
+       mutex_lock(&rdev->lock);
+       rpmb_cdev_del(rdev);

I can't find the function above. I guess it should be included as a
part of the patch too?

Sorry for the confusion, this is leftover from original version
Will be removed in next iteration.

+       device_del(&rdev->dev);
+       mutex_unlock(&rdev->lock);
+
+       rpmb_dev_put(rdev);
+
+       return 0;
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_unregister);

[...]

+/**
+ * rpmb_dev_register - register RPMB partition with the RPMB subsystem
+ * @dev: storage device of the rpmb device
+ * @target: RPMB target/region within the physical device
+ * @ops: device specific operations
+ *
+ * Return: a pointer to rpmb device
+ */
+struct rpmb_dev *rpmb_dev_register(struct device *dev, u8 target,
+                                  const struct rpmb_ops *ops)
+{
+       struct rpmb_dev *rdev;
+       int id;
+       int ret;
+
+       if (!dev || !ops)
+               return ERR_PTR(-EINVAL);
+
+       if (!ops->program_key)
+               return ERR_PTR(-EINVAL);
+
+       if (!ops->get_capacity)
+               return ERR_PTR(-EINVAL);
+
+       if (!ops->get_write_counter)
+               return ERR_PTR(-EINVAL);
+
+       if (!ops->write_blocks)
+               return ERR_PTR(-EINVAL);
+
+       if (!ops->read_blocks)
+               return ERR_PTR(-EINVAL);
+
+       rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
+       if (!rdev)
+               return ERR_PTR(-ENOMEM);
+
+       id = ida_simple_get(&rpmb_ida, 0, 0, GFP_KERNEL);
+       if (id < 0) {
+               ret = id;
+               goto exit;
+       }
+
+       mutex_init(&rdev->lock);
+       rdev->ops = ops;
+       rdev->id = id;
+       rdev->target = target;
+
+       dev_set_name(&rdev->dev, "rpmb%d", id);
+       rdev->dev.class = &rpmb_class;
+       rdev->dev.parent = dev;
+
+       rpmb_cdev_prepare(rdev);

Ditto.

same as my last comment
+
+       ret = device_register(&rdev->dev);
+       if (ret)
+               goto exit;
+
+       rpmb_cdev_add(rdev);

Ditto.

same as above.

+
+       dev_dbg(&rdev->dev, "registered device\n");
+
+       return rdev;
+
+exit:
+       if (id >= 0)
+               ida_simple_remove(&rpmb_ida, id);
+       kfree(rdev);
+       return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_register);
+

[...]



[1] https://source.denx.de/u-boot/u-boot/-/commit/4853ad3e13e21462a86e09caee4ea27ae68e764b

Best Regards,
Shyam

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux