From 3370dd586a81d9adb21beee7777bf3601ef003e3 Mon Sep 17 00:00:00 2001 From: morganamilo Date: Sun, 11 May 2025 00:52:01 +0100 Subject: [PATCH 1/4] libalpm: don't chown download dir if not root Commit 7bc5d55b56f41518e0a53eed13d4c523aea848e5 disbaled the chown after downloading if the current user is not root. It only makes sense for us to do the same on this side otherwise files would end up stuck as the download user. This is also important for other alpm tools that configure themselves via pacman.conf but download to user directories. --- lib/libalpm/util.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 7877b17a..00cc3734 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -955,10 +955,11 @@ const char *_alpm_filecache_setup(alpm_handle_t *handle) */ char *_alpm_temporary_download_dir_setup(const char *dir, const char *user) { + uid_t myuid = getuid(); struct passwd const *pw = NULL; ASSERT(dir != NULL, return NULL); - if(user != NULL) { + if(myuid == 0 && user != NULL) { ASSERT((pw = getpwnam(user)) != NULL, return NULL); } From f6aa79c10a1b4295f10d79b23a37f474b6b90cab Mon Sep 17 00:00:00 2001 From: morganamilo Date: Tue, 20 May 2025 12:28:24 +0100 Subject: [PATCH 2/4] libalpm: pass handle to alpm_temporary_download_dir_setup --- lib/libalpm/be_sync.c | 2 +- lib/libalpm/dload.c | 2 +- lib/libalpm/sync.c | 2 +- lib/libalpm/util.c | 5 +++-- lib/libalpm/util.h | 2 +- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 240f07be..155c6c54 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -153,7 +153,7 @@ int SYMEXPORT alpm_db_update(alpm_handle_t *handle, alpm_list_t *dbs, int force) syncpath = get_sync_dir(handle); ASSERT(syncpath != NULL, return -1); - temporary_syncpath = _alpm_temporary_download_dir_setup(syncpath, handle->sandboxuser); + temporary_syncpath = _alpm_temporary_download_dir_setup(handle, syncpath); ASSERT(temporary_syncpath != NULL, FREE(syncpath); return -1); /* make sure we have a sane umask */ diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 59ccdcd4..881cc437 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -1304,7 +1304,7 @@ int SYMEXPORT alpm_fetch_pkgurl(alpm_handle_t *handle, const alpm_list_t *urls, /* find a valid cache dir to download to */ cachedir = _alpm_filecache_setup(handle); - temporary_cachedir = _alpm_temporary_download_dir_setup(cachedir, handle->sandboxuser); + temporary_cachedir = _alpm_temporary_download_dir_setup(handle, cachedir); ASSERT(temporary_cachedir != NULL, return -1); for(i = urls; i; i = i->next) { diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 13cf1cef..1dbd4999 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -780,7 +780,7 @@ static int download_files(alpm_handle_t *handle) alpm_list_t *payloads = NULL; cachedir = _alpm_filecache_setup(handle); - temporary_cachedir = _alpm_temporary_download_dir_setup(cachedir, handle->sandboxuser); + temporary_cachedir = _alpm_temporary_download_dir_setup(handle, cachedir); if(temporary_cachedir == NULL) { ret = -1; goto finish; diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 00cc3734..8577eb42 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -949,12 +949,13 @@ const char *_alpm_filecache_setup(alpm_handle_t *handle) /** Create a temporary directory under the supplied directory. * The new directory is writable by the download user, and will be * removed after the download operation has completed. + * @param handle an alpm handle * @param dir existing sync or cache directory - * @param user download user name * @return pointer to a sub-directory writable by the download user inside the existing directory. */ -char *_alpm_temporary_download_dir_setup(const char *dir, const char *user) +char *_alpm_temporary_download_dir_setup(alpm_handle_t *handle, const char *dir) { + const char *user = handle->sandboxuser; uid_t myuid = getuid(); struct passwd const *pw = NULL; diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 5090c545..f58704fb 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -139,7 +139,7 @@ char *_alpm_filecache_find(alpm_handle_t *handle, const char *filename); /* Checks whether a file exists in cache */ int _alpm_filecache_exists(alpm_handle_t *handle, const char *filename); const char *_alpm_filecache_setup(alpm_handle_t *handle); -char *_alpm_temporary_download_dir_setup(const char *dir, const char *user); +char *_alpm_temporary_download_dir_setup(alpm_handle_t *handle, const char *dir); void _alpm_remove_temporary_download_dir(const char *dir); /* Unlike many uses of alpm_pkgvalidation_t, _alpm_test_checksum expects From a1ff7d8c72a1a659a4624d768930ad803c83aac5 Mon Sep 17 00:00:00 2001 From: morganamilo Date: Tue, 20 May 2025 13:20:45 +0100 Subject: [PATCH 3/4] libalpm: set errno and log when setting up temp download dir Before: % pacman -Syy :: Synchronising package databases... error: failed to synchronize all databases (unexpected error) After: % pacman -Syy :: Synchronising package databases... error: failed to create temporary download directory /var/lib/pacman/sync/download-P77oXs: Permission denied error: failed to synchronize all databases (failed to retrieve some files) --- lib/libalpm/util.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 8577eb42..058bb77f 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -23,6 +23,7 @@ */ #include +#include #include #include #include @@ -959,24 +960,43 @@ char *_alpm_temporary_download_dir_setup(alpm_handle_t *handle, const char *dir) uid_t myuid = getuid(); struct passwd const *pw = NULL; - ASSERT(dir != NULL, return NULL); + ASSERT(dir != NULL, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, NULL)); if(myuid == 0 && user != NULL) { - ASSERT((pw = getpwnam(user)) != NULL, return NULL); + errno = 0; + pw = getpwnam(user); + + if(pw == NULL) { + if(errno == 0) { + _alpm_log(handle, ALPM_LOG_ERROR, + _("download user '%s' does not exist\n"), user); + } else { + _alpm_log(handle, ALPM_LOG_ERROR, + _("failed to get download user '%s': %s\n"), + user, strerror(errno)); + } + RET_ERR(handle, ALPM_ERR_RETRIEVE, NULL); + } } const char template[] = "download-XXXXXX"; size_t newdirlen = strlen(dir) + sizeof(template) + 1; char *newdir = NULL; - MALLOC(newdir, newdirlen, return NULL); + MALLOC(newdir, newdirlen, RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); snprintf(newdir, newdirlen - 1, "%s%s", dir, template); if(mkdtemp(newdir) == NULL) { + _alpm_log(handle, ALPM_LOG_ERROR, + _("failed to create temporary download directory %s: %s\n"), + newdir, strerror(errno)); free(newdir); - return NULL; + RET_ERR(handle, ALPM_ERR_RETRIEVE, NULL); } if(pw != NULL) { if(chown(newdir, pw->pw_uid, pw->pw_gid) == -1) { + _alpm_log(handle, ALPM_LOG_ERROR, + _("failed to chown temporary download directory %s: %s\n"), + newdir, strerror(errno)); free(newdir); - return NULL; + RET_ERR(handle, ALPM_ERR_RETRIEVE, NULL); } } newdir[newdirlen-2] = '/'; From 3612510ee0b4d67a05f7bbdbfa9bf90d4f0e72bd Mon Sep 17 00:00:00 2001 From: morganamilo Date: Tue, 20 May 2025 13:45:02 +0100 Subject: [PATCH 4/4] libalpm: add new errno for download initialization The error string "failed to retrieve some files" implies that the download may have begun and some files may have been partially downloaded. If we know no download actually took place we can be more clear about this. --- lib/libalpm/alpm.h | 2 ++ lib/libalpm/error.c | 2 ++ lib/libalpm/util.c | 6 +++--- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 96e5e643..f72d61a9 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -308,6 +308,8 @@ typedef enum _alpm_errno_t { /** Files conflict */ ALPM_ERR_FILE_CONFLICTS, /* Misc */ + /** Download setup failed */ + ALPM_ERR_RETRIEVE_PREPARE, /** Download failed */ ALPM_ERR_RETRIEVE, /** Invalid Regex */ diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c index 58842eac..ca97d884 100644 --- a/lib/libalpm/error.c +++ b/lib/libalpm/error.c @@ -138,6 +138,8 @@ const char SYMEXPORT *alpm_strerror(alpm_errno_t err) case ALPM_ERR_FILE_CONFLICTS: return _("conflicting files"); /* Miscellaneous */ + case ALPM_ERR_RETRIEVE_PREPARE: + return _("failed to initialize download"); case ALPM_ERR_RETRIEVE: return _("failed to retrieve some files"); case ALPM_ERR_INVALID_REGEX: diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 058bb77f..6d486d4c 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -974,7 +974,7 @@ char *_alpm_temporary_download_dir_setup(alpm_handle_t *handle, const char *dir) _("failed to get download user '%s': %s\n"), user, strerror(errno)); } - RET_ERR(handle, ALPM_ERR_RETRIEVE, NULL); + RET_ERR(handle, ALPM_ERR_RETRIEVE_PREPARE, NULL); } } @@ -988,7 +988,7 @@ char *_alpm_temporary_download_dir_setup(alpm_handle_t *handle, const char *dir) _("failed to create temporary download directory %s: %s\n"), newdir, strerror(errno)); free(newdir); - RET_ERR(handle, ALPM_ERR_RETRIEVE, NULL); + RET_ERR(handle, ALPM_ERR_RETRIEVE_PREPARE, NULL); } if(pw != NULL) { if(chown(newdir, pw->pw_uid, pw->pw_gid) == -1) { @@ -996,7 +996,7 @@ char *_alpm_temporary_download_dir_setup(alpm_handle_t *handle, const char *dir) _("failed to chown temporary download directory %s: %s\n"), newdir, strerror(errno)); free(newdir); - RET_ERR(handle, ALPM_ERR_RETRIEVE, NULL); + RET_ERR(handle, ALPM_ERR_RETRIEVE_PREPARE, NULL); } } newdir[newdirlen-2] = '/';