| Message ID | 20191021203157.28524-3-adrian.freihofer@siemens.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | lstat instead of stat | expand |
On 21/10/19 22:31, 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 > image must be written, swupdate temporarily switches to read/write > mode. > > Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com> > --- > handlers/raw_handler.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 84 insertions(+) > > diff --git a/handlers/raw_handler.c b/handlers/raw_handler.c > index ba87191..6dfbe7d 100644 > --- a/handlers/raw_handler.c > +++ b/handlers/raw_handler.c > @@ -27,12 +27,91 @@ void raw_image_handler(void); > void raw_file_handler(void); > void raw_copyimage_handler(void); > > +/** > + * Handle write protection for block devices > + * > + * Automatically remove write protection for block devices if: > + * - The device name starts with /dev/* > + * - The device is a block device > + * - A corresponding ro flag e.g. /sys/class/block/mmcblk0boot0/force_ro is available > + * - The force_ro flag can be opened writeable > + */ > +static int blkprotect(struct img_type *img, bool on) > +{ > + const char c_sys_path[] = "/sys/class/block/%s/force_ro"; > + const char c_unprot_char = '0'; > + const char c_prot_char = '1'; > + int ret = 0; // 0 means OK nothing to do, 1 OK unprotected, 2 OK protected, negative means error > + int ret_int = 0; > + char *sysfs_path = NULL; > + int fd_force_ro; > + struct stat sb; > + char current_prot; > + > + if (strncmp("/dev/", img->device, 5) != 0) { > + return ret; > + } > + > + if (lstat(img->device, &sb) == -1) { > + TRACE("stat for device %s failed: %s", img->device, strerror(errno)); > + return ret; > + } > + if(!S_ISBLK(sb.st_mode)) { > + return ret; > + } > + > + ret_int = asprintf(&sysfs_path, c_sys_path, img->device + 5); // remove "/dev/" from device path > + if(ret_int < 0) { > + ret = -ENOMEM; > + goto blkprotect_out; > + } > + > + if (access(sysfs_path, W_OK) == -1) { > + goto blkprotect_out; > + } > + > + // There is a ro flag, the device needs to be protected or unprotected > + fd_force_ro = open(sysfs_path, O_RDWR); > + if (fd_force_ro == -1) { > + ret = -EBADF; > + goto blkprotect_out; > + } > + > + ssize_t n = read(fd_force_ro, ¤t_prot, 1); > + if (n != 1) { > + ret = -EBADFD; > + } > + if (on == false) { > + if (current_prot == c_prot_char) { > + write(fd_force_ro, &c_unprot_char, 1); > + TRACE("Device %s: read-only protection disabled", img->device); > + ret = 1; > + } > + } else { > + if (current_prot == c_unprot_char) { > + write(fd_force_ro, &c_prot_char, 1); > + TRACE("Device %s: read-only protection enabled", img->device); > + ret = 2; > + } > + } > + close(fd_force_ro); > + > +blkprotect_out: > + if(sysfs_path) > + free(sysfs_path); > + return ret; > +} > + > static int install_raw_image(struct img_type *img, > void __attribute__ ((__unused__)) *data) > { > int ret; > int fdout; > > + int prot_stat = blkprotect(img, false); > + if (prot_stat < 0) > + return prot_stat; > + > fdout = open(img->device, O_RDWR); > if (fdout < 0) { > TRACE("Device %s cannot be opened: %s", > @@ -45,6 +124,11 @@ static int install_raw_image(struct img_type *img, > ret = copyimage(&fdout, img, NULL); > #endif > > + if (prot_stat == 1) { > + fsync(fdout); // At least with Linux 4.14 data are not automatically flushed before ro mode is enabled > + blkprotect(img, true); // no error handling, keep ret from copyimage > + } > + > close(fdout); > return ret; > } > Acked-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
Hi Adrian, sorry, there is still an issue: On 22/10/19 11:54, Stefano Babic wrote: > On 21/10/19 22:31, 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 >> image must be written, swupdate temporarily switches to read/write >> mode. >> >> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com> >> --- >> handlers/raw_handler.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 84 insertions(+) >> >> diff --git a/handlers/raw_handler.c b/handlers/raw_handler.c >> index ba87191..6dfbe7d 100644 >> --- a/handlers/raw_handler.c >> +++ b/handlers/raw_handler.c >> @@ -27,12 +27,91 @@ void raw_image_handler(void); >> void raw_file_handler(void); >> void raw_copyimage_handler(void); >> >> +/** >> + * Handle write protection for block devices >> + * >> + * Automatically remove write protection for block devices if: >> + * - The device name starts with /dev/* >> + * - The device is a block device >> + * - A corresponding ro flag e.g. /sys/class/block/mmcblk0boot0/force_ro is available >> + * - The force_ro flag can be opened writeable >> + */ >> +static int blkprotect(struct img_type *img, bool on) >> +{ >> + const char c_sys_path[] = "/sys/class/block/%s/force_ro"; >> + const char c_unprot_char = '0'; >> + const char c_prot_char = '1'; >> + int ret = 0; // 0 means OK nothing to do, 1 OK unprotected, 2 OK protected, negative means error >> + int ret_int = 0; >> + char *sysfs_path = NULL; >> + int fd_force_ro; >> + struct stat sb; >> + char current_prot; >> + >> + if (strncmp("/dev/", img->device, 5) != 0) { >> + return ret; >> + } >> + >> + if (lstat(img->device, &sb) == -1) { >> + TRACE("stat for device %s failed: %s", img->device, strerror(errno)); >> + return ret; >> + } >> + if(!S_ISBLK(sb.st_mode)) { >> + return ret; >> + } >> + >> + ret_int = asprintf(&sysfs_path, c_sys_path, img->device + 5); // remove "/dev/" from device path >> + if(ret_int < 0) { >> + ret = -ENOMEM; >> + goto blkprotect_out; >> + } >> + >> + if (access(sysfs_path, W_OK) == -1) { >> + goto blkprotect_out; >> + } >> + >> + // There is a ro flag, the device needs to be protected or unprotected >> + fd_force_ro = open(sysfs_path, O_RDWR); >> + if (fd_force_ro == -1) { >> + ret = -EBADF; >> + goto blkprotect_out; >> + } >> + >> + ssize_t n = read(fd_force_ro, ¤t_prot, 1); >> + if (n != 1) { >> + ret = -EBADFD; >> + } >> + if (on == false) { >> + if (current_prot == c_prot_char) { >> + write(fd_force_ro, &c_unprot_char, 1); This generates a warning because the return value is not checked. Please fix it, thanks. I also thought about if this could be written in a more concise and readable way. Instead of : + if (on == false) { ...... something like: if (current_prot != (on ? c_prot_char : c_unprot_char) { ret=write(fd_force_ro, on ? &c_prot_char : &c_unprot_char, 1); .. } What do you think ? Best regards, Stefano Babic >> + TRACE("Device %s: read-only protection disabled", img->device); >> + ret = 1; >> + } >> + } else { >> + if (current_prot == c_unprot_char) { >> + write(fd_force_ro, &c_prot_char, 1); Ditto. >> + TRACE("Device %s: read-only protection enabled", img->device); >> + ret = 2; >> + } >> + } >> + close(fd_force_ro); >> + >> +blkprotect_out: >> + if(sysfs_path) >> + free(sysfs_path); >> + return ret; >> +} >> + >> static int install_raw_image(struct img_type *img, >> void __attribute__ ((__unused__)) *data) >> { >> int ret; >> int fdout; >> >> + int prot_stat = blkprotect(img, false); >> + if (prot_stat < 0) >> + return prot_stat; >> + >> fdout = open(img->device, O_RDWR); >> if (fdout < 0) { >> TRACE("Device %s cannot be opened: %s", >> @@ -45,6 +124,11 @@ static int install_raw_image(struct img_type *img, >> ret = copyimage(&fdout, img, NULL); >> #endif >> >> + if (prot_stat == 1) { >> + fsync(fdout); // At least with Linux 4.14 data are not automatically flushed before ro mode is enabled >> + blkprotect(img, true); // no error handling, keep ret from copyimage >> + } >> + >> close(fdout); >> return ret; >> } >> > > Acked-by: Stefano Babic <sbabic@denx.de> > > Best regards, > Stefano Babic > >
diff --git a/handlers/raw_handler.c b/handlers/raw_handler.c index ba87191..6dfbe7d 100644 --- a/handlers/raw_handler.c +++ b/handlers/raw_handler.c @@ -27,12 +27,91 @@ void raw_image_handler(void); void raw_file_handler(void); void raw_copyimage_handler(void); +/** + * Handle write protection for block devices + * + * Automatically remove write protection for block devices if: + * - The device name starts with /dev/* + * - The device is a block device + * - A corresponding ro flag e.g. /sys/class/block/mmcblk0boot0/force_ro is available + * - The force_ro flag can be opened writeable + */ +static int blkprotect(struct img_type *img, bool on) +{ + const char c_sys_path[] = "/sys/class/block/%s/force_ro"; + const char c_unprot_char = '0'; + const char c_prot_char = '1'; + int ret = 0; // 0 means OK nothing to do, 1 OK unprotected, 2 OK protected, negative means error + int ret_int = 0; + char *sysfs_path = NULL; + int fd_force_ro; + struct stat sb; + char current_prot; + + if (strncmp("/dev/", img->device, 5) != 0) { + return ret; + } + + if (lstat(img->device, &sb) == -1) { + TRACE("stat for device %s failed: %s", img->device, strerror(errno)); + return ret; + } + if(!S_ISBLK(sb.st_mode)) { + return ret; + } + + ret_int = asprintf(&sysfs_path, c_sys_path, img->device + 5); // remove "/dev/" from device path + if(ret_int < 0) { + ret = -ENOMEM; + goto blkprotect_out; + } + + if (access(sysfs_path, W_OK) == -1) { + goto blkprotect_out; + } + + // There is a ro flag, the device needs to be protected or unprotected + fd_force_ro = open(sysfs_path, O_RDWR); + if (fd_force_ro == -1) { + ret = -EBADF; + goto blkprotect_out; + } + + ssize_t n = read(fd_force_ro, ¤t_prot, 1); + if (n != 1) { + ret = -EBADFD; + } + if (on == false) { + if (current_prot == c_prot_char) { + write(fd_force_ro, &c_unprot_char, 1); + TRACE("Device %s: read-only protection disabled", img->device); + ret = 1; + } + } else { + if (current_prot == c_unprot_char) { + write(fd_force_ro, &c_prot_char, 1); + TRACE("Device %s: read-only protection enabled", img->device); + ret = 2; + } + } + close(fd_force_ro); + +blkprotect_out: + if(sysfs_path) + free(sysfs_path); + return ret; +} + static int install_raw_image(struct img_type *img, void __attribute__ ((__unused__)) *data) { int ret; int fdout; + int prot_stat = blkprotect(img, false); + if (prot_stat < 0) + return prot_stat; + fdout = open(img->device, O_RDWR); if (fdout < 0) { TRACE("Device %s cannot be opened: %s", @@ -45,6 +124,11 @@ static int install_raw_image(struct img_type *img, ret = copyimage(&fdout, img, NULL); #endif + if (prot_stat == 1) { + fsync(fdout); // At least with Linux 4.14 data are not automatically flushed before ro mode is enabled + blkprotect(img, true); // no error handling, keep ret from copyimage + } + close(fdout); 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 image must be written, swupdate temporarily switches to read/write mode. Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com> --- handlers/raw_handler.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+)