Message ID | 20191009162934.20998-4-adrian.freihofer@siemens.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [[libubootenv] v2 3/3] handle protected mmcblk_boot_ devices | expand |
Hi Adrian, On 09/10/19 18:29, Adrian Freihofer wrote: > Some block devices support physical write protection. The kernel > provides a standard interface to enable or disable protection in > /sys/class/block/*/force_ro. > > This patch adds functionality to automatically detect these memory > types. If read-only mode is enabled on the partition on which the > uboot environment must be written, libubootenv temporarily switches > to read/write mode. > > Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com> > --- > src/CMakeLists.txt | 2 + > src/mmcblkboot_ro.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/mmcblkboot_ro.h | 11 +++++ > src/uboot_env.c | 23 +++++++++ > 4 files changed, 171 insertions(+) > create mode 100644 src/mmcblkboot_ro.c > create mode 100644 src/mmcblkboot_ro.h > > diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt > index 051732b..0b7aeaf 100644 > --- a/src/CMakeLists.txt > +++ b/src/CMakeLists.txt > @@ -3,6 +3,8 @@ cmake_minimum_required (VERSION 2.6) > SET(libubootenv_SOURCES > uboot_env.c > uboot_private.h > + mmcblkboot_ro.c > + mmcblkboot_ro.h > ) > > # Public headers > diff --git a/src/mmcblkboot_ro.c b/src/mmcblkboot_ro.c > new file mode 100644 > index 0000000..f9f768e > --- /dev/null > +++ b/src/mmcblkboot_ro.c > @@ -0,0 +1,135 @@ > +/* > + * SPDX-License-Identifier: LGPL-2.1-or-later > + * > + * For some block device types, the uboot environment variables are stored in > + * a read-only partition. The kernel provides an interface > + * /sys/class/block/mmcblk?boot?/force_ro to enable and disable the write only > + * flag. > + * > + * The reprotect function does not enable write protection if the memory was > + * not protected before the unprotect function was called. > + */ > + > +#include <stdlib.h> > +#include <fcntl.h> > +#include <unistd.h> > +#include <string.h> > +#include <stdio.h> > +#include <errno.h> > +#include "mmcblkboot_ro.h" > + > +typedef void (*fp_unprotect)(env_protect_mmcblkboot_t*); > +typedef void (*fp_reprotect)(env_protect_mmcblkboot_t*); > + > + > +#define SYSFS_PATH_MAX 120 > + > +static const char c_sys_path_1[] = "/sys/class/block/"; > +static const char c_sys_path_2[] = "/force_ro"; > +static const char c_dev_name_1[] = "mmcblk"; > +static const char c_dev_name_2[] = "boot"; > + > +/** mmcblk_boot device specific class */ > +typedef struct env_protect_mmcblkboot { > + fp_unprotect unprotect; > + fp_reprotect reprotect; > + char sysfs_path[SYSFS_PATH_MAX]; > + char current_prot; > +} env_protect_mmcblkboot_t; > + > +/** Enables read/write mode for the given device */ > +void mmcblkboot_unprotect(env_protect_mmcblkboot_t *p_obj) { > + const char c_unprot_char = '0'; > + const char c_prot_char = '1'; > + int fd; > + ssize_t n; > + > + fd = open(p_obj->sysfs_path, O_RDWR); > + if (fd == -1) { > + return; > + } > + > + // Verify and archive the current write protect state, unprotect the device > + n = read(fd, &(p_obj->current_prot), 1); > + if (n == 1 && (p_obj->current_prot == c_unprot_char || p_obj->current_prot == c_prot_char)) { > + write(fd, &c_unprot_char, 1); > + } else { > + p_obj->current_prot = 0; // undefined state > + } > + close(fd); > +} > + > +/** re-activates the read only protection > + * > + * if it has been removed by the previous unprotect call. > + * > + */ > +void mmcblkboot_reprotect(env_protect_mmcblkboot_t *p_obj) { > + int fd; > + > + if (p_obj->current_prot != 0) { > + fd = open(p_obj->sysfs_path, O_WRONLY); > + if (fd == -1) { > + return; > + } > + write(fd, &(p_obj->current_prot), 1); > + close(fd); > + } > +} > + > +/** > + * constructor of env_protect_mmcblkboot_t > + * > + * Creates an oject assigned to pp_obj if: > + * - devname matches /dev/mmcblk[0-9]boot[0-9] > + * - if a corresponding sysfs entry "force_ro" exists > + * It returns 1 of the device needs to be unprotected, 0 if not. In case of an error, > + * a negative error code is returned. > + */ > +int mmcblkboot_create(env_protect_mmcblkboot_t **pp_obj, const char *devname) { > + env_protect_mmcblkboot_t *p_obj = NULL; > + const char *devfile = devname; > + int ret; > + > + if (strncmp("/dev/", devname, 5) == 0) { > + devfile = devname + 5; > + } else { > + return 0; > + } > + > + ret = strncmp(devfile, c_dev_name_1, sizeof(c_dev_name_1) - 1); > + if (ret != 0) { > + return 0; > + } > + > + if (strncmp(devfile + sizeof(c_dev_name_1), c_dev_name_2, sizeof(c_dev_name_2) - 1) != 0) { > + return 0; > + } > + > + if (*(devfile + sizeof(c_dev_name_1) - 1) < '0' || > + *(devfile + sizeof(c_dev_name_1) - 1) > '9') { > + return 0; > + } > + > + if (*(devfile + sizeof(c_dev_name_1) + sizeof(c_dev_name_2) - 1) < '0' || > + *(devfile + sizeof(c_dev_name_1) + sizeof(c_dev_name_2) - 1) > '9') { > + return 0; > + } > + > + p_obj = (env_protect_mmcblkboot_t *)calloc(1, sizeof(env_protect_mmcblkboot_t)); > + if (p_obj == NULL) { > + return -ENOMEM; > + } > + snprintf(p_obj->sysfs_path, SYSFS_PATH_MAX, "%s%s%s", c_sys_path_1, devfile, c_sys_path_2); > + > + if (access(p_obj->sysfs_path, W_OK) == -1) { > + free(p_obj); > + return 0; > + } > + > + p_obj->unprotect = mmcblkboot_unprotect; > + p_obj->reprotect = mmcblkboot_reprotect; > + > + *pp_obj = p_obj; > + return 1; > +} > diff --git a/src/mmcblkboot_ro.h b/src/mmcblkboot_ro.h > new file mode 100644 > index 0000000..b86c7d7 > --- /dev/null > +++ b/src/mmcblkboot_ro.h > @@ -0,0 +1,11 @@ > +/* > + * SPDX-License-Identifier: LGPL-2.1-or-later > + */ > + > +#pragma once > + > +typedef struct env_protect_mmcblkboot env_protect_mmcblkboot_t; > + > +int mmcblkboot_create(env_protect_mmcblkboot_t **pp_obj, const char *devname); > +void mmcblkboot_unprotect(env_protect_mmcblkboot_t *p_obj); > +void mmcblkboot_reprotect(env_protect_mmcblkboot_t *p_obj); Quite, but this does not guarantee the level of abstraction I will like to get. The above code is ok, but the code in uboot_env should be unaware of it and just call protect / unprotect function. Something like (just to render the idea): src/uboot_private.h: struct uboot_flash_env { ... enum device_type device_type; typedef void (*fp_protect) (struct uboot_flash_env *dev, bool on); }; And in filewrite: if (dev->protect) dev->protect(dev, on); .... if (dev->protect) dev->protect(dev, off); So that this function is unaware if device supports it or not, just call it. What do you mind ? Regards, Stefano > diff --git a/src/uboot_env.c b/src/uboot_env.c > index 726c433..4281468 100644 > --- a/src/uboot_env.c > +++ b/src/uboot_env.c > @@ -35,6 +35,7 @@ > #include <mtd/ubi-user.h> > > #include "uboot_private.h" > +#include "mmcblkboot_ro.h" > > #define DEVICE_MTD_NAME "/dev/mtd" > #define DEVICE_UBI_NAME "/dev/ubi" > @@ -491,12 +492,34 @@ static int devread(struct uboot_ctx *ctx, unsigned int copy, void *data) > static int filewrite(struct uboot_flash_env *dev, void *data) > { > int ret; > + env_protect_mmcblkboot_t *prot_handler = NULL; > + > + // remove hardware write protection if needed > + ret = mmcblkboot_create(&prot_handler, dev->devname); But if no block can be created (SD /SATA/..), the write is skipped. > + if(ret < 0) { > + goto filewrite_out; > + } > + if (prot_handler) { > + mmcblkboot_unprotect(prot_handler); > + } > > if (dev->offset) > lseek(dev->fd, dev->offset, SEEK_SET); > > ret = write(dev->fd, data, dev->envsize); > > + // ensure all data are written before we might enable the read-only flag on the environment > + fsync(dev->fd); > + > + // enforce hardware write protection again if it was active before this write > + if (prot_handler) { > + mmcblkboot_reprotect(prot_handler); > + } > + > +filewrite_out: > + if (prot_handler) { > + free(prot_handler); > + } > return ret; > } > >
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 051732b..0b7aeaf 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -3,6 +3,8 @@ cmake_minimum_required (VERSION 2.6) SET(libubootenv_SOURCES uboot_env.c uboot_private.h + mmcblkboot_ro.c + mmcblkboot_ro.h ) # Public headers diff --git a/src/mmcblkboot_ro.c b/src/mmcblkboot_ro.c new file mode 100644 index 0000000..f9f768e --- /dev/null +++ b/src/mmcblkboot_ro.c @@ -0,0 +1,135 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-or-later + * + * For some block device types, the uboot environment variables are stored in + * a read-only partition. The kernel provides an interface + * /sys/class/block/mmcblk?boot?/force_ro to enable and disable the write only + * flag. + * + * The reprotect function does not enable write protection if the memory was + * not protected before the unprotect function was called. + */ + +#include <stdlib.h> +#include <fcntl.h> +#include <unistd.h> +#include <string.h> +#include <stdio.h> +#include <errno.h> +#include "mmcblkboot_ro.h" + +typedef void (*fp_unprotect)(env_protect_mmcblkboot_t*); +typedef void (*fp_reprotect)(env_protect_mmcblkboot_t*); + + +#define SYSFS_PATH_MAX 120 + +static const char c_sys_path_1[] = "/sys/class/block/"; +static const char c_sys_path_2[] = "/force_ro"; +static const char c_dev_name_1[] = "mmcblk"; +static const char c_dev_name_2[] = "boot"; + +/** mmcblk_boot device specific class */ +typedef struct env_protect_mmcblkboot { + fp_unprotect unprotect; + fp_reprotect reprotect; + char sysfs_path[SYSFS_PATH_MAX]; + char current_prot; +} env_protect_mmcblkboot_t; + +/** Enables read/write mode for the given device */ +void mmcblkboot_unprotect(env_protect_mmcblkboot_t *p_obj) { + const char c_unprot_char = '0'; + const char c_prot_char = '1'; + int fd; + ssize_t n; + + fd = open(p_obj->sysfs_path, O_RDWR); + if (fd == -1) { + return; + } + + // Verify and archive the current write protect state, unprotect the device + n = read(fd, &(p_obj->current_prot), 1); + if (n == 1 && (p_obj->current_prot == c_unprot_char || p_obj->current_prot == c_prot_char)) { + write(fd, &c_unprot_char, 1); + } else { + p_obj->current_prot = 0; // undefined state + } + close(fd); +} + +/** re-activates the read only protection + * + * if it has been removed by the previous unprotect call. + * + */ +void mmcblkboot_reprotect(env_protect_mmcblkboot_t *p_obj) { + int fd; + + if (p_obj->current_prot != 0) { + fd = open(p_obj->sysfs_path, O_WRONLY); + if (fd == -1) { + return; + } + write(fd, &(p_obj->current_prot), 1); + close(fd); + } +} + +/** + * constructor of env_protect_mmcblkboot_t + * + * Creates an oject assigned to pp_obj if: + * - devname matches /dev/mmcblk[0-9]boot[0-9] + * - if a corresponding sysfs entry "force_ro" exists + * It returns 1 of the device needs to be unprotected, 0 if not. In case of an error, + * a negative error code is returned. + */ +int mmcblkboot_create(env_protect_mmcblkboot_t **pp_obj, const char *devname) { + env_protect_mmcblkboot_t *p_obj = NULL; + const char *devfile = devname; + int ret; + + if (strncmp("/dev/", devname, 5) == 0) { + devfile = devname + 5; + } else { + return 0; + } + + ret = strncmp(devfile, c_dev_name_1, sizeof(c_dev_name_1) - 1); + if (ret != 0) { + return 0; + } + + if (strncmp(devfile + sizeof(c_dev_name_1), c_dev_name_2, sizeof(c_dev_name_2) - 1) != 0) { + return 0; + } + + if (*(devfile + sizeof(c_dev_name_1) - 1) < '0' || + *(devfile + sizeof(c_dev_name_1) - 1) > '9') { + return 0; + } + + if (*(devfile + sizeof(c_dev_name_1) + sizeof(c_dev_name_2) - 1) < '0' || + *(devfile + sizeof(c_dev_name_1) + sizeof(c_dev_name_2) - 1) > '9') { + return 0; + } + + p_obj = (env_protect_mmcblkboot_t *)calloc(1, sizeof(env_protect_mmcblkboot_t)); + if (p_obj == NULL) { + return -ENOMEM; + } + snprintf(p_obj->sysfs_path, SYSFS_PATH_MAX, "%s%s%s", c_sys_path_1, devfile, c_sys_path_2); + + if (access(p_obj->sysfs_path, W_OK) == -1) { + free(p_obj); + return 0; + } + + p_obj->unprotect = mmcblkboot_unprotect; + p_obj->reprotect = mmcblkboot_reprotect; + + *pp_obj = p_obj; + return 1; +} diff --git a/src/mmcblkboot_ro.h b/src/mmcblkboot_ro.h new file mode 100644 index 0000000..b86c7d7 --- /dev/null +++ b/src/mmcblkboot_ro.h @@ -0,0 +1,11 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-or-later + */ + +#pragma once + +typedef struct env_protect_mmcblkboot env_protect_mmcblkboot_t; + +int mmcblkboot_create(env_protect_mmcblkboot_t **pp_obj, const char *devname); +void mmcblkboot_unprotect(env_protect_mmcblkboot_t *p_obj); +void mmcblkboot_reprotect(env_protect_mmcblkboot_t *p_obj); diff --git a/src/uboot_env.c b/src/uboot_env.c index 726c433..4281468 100644 --- a/src/uboot_env.c +++ b/src/uboot_env.c @@ -35,6 +35,7 @@ #include <mtd/ubi-user.h> #include "uboot_private.h" +#include "mmcblkboot_ro.h" #define DEVICE_MTD_NAME "/dev/mtd" #define DEVICE_UBI_NAME "/dev/ubi" @@ -491,12 +492,34 @@ static int devread(struct uboot_ctx *ctx, unsigned int copy, void *data) static int filewrite(struct uboot_flash_env *dev, void *data) { int ret; + env_protect_mmcblkboot_t *prot_handler = NULL; + + // remove hardware write protection if needed + ret = mmcblkboot_create(&prot_handler, dev->devname); + if(ret < 0) { + goto filewrite_out; + } + if (prot_handler) { + mmcblkboot_unprotect(prot_handler); + } if (dev->offset) lseek(dev->fd, dev->offset, SEEK_SET); ret = write(dev->fd, data, dev->envsize); + // ensure all data are written before we might enable the read-only flag on the environment + fsync(dev->fd); + + // enforce hardware write protection again if it was active before this write + if (prot_handler) { + mmcblkboot_reprotect(prot_handler); + } + +filewrite_out: + if (prot_handler) { + free(prot_handler); + } return ret; }
Some block devices support physical write protection. The kernel provides a standard interface to enable or disable protection in /sys/class/block/*/force_ro. This patch adds functionality to automatically detect these memory types. If read-only mode is enabled on the partition on which the uboot environment must be written, libubootenv temporarily switches to read/write mode. Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com> --- src/CMakeLists.txt | 2 + src/mmcblkboot_ro.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/mmcblkboot_ro.h | 11 +++++ src/uboot_env.c | 23 +++++++++ 4 files changed, 171 insertions(+) create mode 100644 src/mmcblkboot_ro.c create mode 100644 src/mmcblkboot_ro.h