[PATCH] util: Ensure swupdate_remove_directory unmounts path

10 views
Skip to first unread message

James Hilliard

unread,
Jun 29, 2024, 6:47:18 PM (4 days ago) Jun 29
to swup...@googlegroups.com, James Hilliard
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.h...@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;
+ }
+
+ 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;
}
--
2.34.1

Stefano Babic

unread,
Jun 30, 2024, 8:20:21 AM (4 days ago) Jun 30
to James Hilliard, swup...@googlegroups.com
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 <stefan...@swupdate.org>

Best regards,
Stefano

James Hilliard

unread,
3:28 PM (5 hours ago) 3:28 PM
to swup...@googlegroups.com, James Hilliard
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.h...@gmail.com>
---
Changes v1 -> v2:
- fix stat return value check
---
core/util.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/core/util.c b/core/util.c
index 70f0d28f..33a9af71 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;
}
--
2.34.1

Reply all
Reply to author
Forward
0 new messages