[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