[pacman-dev] [PATCH] libalpm: never attempt to remove a mountpoint

[Allan McRae]

From: Dave Reisner <dreisner at archlinux.org>

Arch Linux typically runs into this with /sys when upgrading the
filesystem package in build chroots, but LXC users might also run into
this, since their /sys is shared from the host and must, for security
reasons, be mounted RO.

I've neglected to add any tests for this because they would require root
in order to run. Current tests all pass with this patch and I've
confirmed the desired behavior in a VM. Incidentally, the first hunk of
this patch (skipping can_remove_file checks for directories) resolves the
case of API mountpoints being removed since they eventually fall into
unlink_file and fail with "contains files". However, this patch should
still be the Right Thing To Do™, as we can't possibly remove a directory
that is also a mountpoint.

Signed-off-by: Dave Reisner <dreisner at archlinux.org>

[Allan] Do not skip checking if directories can be removed. Instead test
if directories are mountpoints in can_remove_file.

Signed-off-by: Allan McRae <allan at archlinux.org>
---

This is an updated version of Dave's original patch.  Instead of skipping
all directory checks which could result in us trying to remove empty directories
on read-only file systems, test all directories to see if they are a mountpoint
during can_remove_file.   This is hugely inefficient...  it results in two stat
calls per directory, but it fixes the current issue.  A better fix would require
completely rewriting the pre-removal checks to be done globally and not on a per
package basis.

 lib/libalpm/remove.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
index 50fc93c..64888c5 100644
--- a/lib/libalpm/remove.c
+++ b/lib/libalpm/remove.c
@@ -267,6 +267,37 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data)
 	return 0;
 }
 
+static int dir_is_mountpoint(alpm_handle_t *handle, const char *directory,
+		const struct stat *stbuf)
+{
+	char parent_dir[PATH_MAX];
+	struct stat parent_stbuf;
+	dev_t dir_st_dev;
+
+	if(stbuf == NULL) {
+		struct stat dir_stbuf;
+		if(stat(directory, &dir_stbuf) < 0) {
+			_alpm_log(handle, ALPM_LOG_DEBUG,
+					"failed to stat directory %s: %s",
+					directory, strerror(errno));
+			return 0;
+		}
+		dir_st_dev = dir_stbuf.st_dev;
+	} else {
+		dir_st_dev = stbuf->st_dev;
+	}
+
+	snprintf(parent_dir, PATH_MAX, "%s..", directory);
+	if(stat(parent_dir, &parent_stbuf) < 0) {
+		_alpm_log(handle, ALPM_LOG_DEBUG,
+				"failed to stat parent of %s: %s: %s",
+				directory, parent_dir, strerror(errno));
+		return 0;
+	}
+
+	return dir_st_dev != parent_stbuf.st_dev;
+}
+
 /**
  * @brief Check if alpm can delete a file.
  *
@@ -286,6 +317,12 @@ static int can_remove_file(alpm_handle_t *handle, const alpm_file_t *file,
 		return 1;
 	}
 
+	if(file->name[strlen(file->name) - 1] == '/' &&
+			dir_is_mountpoint(handle, file->name, NULL)) {
+		/* we do not remove mountpoints */
+		return 1;
+	}
+
 	snprintf(filepath, PATH_MAX, "%s%s", handle->root, file->name);
 	/* If we fail write permissions due to a read-only filesystem, abort.
 	 * Assume all other possible failures are covered somewhere else */
@@ -427,6 +464,9 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg,
 					fileobj->name)) {
 			_alpm_log(handle, ALPM_LOG_DEBUG,
 					"keeping directory %s (in new package)\n", file);
+		} else if(dir_is_mountpoint(handle, file, &buf)) {
+			_alpm_log(handle, ALPM_LOG_DEBUG,
+					"keeping directory %s (mountpoint)\n", file);
 		} else {
 			/* one last check- does any other package own this file? */
 			alpm_list_t *local, *local_pkgs;
-- 
1.8.1.3