Date: Tue, 21 Sep 2010 17:28:32 -0400 From: "Stephane E. Potvin" <sepotvin@FreeBSD.org> To: Tim Kientzle <kientzle@freebsd.org> Cc: freebsd-hackers@freebsd.org, Benjamin Kaduk <kaduk@MIT.EDU>, Jilles Tjoelker <jilles@stack.nl>, kaiw@freebsd.org Subject: Re: ar(1) format_decimal failure is fatal? Message-ID: <4C992380.3040700@FreeBSD.org> In-Reply-To: <F56D9CB9-E644-4279-8830-71292C880D9B@freebsd.org> References: <alpine.GSO.1.10.1008281833470.9337@multics.mit.edu> <20100829201050.GA60715@stack.nl> <alpine.GSO.1.10.1009032036310.9337@multics.mit.edu> <F56D9CB9-E644-4279-8830-71292C880D9B@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--Boundary_(ID_aBuVVhu/JWJqoEVI3WdyTA)
Content-type: text/plain; charset=ISO-8859-1
Content-transfer-encoding: 7BIT
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 09/18/10 03:24, Tim Kientzle wrote:
>
> On Sep 17, 2010, at 9:01 PM, Benjamin Kaduk wrote:
>
>> On Sun, 29 Aug 2010, Jilles Tjoelker wrote:
>>
>>> On Sat, Aug 28, 2010 at 07:08:34PM -0400, Benjamin Kaduk wrote:
>>>> [...]
>>>> building static egacy library
>>>> ar: fatal: Numeric user ID too large
>>>> *** Error code 70
>>>
>>>> This error appears to be coming from
>>>> lib/libarchive/archive_write_set_format_ar.c , which seems to only have
>>>> provisions for outputting a user ID in AR_uid_size = 6 columns.
>> [...]
>>>> It looks like this macro was so defined in version 1.1 of that file, with
>>>> commit message "'ar' format support for libarchive, contributed by Kai
>>>> Wang.". This doesn't make it terribly clear whether the 'ar' format
>>>> mandates this length, or if it is an implementation decision...
>
> There's no official standard for the ar format, only old
> conventions and compatibility with legacy implementations.
>
>>> I wonder if the uid/gid fields are useful at all for ar archives. Ar
>>> archives are usually not extracted, and when they are, the current
>>> user's values seem good enough. The uid/gid also prevent exactly
>>> reproducible builds (together with the timestamp).
>>
>> GNU binutils has recently (well, March 2009) added a -D ("deterministic") argument to ar(1) which sets the timestamp, uid, and gid to zero, and the mode to 644. If that argument is not given, linux's ar(1) happily uses my 8-digit uid as-is; the manual page seems to imply that it will handle 15 or 16 digits in that field.
>
> Please send me a small example file... I don't think I've seen
> this format variant. Maybe we can extend our ar(1) to support
> this variant.
>
> Personally, I wonder if it wouldn't make sense to just always
> force the timestamp, uid, and gid to zero. I find it hard
> to believe anyone is using ar(1) as a general-purpose archiving
> tool. Of course, it should be trivial to add -D support to our ar(1).
>
>> I propose that format_{decimal,octal}() return ARCHIVE_FAILED for negative input, and ARCHIVE_WARN for overflow. archive_write_ar_header() can then catch ARCHIVE_WARN from the format_foo functions and continue on, propagating the ARCHIVE_WARN return value at the end of its execution ...
>
> This sounds entirely reasonable to me. I personally don't see much
> advantage to distinguishing negative versus overflow, but certainly
> have no objections to that part. Definitely ar(1) should not abort on
> a simple ARCHIVE_WARN.
>
>> Would (one of) you be willing to review a patch to that effect?
>
> Happy to do so.
>
Hi,
I've been using the attached patch for quite some time now. It basically
replace the offending gid/uid with nobody's id when necessary.
If I remember correctly, Tim was supposed to add them to the upstream version of
libarchive and then import them back in fbsd. Tim, do you remember what
happened with those?
Regards,
Steph
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.16 (FreeBSD)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkyZI38ACgkQmdOXtTCX/nt2WwCgqvd4GIyE5zRvL5kkHCWTGoAA
yA0AoJ/8Dx2QrLXAJHkOrd1YqW+QR03h
=KxCW
-----END PGP SIGNATURE-----
--Boundary_(ID_aBuVVhu/JWJqoEVI3WdyTA)
Content-type: text/plain; CHARSET=US-ASCII; name=libarchive_bigids.diff
Content-transfer-encoding: 7BIT
Content-disposition: attachment; filename=libarchive_bigids.diff
Index: usr.bin/tar/write.c
===================================================================
--- usr.bin/tar/write.c (revision 212556)
+++ usr.bin/tar/write.c (working copy)
@@ -439,7 +439,30 @@
{
const char *arg;
struct archive_entry *entry, *sparse_entry;
+ struct passwd nobody_pw, *nobody_ppw;
+ struct group nobody_gr, *nobody_pgr;
+ char id_buffer[128];
+ /*
+ * Some formats (like ustar) have a limit on the size of the uids/gids
+ * supported. Tell libarchive to use the uid/gid of nobody in this case
+ * instead of failing.
+ */
+ getpwnam_r("nobody", &nobody_pw, id_buffer, sizeof (id_buffer),
+ &nobody_ppw);
+ if (nobody_ppw)
+ archive_write_set_nobody_uid(a, nobody_ppw->pw_uid);
+ else
+ bsdtar_warnc(0,
+ "nobody's uid not found, large uids won't be supported.");
+ getgrnam_r("nobody", &nobody_gr, id_buffer, sizeof (id_buffer),
+ &nobody_pgr);
+ if (nobody_pgr)
+ archive_write_set_nobody_gid(a, nobody_pgr->gr_gid);
+ else
+ bsdtar_warnc(0,
+ "nobody's gid not found, large gids won't be supported.");
+
/* Allocate a buffer for file data. */
if ((bsdtar->buff = malloc(FILEDATABUFLEN)) == NULL)
bsdtar_errc(1, 0, "cannot allocate memory");
Index: usr.bin/tar/bsdtar.1
===================================================================
--- usr.bin/tar/bsdtar.1 (revision 212556)
+++ usr.bin/tar/bsdtar.1 (working copy)
@@ -1027,3 +1027,6 @@
convention can cause hard link information to be lost.
(This is a consequence of the incompatible ways that different archive
formats store hardlink information.)
+.Pp
+Owner and group of the files added to the archive will be replaced by
+"nobody" if they are larger than 6 digits and the ustar format is used.
Index: usr.bin/ar/ar.1
===================================================================
--- usr.bin/ar/ar.1 (revision 212556)
+++ usr.bin/ar/ar.1 (working copy)
@@ -402,3 +402,6 @@
.Lb libarchive
and the
.Lb libelf .
+.Sh BUGS
+Owner and group of the files added to the archive will be replaced
+by "nobody" if they are larger than 6 digits.
Index: usr.bin/ar/write.c
===================================================================
--- usr.bin/ar/write.c (revision 212556)
+++ usr.bin/ar/write.c (working copy)
@@ -41,6 +41,8 @@
#include <stdlib.h>
#include <string.h>
#include <sysexits.h>
+#include <pwd.h>
+#include <grp.h>
#include "ar.h"
@@ -554,6 +556,9 @@
size_t s_sz; /* size of archive symbol table. */
size_t pm_sz; /* size of pseudo members */
int i, nr;
+ struct passwd nobody_pw, *nobody_ppw;
+ struct group nobody_gr, *nobody_pgr;
+ char id_buffer[128];
if (elf_version(EV_CURRENT) == EV_NONE)
bsdar_errc(bsdar, EX_SOFTWARE, 0,
@@ -610,6 +615,27 @@
archive_write_set_format_ar_svr4(a);
archive_write_set_compression_none(a);
+ /*
+ * The archive format doesn't support ids larger than 6 char.
+ * Try to tell libarchive to use uid/gid of nobody in case the uid/gid
+ * of the file being added is too large.
+ */
+ getpwnam_r("nobody", &nobody_pw, id_buffer, sizeof (id_buffer),
+ &nobody_ppw);
+ if (nobody_ppw)
+ archive_write_set_nobody_uid(a, nobody_ppw->pw_uid);
+ else
+ bsdar_warnc(bsdar, 0,
+ "nobody's uid not found, large uids won't be supported.");
+
+ getgrnam_r("nobody", &nobody_gr, id_buffer, sizeof (id_buffer),
+ &nobody_pgr);
+ if (nobody_pgr)
+ archive_write_set_nobody_gid(a, nobody_pgr->gr_gid);
+ else
+ bsdar_warnc(bsdar, 0,
+ "nobody's gid not found, large gids won't be supported.");
+
AC(archive_write_open_filename(a, bsdar->filename));
/*
Index: lib/libarchive/archive_write.c
===================================================================
--- lib/libarchive/archive_write.c (revision 212556)
+++ lib/libarchive/archive_write.c (working copy)
@@ -114,6 +114,11 @@
}
memset(nulls, 0, a->null_length);
a->nulls = nulls;
+
+ /* Initialize the nobody ids */
+ a->nobody_uid = -1;
+ a->nobody_gid = -1;
+
/*
* Set default compression, but don't set a default format.
* Were we to set a default format here, we would force every
@@ -284,8 +289,59 @@
return (a->bytes_in_last_block);
}
+/*
+ * Set the uid to use when the uid is too large to fit into the archive.
+ * Usually set to 'nobody'
+ */
+int
+archive_write_set_nobody_uid(struct archive *_a, id_t uid)
+{
+ struct archive_write *a = (struct archive_write *)_a;
+ __archive_check_magic(&a->archive, ARCHIVE_WRITE_MAGIC,
+ ARCHIVE_STATE_ANY, "archive_write_set_nobody_uid");
+ a->nobody_uid = uid;
+ return (ARCHIVE_OK);
+}
/*
+ * Return the value set above. -1 indicates it has not been set.
+ */
+id_t
+archive_write_get_nobody_uid(struct archive *_a)
+{
+ struct archive_write *a = (struct archive_write *)_a;
+ __archive_check_magic(&a->archive, ARCHIVE_WRITE_MAGIC,
+ ARCHIVE_STATE_ANY, "archive_write_get_nobody_uid");
+ return (a->nobody_uid);
+}
+
+/*
+ * Set the gid to use when the gid is too large to fit into the archive.
+ * Usually set to 'nobody'
+ */
+int
+archive_write_set_nobody_gid(struct archive *_a, id_t gid)
+{
+ struct archive_write *a = (struct archive_write *)_a;
+ __archive_check_magic(&a->archive, ARCHIVE_WRITE_MAGIC,
+ ARCHIVE_STATE_ANY, "archive_write_set_nobody_gid");
+ a->nobody_gid = gid;
+ return (ARCHIVE_OK);
+}
+
+/*
+ * Return the value set avobe. -1 indicates it has not been set.
+ */
+id_t
+archive_write_get_nobody_gid(struct archive *_a)
+{
+ struct archive_write *a = (struct archive_write *)_a;
+ __archive_check_magic(&a->archive, ARCHIVE_WRITE_MAGIC,
+ ARCHIVE_STATE_ANY, "archive_write_get_nobody_gid");
+ return (a->nobody_gid);
+}
+
+/*
* dev/ino of a file to be rejected. Used to prevent adding
* an archive to itself recursively.
*/
Index: lib/libarchive/archive_write_set_format_ustar.c
===================================================================
--- lib/libarchive/archive_write_set_format_ustar.c (revision 212556)
+++ lib/libarchive/archive_write_set_format_ustar.c (working copy)
@@ -365,13 +365,17 @@
}
if (format_number(archive_entry_uid(entry), h + USTAR_uid_offset, USTAR_uid_size, USTAR_uid_max_size, strict)) {
- archive_set_error(&a->archive, ERANGE, "Numeric user ID too large");
- ret = ARCHIVE_FAILED;
+ if (a->nobody_uid == -1 || format_number(a->nobody_uid, h + USTAR_uid_offset, USTAR_uid_size, USTAR_uid_max_size, strict)) {
+ archive_set_error(&a->archive, ERANGE, "Numeric user ID too large");
+ ret = ARCHIVE_FAILED;
+ }
}
if (format_number(archive_entry_gid(entry), h + USTAR_gid_offset, USTAR_gid_size, USTAR_gid_max_size, strict)) {
- archive_set_error(&a->archive, ERANGE, "Numeric group ID too large");
- ret = ARCHIVE_FAILED;
+ if (a->nobody_uid == -1 || format_number(a->nobody_gid, h + USTAR_gid_offset, USTAR_gid_size, USTAR_gid_max_size, strict)) {
+ archive_set_error(&a->archive, ERANGE, "Numeric group ID too large");
+ ret = ARCHIVE_FAILED;
+ }
}
if (format_number(archive_entry_size(entry), h + USTAR_size_offset, USTAR_size_size, USTAR_size_max_size, strict)) {
Index: lib/libarchive/archive.h
===================================================================
--- lib/libarchive/archive.h (revision 212556)
+++ lib/libarchive/archive.h (working copy)
@@ -514,6 +514,15 @@
int bytes_in_last_block);
__LA_DECL int archive_write_get_bytes_in_last_block(struct archive *);
+/* The uid/gid to use when the uid/gid of the file that is to be archived
+ * is too large to be expressed in the archive format selected. */
+__LA_DECL int archive_write_set_nobody_uid(struct archive *,
+ id_t nobody_uid);
+__LA_DECL id_t archive_write_get_nobody_uid(struct archive *);
+__LA_DECL int archive_write_set_nobody_gid(struct archive *,
+ id_t nobody_gid);
+__LA_DECL id_t archive_write_get_nobody_gid(struct archive *);
+
/* The dev/ino of a file that won't be archived. This is used
* to avoid recursively adding an archive to itself. */
__LA_DECL int archive_write_set_skip_file(struct archive *, dev_t, ino_t);
Index: lib/libarchive/archive_write_private.h
===================================================================
--- lib/libarchive/archive_write_private.h (revision 212556)
+++ lib/libarchive/archive_write_private.h (working copy)
@@ -103,6 +103,13 @@
struct archive_entry *);
ssize_t (*format_write_data)(struct archive_write *,
const void *buff, size_t);
+
+ /*
+ * Uid/Gid that should be used when the file uid/gid is too large
+ * to be adequately expressed in the archive format (usually nobody).
+ */
+ id_t nobody_uid;
+ id_t nobody_gid;
};
/*
Index: lib/libarchive/archive_write_set_format_ar.c
===================================================================
--- lib/libarchive/archive_write_set_format_ar.c (revision 212556)
+++ lib/libarchive/archive_write_set_format_ar.c (working copy)
@@ -299,14 +299,18 @@
return (ARCHIVE_WARN);
}
if (format_decimal(archive_entry_uid(entry), buff + AR_uid_offset, AR_uid_size)) {
- archive_set_error(&a->archive, ERANGE,
- "Numeric user ID too large");
- return (ARCHIVE_WARN);
+ if (a->nobody_uid == -1 || format_decimal(a->nobody_uid, buff + AR_uid_offset, AR_uid_size)) {
+ archive_set_error(&a->archive, ERANGE,
+ "Numeric user ID too large");
+ return (ARCHIVE_WARN);
+ }
}
if (format_decimal(archive_entry_gid(entry), buff + AR_gid_offset, AR_gid_size)) {
- archive_set_error(&a->archive, ERANGE,
- "Numeric group ID too large");
- return (ARCHIVE_WARN);
+ if (a->nobody_gid == -1 || format_decimal(a->nobody_gid, buff + AR_gid_offset, AR_gid_size)) {
+ archive_set_error(&a->archive, ERANGE,
+ "Numeric group ID too large");
+ return (ARCHIVE_WARN);
+ }
}
if (format_octal(archive_entry_mode(entry), buff + AR_mode_offset, AR_mode_size)) {
archive_set_error(&a->archive, ERANGE,
Index: lib/libarchive/archive_write.3
===================================================================
--- lib/libarchive/archive_write.3 (revision 212556)
+++ lib/libarchive/archive_write.3 (working copy)
@@ -38,6 +38,10 @@
.Nm archive_write_get_bytes_per_block ,
.Nm archive_write_set_bytes_per_block ,
.Nm archive_write_set_bytes_in_last_block ,
+.Nm archive_write_set_nobody_uid ,
+.Nm archive_write_get_nobody_uid ,
+.Nm archive_write_set_nobody_gid ,
+.Nm archive_write_get_nobody_gid ,
.Nm archive_write_set_compression_bzip2 ,
.Nm archive_write_set_compression_compress ,
.Nm archive_write_set_compression_gzip ,
@@ -68,6 +72,14 @@
.Ft int
.Fn archive_write_set_bytes_in_last_block "struct archive *" "int"
.Ft int
+.Fn archive_write_set_nobody_uid "struct archive *" "id_t"
+.Ft id_t
+.Fn archive_write_get_nobody_uid "struct archive *"
+.Ft int
+.Fn archive_write_set_nobody_gid "struct archive *" "id_t"
+.Ft id_t
+.Fn archive_write_get_nobody_gid "struct archive *"
+.Ft int
.Fn archive_write_set_compression_bzip2 "struct archive *"
.Ft int
.Fn archive_write_set_compression_compress "struct archive *"
@@ -174,6 +186,19 @@
Retrieve the currently-set value for last block size.
A value of -1 here indicates that the library should use default values.
.It Xo
+.Fn archive_write_set_nobody_uid ,
+.Fn archive_write_set_nobody_gid
+.Xc
+The uid/gid to use when the uid/gid of the file that is to be written
+is too large for the underlying archive format.
+.It Xo
+.Fn archive_write_get_nobody_uid ,
+.Fn archive_write_get_nobody_gid
+.Xc
+Retrieve the currently-set value for the nobody uid/gid.
+A value of -1 here indicates that the library should use default
+behavior (usually failing with an error).
+.It Xo
.Fn archive_write_set_format_cpio ,
.Fn archive_write_set_format_pax ,
.Fn archive_write_set_format_pax_restricted ,
--Boundary_(ID_aBuVVhu/JWJqoEVI3WdyTA)
Content-type: application/octet-stream; name=libarchive_bigids.diff.sig
Content-transfer-encoding: base64
Content-disposition: attachment; filename=libarchive_bigids.diff.sig
iEYEABECAAYFAkyZI4AACgkQmdOXtTCX/nuawwCgiUl5FgTQKlfLNdWtFsl0PXrenoIAn1yK
rmwVn1kFg2Hp0/1/5EV7c8wJ
--Boundary_(ID_aBuVVhu/JWJqoEVI3WdyTA)--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C992380.3040700>