[pacman-dev] [PATCH] libalpm: Check cachedir access before re-creating

[Andrew Gregory]

On 01/23/19 at 10:52pm, David Phillips wrote:
> If users have mounted a filesystem such as sshfs or NFS that doesn't
> allow root access by default for security reasons, _alpm_filecache_setup
> will blindly try and re-create the components of the cachedir path,
> eating EACCES (to be liek mkdir -p). Then, it will return this path as
> if it is valid when it may not be readable by root, causing
> alpm_check_downloadspace to trigger a FPE since it assumes the cachedir
> is R/W and uses fsinfo which may be zero.
> 
> This patch adds a check before trying to re-create the cachedir in order
> to determine if creating it will result in EACCES, and outputs a warning
> if so. It also discards the current cachedir from being considered. This
> causes a more meaningful and graceful failure than a FPE.
> ---
>  lib/libalpm/util.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index d33eef2a..d4375593 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -849,12 +849,16 @@ const char *_alpm_filecache_setup(alpm_handle_t *handle)
>  	for(i = handle->cachedirs; i; i = i->next) {
>  		cachedir = i->data;
>  		if(stat(cachedir, &buf) != 0) {
> -			/* cache directory does not exist.... try creating it */
> -			_alpm_log(handle, ALPM_LOG_WARNING, _("no %s cache exists, creating...\n"),
> -					cachedir);
> -			if(_alpm_makepath(cachedir) == 0) {
> -				_alpm_log(handle, ALPM_LOG_DEBUG, "using cachedir: %s\n", cachedir);
> -				return cachedir;
> +			if (errno == EACCES) {
> +				_alpm_log(handle, ALPM_LOG_WARNING, _("failed to stat %s: %s\n"), cachedir, strerror(errno));
> +			} else {
> +				/* cache directory does not exist.... try creating it */
> +				_alpm_log(handle, ALPM_LOG_WARNING, _("no %s cache exists, creating...\n"),
> +						cachedir);
> +				if(_alpm_makepath(cachedir) == 0) {
> +					_alpm_log(handle, ALPM_LOG_DEBUG, "using cachedir: %s\n", cachedir);
> +					return cachedir;
> +				}

It's been quite a while since I used sshfs or NFS, so maybe I'm
missing something, but this doesn't make a lot of sense to me.  Does
makepath really succeed if we don't have access rights?  I would have
thought that would already fail, causing the next directory to be
tried.  Second, why single out EACCESS as the exception?  Of all the
reasons stat can fail, ENOENT is really the only one that makes sense
for us to try to create the directory.  Where is the FPE actually
occurring?  That should be fixed directly, because it could still be
triggered if our access rights change between selecting the cachedir
and actually using it.

apg