| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c244ed8a-ccd4-6e7d-501c-e3d7f2da3916@linaro.org>
Date: Thu, 29 Oct 2020 12:08:38 +0000
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Evan Green <evgreen@...omium.org>,
Rob Herring <robh+dt@...nel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>
Cc: Douglas Anderson <dianders@...omium.org>,
Stephen Boyd <swboyd@...omium.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/4] nvmem: core: Add support for keepout regions
Thanks Evan for doing this,
On 29/10/2020 00:28, Evan Green wrote:
> Introduce support into the nvmem core for arrays of register ranges
> that should not result in actual device access. For these regions a
> constant byte (repeated) is returned instead on read, and writes are
> quietly ignored and returned as successful.
>
> This is useful for instance if certain efuse regions are protected
> from access by Linux because they contain secret info to another part
> of the system (like an integrated modem).
>
> Signed-off-by: Evan Green <evgreen@...omium.org>
Overall the patch looks good for me.
I have applied just this patch for more testing in next!
I can pick up 1/4 and 4/4 once Rob's Ack/Reviews the patch!
thanks,
srini
> ---
>
> Changes in v3:
> - Use min()/max() macros instead of defining my own (Doug)
> - Comment changes to indicate sorting (Doug)
> - Add function to validate keepouts are proper (Doug)
>
> Changes in v2:
> - Introduced keepout regions into the core (Srini)
>
> drivers/nvmem/core.c | 153 ++++++++++++++++++++++++++++++++-
> include/linux/nvmem-provider.h | 17 ++++
> 2 files changed, 166 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index a09ff8409f600..177f5bf27c6d5 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -34,6 +34,8 @@ struct nvmem_device {
> struct bin_attribute eeprom;
> struct device *base_dev;
> struct list_head cells;
> + const struct nvmem_keepout *keepout;
> + unsigned int nkeepout;
> nvmem_reg_read_t reg_read;
> nvmem_reg_write_t reg_write;
> struct gpio_desc *wp_gpio;
> @@ -66,8 +68,8 @@ static LIST_HEAD(nvmem_lookup_list);
>
> static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
>
> -static int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
> - void *val, size_t bytes)
> +static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
> + void *val, size_t bytes)
> {
> if (nvmem->reg_read)
> return nvmem->reg_read(nvmem->priv, offset, val, bytes);
> @@ -75,8 +77,8 @@ static int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
> return -EINVAL;
> }
>
> -static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
> - void *val, size_t bytes)
> +static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
> + void *val, size_t bytes)
> {
> int ret;
>
> @@ -90,6 +92,88 @@ static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
> return -EINVAL;
> }
>
> +static int nvmem_access_with_keepouts(struct nvmem_device *nvmem,
> + unsigned int offset, void *val,
> + size_t bytes, int write)
> +{
> +
> + unsigned int end = offset + bytes;
> + unsigned int kend, ksize;
> + const struct nvmem_keepout *keepout = nvmem->keepout;
> + const struct nvmem_keepout *keepoutend = keepout + nvmem->nkeepout;
> + int rc;
> +
> + /*
> + * Skip all keepouts before the range being accessed.
> + * Keepouts are sorted.
> + */
> + while ((keepout < keepoutend) && (keepout->end <= offset))
> + keepout++;
> +
> + while ((offset < end) && (keepout < keepoutend)) {
> + /* Access the valid portion before the keepout. */
> + if (offset < keepout->start) {
> + kend = min(end, keepout->start);
> + ksize = kend - offset;
> + if (write)
> + rc = __nvmem_reg_write(nvmem, offset, val, ksize);
> + else
> + rc = __nvmem_reg_read(nvmem, offset, val, ksize);
> +
> + if (rc)
> + return rc;
> +
> + offset += ksize;
> + val += ksize;
> + }
> +
> + /*
> + * Now we're aligned to the start of this keepout zone. Go
> + * through it.
> + */
> + kend = min(end, keepout->end);
> + ksize = kend - offset;
> + if (!write)
> + memset(val, keepout->value, ksize);
> +
> + val += ksize;
> + offset += ksize;
> + keepout++;
> + }
> +
> + /*
> + * If we ran out of keepouts but there's still stuff to do, send it
> + * down directly
> + */
> + if (offset < end) {
> + ksize = end - offset;
> + if (write)
> + return __nvmem_reg_write(nvmem, offset, val, ksize);
> + else
> + return __nvmem_reg_read(nvmem, offset, val, ksize);
> + }
> +
> + return 0;
> +}
> +
> +static int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
> + void *val, size_t bytes)
> +{
> + if (!nvmem->nkeepout)
> + return __nvmem_reg_read(nvmem, offset, val, bytes);
> +
> + return nvmem_access_with_keepouts(nvmem, offset, val, bytes, false);
> +}
> +
> +static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
> + void *val, size_t bytes)
> +{
> + if (!nvmem->nkeepout)
> + return __nvmem_reg_write(nvmem, offset, val, bytes);
> +
> + return nvmem_access_with_keepouts(nvmem, offset, val, bytes, true);
> +}
> +
> #ifdef CONFIG_NVMEM_SYSFS
> static const char * const nvmem_type_str[] = {
> [NVMEM_TYPE_UNKNOWN] = "Unknown",
> @@ -533,6 +617,59 @@ nvmem_find_cell_by_name(struct nvmem_device *nvmem, const char *cell_id)
> return cell;
> }
>
> +static int nvmem_validate_keepouts(struct nvmem_device *nvmem)
> +{
> + unsigned int cur = 0;
> + const struct nvmem_keepout *keepout = nvmem->keepout;
> + const struct nvmem_keepout *keepoutend = keepout + nvmem->nkeepout;
> +
> + while (keepout < keepoutend) {
> + /* Ensure keepouts are sorted and don't overlap. */
> + if (keepout->start < cur) {
> + dev_err(&nvmem->dev,
> + "Keepout regions aren't sorted or overlap.\n");
> +
> + return -ERANGE;
> + }
> +
> + if (keepout->end < keepout->start) {
> + dev_err(&nvmem->dev,
> + "Invalid keepout region.\n");
> +
> + return -EINVAL;
> + }
> +
> + /*
> + * Validate keepouts (and holes between) don't violate
> + * word_size constraints.
> + */
> + if ((keepout->end - keepout->start < nvmem->word_size) ||
> + ((keepout->start != cur) &&
> + (keepout->start - cur < nvmem->word_size))) {
> +
> + dev_err(&nvmem->dev,
> + "Keepout regions violate word_size constraints.\n");
> +
> + return -ERANGE;
> + }
> +
> + /* Validate keepouts don't violate stride (alignment). */
> + if (!IS_ALIGNED(keepout->start, nvmem->stride) ||
> + !IS_ALIGNED(keepout->end, nvmem->stride)) {
> +
> + dev_err(&nvmem->dev,
> + "Keepout regions violate stride.\n");
> +
> + return -EINVAL;
> + }
> +
> + cur = keepout->end;
> + keepout++;
> + }
> +
> + return 0;
> +}
> +
> static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
> {
> struct device_node *parent, *child;
> @@ -647,6 +784,8 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> nvmem->type = config->type;
> nvmem->reg_read = config->reg_read;
> nvmem->reg_write = config->reg_write;
> + nvmem->keepout = config->keepout;
> + nvmem->nkeepout = config->nkeepout;
> if (!config->no_of_node)
> nvmem->dev.of_node = config->dev->of_node;
>
> @@ -671,6 +810,12 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> nvmem->dev.groups = nvmem_dev_groups;
> #endif
>
> + if (nvmem->nkeepout) {
> + rval = nvmem_validate_keepouts(nvmem);
> + if (rval)
> + goto err_put_device;
> + }
> +
> dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
>
> rval = device_register(&nvmem->dev);
> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> index 06409a6c40bcb..e162b757b6d54 100644
> --- a/include/linux/nvmem-provider.h
> +++ b/include/linux/nvmem-provider.h
> @@ -30,6 +30,19 @@ enum nvmem_type {
> #define NVMEM_DEVID_NONE (-1)
> #define NVMEM_DEVID_AUTO (-2)
>
> +/**
> + * struct nvmem_keepout - NVMEM register keepout range.
> + *
> + * @start: The first byte offset to avoid.
> + * @end: One beyond the last byte offset to avoid.
> + * @value: The byte to fill reads with for this region.
> + */
> +struct nvmem_keepout {
> + unsigned int start;
> + unsigned int end;
> + unsigned char value;
> +};
> +
> /**
> * struct nvmem_config - NVMEM device configuration
> *
> @@ -39,6 +52,8 @@ enum nvmem_type {
> * @owner: Pointer to exporter module. Used for refcounting.
> * @cells: Optional array of pre-defined NVMEM cells.
> * @ncells: Number of elements in cells.
> + * @keepout: Optional array of keepout ranges (sorted ascending by start).
> + * @nkeepout: Number of elements in the keepout array.
> * @type: Type of the nvmem storage
> * @read_only: Device is read-only.
> * @root_only: Device is accessibly to root only.
> @@ -66,6 +81,8 @@ struct nvmem_config {
> struct gpio_desc *wp_gpio;
> const struct nvmem_cell_info *cells;
> int ncells;
> + const struct nvmem_keepout *keepout;
> + unsigned int nkeepout;
> enum nvmem_type type;
> bool read_only;
> bool root_only;
>
Powered by blists - more mailing lists