[pacman-dev] [PATCH] FS#7484 - pacman clobbers directory symlinks.

Andrew Fyfe andrew at neptune-one.net
Thu Jul 12 15:53:04 EDT 2007


If a file/directory/symlink does not already exist on the filesystem,
then it is extracted as normal.

Never extract a package entry over a directory.

When extracting a directory:
- If a symlink already exists on the filesystem then the
  directory is not extracted.

  BUT if the symlink does not point to a directory we have a problem!
  What do we do in this situation? If we abort or continue we can
  end up breaking the system!

When extracting a symlink:
- If a directory already exists on the filesystem then the symlink is
  not extracted.

When extracting a reqular file
- Do the normal backup checks.

For all other cases the conflict checks have probably failed.

Signed-off-by: Andrew Fyfe <andrew at neptune-one.net>
---
 lib/libalpm/add.c |  110 ++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 76 insertions(+), 34 deletions(-)

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index b4345e9..81be53e 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -443,6 +443,7 @@ int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db)
 				int needbackup = 0, notouch = 0;
 				char *hash_orig = NULL;
 				struct stat buf;
+				mode_t entrytype = archive_entry_filetype(entry);
 
 				entryname = archive_entry_pathname(entry);
 
@@ -464,11 +465,7 @@ int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db)
 
 				memset(filename, 0, PATH_MAX); /* just to be sure */
 
-				if(strcmp(entryname, ".PKGINFO") == 0
-						|| strcmp(entryname, ".FILELIST") == 0) {
-					archive_read_data_skip(archive);
-					continue;
-				} else if(strcmp(entryname, ".INSTALL") == 0) {
+				if(strcmp(entryname, ".INSTALL") == 0) {
 					/* the install script goes inside the db */
 					snprintf(filename, PATH_MAX, "%s/%s-%s/install", db->path,
 									 newpkg->name, newpkg->version);
@@ -503,32 +500,79 @@ int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db)
 					continue;
 				}
 
-				/* check is file already exists */
-				if(stat(filename, &buf) == 0 && !S_ISDIR(buf.st_mode)) {
-					/* it does, is it a backup=() file?
-					 * always check the newpkg first, so when we do add a backup=() file,
-					 * we don't have to wait a full upgrade cycle */
-					needbackup = alpm_list_find_str(alpm_pkg_get_backup(newpkg), entryname);
-
-					if(is_upgrade) {
-						hash_orig = _alpm_needbackup(entryname, alpm_pkg_get_backup(oldpkg));
-						if(hash_orig) {
-							needbackup = 1;
+				if(lstat(filename, &buf) == 0) {
+					/* Entry exists on filesystem... */
+
+					if(S_ISDIR(buf.st_mode)) {
+						/* Don't extract anything over an existing directory.*/
+						if(!S_ISDIR(entrytype)) {
+							_alpm_log(PM_LOG_WARNING, _("%s won't be extracted over an existing directory."), entryname);
 						}
+
+						_alpm_log(PM_LOG_DEBUG, _("%s won't be extracted over an existing directory."), entryname);
+						archive_read_data_skip(archive);
+						continue;
 					}
 
-					/* this is kind of gross.  if we force hash_orig to be non-NULL we can
-					 * catch the pro-active backup=() case (when the backup entry is in
-					 * the new package, and not the old */
-					if(needbackup && !hash_orig) {
-						hash_orig = strdup("");
+					if(S_ISDIR(entrytype) && S_ISLNK(buf.st_mode)) {
+						/* Don't extract a directory over an existing symlink. */
+						if(stat(filename, &buf) == 0 && !S_ISDIR(buf.st_mode)) {
+							/* Something's going to break! A symlink exists where we
+							 * want to extract this directory, and it doesn't point to
+							 * a directory.
+							 *
+							 * This will cause problems if this package or any other
+							 * package try to extract files to this directory. */
+
+							/* TODO: How do we handle this? If we continue we leeave a
+							 * broken system, if we abort we leave a broken system! */
+							_alpm_log(PM_LOG_ERROR, _("%s is a symlink, but it does not point to a directory!"), entryname);
+						}
+
+						_alpm_log(PM_LOG_DEBUG, _("%s (directory) won't be extracted over existing symlink."), entryname);
+						archive_read_data_skip(archive);
+						continue;
 					}
-					
-					/* NoUpgrade skips all this backup stuff, because it's just never
-					 * touched */
-					if(alpm_list_find_str(handle->noupgrade, entryname)) {
-						notouch = 1;
-						needbackup = 0;
+
+					if(S_ISLNK(entrytype) && S_ISDIR(buf.st_mode)) {
+						/* Don't extract a symlink over a directory. */
+						_alpm_log(PM_LOG_DEBUG, _("%s (symlink) won't be extracted over existing directory."), entryname);
+						archive_read_data_skip(archive);
+						continue;
+					}
+
+					if(S_ISREG(entrytype)) {
+						/* always check the newpkg first, so when we do add a backup=() file,
+						 * we don't have to wait a full upgrade cycle */
+						needbackup = alpm_list_find_str(alpm_pkg_get_backup(newpkg), entryname);
+
+						if(is_upgrade) {
+							hash_orig = _alpm_needbackup(entryname, alpm_pkg_get_backup(oldpkg));
+							if(hash_orig) {
+								needbackup = 1;
+							}
+						}
+
+						/* this is kind of gross.  if we force hash_orig to be non-NULL we can
+						 * catch the pro-active backup=() case (when the backup entry is in
+						 * the new package, and not the old */
+						if(needbackup && !hash_orig) {
+							hash_orig = strdup("");
+						}
+
+						/* NoUpgrade skips all this backup stuff, because it's just never
+						 * touched */
+						if(alpm_list_find_str(handle->noupgrade, entryname)) {
+							notouch = 1;
+							needbackup = 0;
+						}
+					}
+					else {
+						/* We shouldn't be here! This should have already been
+						 * caught by the conflict check. */
+						_alpm_log(PM_LOG_ERROR, _("%s already exists on tthe filesystem."), entryname);
+						_alpm_log(PM_LOG_DEBUG, _("%s should have been caught by the conflict checks!"), entryname);
+						RET_ERR(PM_ERR_FILE_CONFLICTS, -1);
 					}
 				}
 
@@ -705,14 +749,12 @@ int _alpm_add_commit(pmtrans_t *trans, pmdb_t *db)
 					archive_entry_set_pathname(entry, filename);
 
 					/* FS #7484 
-					 * By default, libarchive 2.2.3 overwrites existing symlinks by directories from the archive,
-					 * which isn't the behavior we want.
-					 * This can be avoided by using the ARCHIVE_EXTRACT_NO_OVERWRITE flag, and this works
-					 * fine because all files where an overwrite could be needed are deleted first :
-					 * 1) if it's an upgrade, existing files are removed when the old pkg is removed
-					 * 2) if there is a file conflict, but --force is used, then files are also removed : see above
+					 * Andrew Fyfe:
+					 * Removed ARCHIVE_EXTRACT_NO_OVERWRITE from the next command.
+					 * The new checks above should catch all the directory extract/
+					 * overwrite issues for now.
 					 */
-					int ret = archive_read_extract(archive, entry, archive_flags | ARCHIVE_EXTRACT_NO_OVERWRITE);
+					int ret = archive_read_extract(archive, entry, archive_flags);
 					if(ret == ARCHIVE_WARN) {
 						/* operation succeeded but a non-critical error was encountered */
 						_alpm_log(PM_LOG_DEBUG, "warning extracting %s (%s)",
-- 
1.5.2.3





More information about the pacman-dev mailing list