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

[Ulf Hansson]

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?

>
>   * 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?

> +
> +/**
> + * 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?

> +       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.

> +
> +       ret = device_register(&rdev->dev);
> +       if (ret)
> +               goto exit;
> +
> +       rpmb_cdev_add(rdev);

Ditto.

> +
> +       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);
> +

[...]

Kind regards
Uffe