diff mbox

[pacman-dev] libalpm: Use archive_read_extract2

Message ID 20170306201520.18514-1-krejzi@email.com
State Accepted, archived
Delegated to: Andrew Gregory
Headers show

Commit Message

Armin K March 6, 2017, 8:15 p.m. UTC
archive_read_extract() forces resolution of uid/gid to names
when extracting the tarball. This can lead to wrong file
ownership when using pacman with -r option and when uid/gid
differ in the host and in the chroot.

archive_read_extract2() uses uid's and gid's only. See also:

https://lists.archlinux.org/pipermail/pacman-dev/2017-March/021912.html

Signed-off-by: Armin K <krejzi@email.com>
---
 lib/libalpm/add.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Allan McRae April 4, 2017, 2 a.m. UTC | #1
On 07/03/17 06:15, Armin K wrote:
> archive_read_extract() forces resolution of uid/gid to names
> when extracting the tarball. This can lead to wrong file
> ownership when using pacman with -r option and when uid/gid
> differ in the host and in the chroot.
> 
> archive_read_extract2() uses uid's and gid's only. See also:
> 
> https://lists.archlinux.org/pipermail/pacman-dev/2017-March/021912.html
> 
> Signed-off-by: Armin K <krejzi@email.com>
> ---

@Andrew:  Is this patch OK to go in until you finish your work on
group/user handling?

A


>  lib/libalpm/add.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index c5bed817..0beed01c 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -110,6 +110,7 @@ static int perform_extraction(alpm_handle_t *handle, struct archive *archive,
>  		struct archive_entry *entry, const char *filename)
>  {
>  	int ret;
> +	struct archive *archive_writer;
>  	const int archive_flags = ARCHIVE_EXTRACT_OWNER |
>  	                          ARCHIVE_EXTRACT_PERM |
>  	                          ARCHIVE_EXTRACT_TIME |
> @@ -118,7 +119,20 @@ static int perform_extraction(alpm_handle_t *handle, struct archive *archive,
>  
>  	archive_entry_set_pathname(entry, filename);
>  
> -	ret = archive_read_extract(archive, entry, archive_flags);
> +	archive_writer = archive_write_disk_new();
> +	if (archive_writer == NULL) {
> +		_alpm_log(handle, ALPM_LOG_ERROR, _("cannot allocate disk archive object"));
> +		alpm_logaction(handle, ALPM_CALLER_PREFIX,
> +				"error: cannot allocate disk archive object");
> +		return 1;
> +	}
> +
> +	archive_write_disk_set_options(archive_writer, archive_flags);
> +
> +	ret = archive_read_extract2(archive, entry, archive_writer);
> +
> +	archive_write_free(archive_writer);
> +
>  	if(ret == ARCHIVE_WARN && archive_errno(archive) != ENOSPC) {
>  		/* operation succeeded but a "non-critical" error was encountered */
>  		_alpm_log(handle, ALPM_LOG_WARNING, _("warning given when extracting %s (%s)\n"),
>
Andrew Gregory April 9, 2017, 11:55 p.m. UTC | #2
On 04/04/17 at 12:00pm, Allan McRae wrote:
> On 07/03/17 06:15, Armin K wrote:
> > archive_read_extract() forces resolution of uid/gid to names
> > when extracting the tarball. This can lead to wrong file
> > ownership when using pacman with -r option and when uid/gid
> > differ in the host and in the chroot.
> > 
> > archive_read_extract2() uses uid's and gid's only. See also:
> > 
> > https://lists.archlinux.org/pipermail/pacman-dev/2017-March/021912.html
> > 
> > Signed-off-by: Armin K <krejzi@email.com>
> > ---
> 
> @Andrew:  Is this patch OK to go in until you finish your work on
> group/user handling?

ACK.  This would need to be done for the group/user handling work
anyway.

apg
diff mbox

Patch

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index c5bed817..0beed01c 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -110,6 +110,7 @@  static int perform_extraction(alpm_handle_t *handle, struct archive *archive,
 		struct archive_entry *entry, const char *filename)
 {
 	int ret;
+	struct archive *archive_writer;
 	const int archive_flags = ARCHIVE_EXTRACT_OWNER |
 	                          ARCHIVE_EXTRACT_PERM |
 	                          ARCHIVE_EXTRACT_TIME |
@@ -118,7 +119,20 @@  static int perform_extraction(alpm_handle_t *handle, struct archive *archive,
 
 	archive_entry_set_pathname(entry, filename);
 
-	ret = archive_read_extract(archive, entry, archive_flags);
+	archive_writer = archive_write_disk_new();
+	if (archive_writer == NULL) {
+		_alpm_log(handle, ALPM_LOG_ERROR, _("cannot allocate disk archive object"));
+		alpm_logaction(handle, ALPM_CALLER_PREFIX,
+				"error: cannot allocate disk archive object");
+		return 1;
+	}
+
+	archive_write_disk_set_options(archive_writer, archive_flags);
+
+	ret = archive_read_extract2(archive, entry, archive_writer);
+
+	archive_write_free(archive_writer);
+
 	if(ret == ARCHIVE_WARN && archive_errno(archive) != ENOSPC) {
 		/* operation succeeded but a "non-critical" error was encountered */
 		_alpm_log(handle, ALPM_LOG_WARNING, _("warning given when extracting %s (%s)\n"),