| Message ID | 20240629224706.1164950-1-james.hilliard1@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | util: Ensure swupdate_remove_directory unmounts path | expand |
On 30.06.24 00:47, James Hilliard wrote: > In the event that the DATADST_DIR is left mounted we should ensure > that it is unmounted prior to deletion, otherwise we may end up > being unable to remove DATADST_DIR and/or accidentially deleting > files inside the mounted filesystem. > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > core/util.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/core/util.c b/core/util.c > index 70f0d28f..b8d3f3ec 100644 > --- a/core/util.c > +++ b/core/util.c > @@ -169,6 +169,26 @@ void swupdate_create_directory(const char* path) { > } > > #ifndef CONFIG_NOCLEANUP > +static int _is_mount_point(const char *path, const char *parent_path) { > + struct stat path_stat, parent_stat; > + > + if (!stat(path, &path_stat)) { > + ERROR("stat for path %s failed: %s", path, strerror(errno)); > + return -errno; > + } > + > + if (!stat(parent_path, &parent_stat)) { > + ERROR("stat for parent path %s failed: %s", parent_path, strerror(errno)); > + return -errno; > + } > + > + if (path_stat.st_dev != parent_stat.st_dev) { > + return 1; Yes, this is the way I know, too. :-) > + } > + > + return 0; > +} > + > static int _remove_directory_cb(const char *fpath, const struct stat *sb, > int typeflag, struct FTW *ftwbuf) > { > @@ -187,7 +207,23 @@ int swupdate_remove_directory(const char* path) > ERROR("OOM: Directory %s not removed", path); > return -ENOMEM; > } > + > + ret = _is_mount_point(dpath, get_tmpdir()); > + if (ret < 0) > + goto out; > + > + if (ret) { > + WARN("Unexpected mountpoint, unmounting: %s", dpath); > + ret = swupdate_umount(dpath); > + if (ret && errno != EINVAL) { > + ret = -errno; > + ERROR("Can't unmount path %s: %s", dpath, strerror(errno)); > + goto out; I think it is ok - if it is mounted by SWUpdate, it should be possible to umount it, and if it doesn't work, we do not know what is happening, then it is safe just to go out. > + } > + } > + > ret = nftw(dpath, _remove_directory_cb, 64, FTW_DEPTH | FTW_PHYS); > +out: > free(dpath); > return ret; > } Reviewed-by: Stefano Babic <stefano.babic@swupdate.org> Best regards, Stefano
diff --git a/core/util.c b/core/util.c index 70f0d28f..b8d3f3ec 100644 --- a/core/util.c +++ b/core/util.c @@ -169,6 +169,26 @@ void swupdate_create_directory(const char* path) { } #ifndef CONFIG_NOCLEANUP +static int _is_mount_point(const char *path, const char *parent_path) { + struct stat path_stat, parent_stat; + + if (!stat(path, &path_stat)) { + ERROR("stat for path %s failed: %s", path, strerror(errno)); + return -errno; + } + + if (!stat(parent_path, &parent_stat)) { + ERROR("stat for parent path %s failed: %s", parent_path, strerror(errno)); + return -errno; + } + + if (path_stat.st_dev != parent_stat.st_dev) { + return 1; + } + + return 0; +} + static int _remove_directory_cb(const char *fpath, const struct stat *sb, int typeflag, struct FTW *ftwbuf) { @@ -187,7 +207,23 @@ int swupdate_remove_directory(const char* path) ERROR("OOM: Directory %s not removed", path); return -ENOMEM; } + + ret = _is_mount_point(dpath, get_tmpdir()); + if (ret < 0) + goto out; + + if (ret) { + WARN("Unexpected mountpoint, unmounting: %s", dpath); + ret = swupdate_umount(dpath); + if (ret && errno != EINVAL) { + ret = -errno; + ERROR("Can't unmount path %s: %s", dpath, strerror(errno)); + goto out; + } + } + ret = nftw(dpath, _remove_directory_cb, 64, FTW_DEPTH | FTW_PHYS); +out: free(dpath); return ret; }
In the event that the DATADST_DIR is left mounted we should ensure that it is unmounted prior to deletion, otherwise we may end up being unable to remove DATADST_DIR and/or accidentially deleting files inside the mounted filesystem. Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- core/util.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)