Tobias Stoeckmann wrote:
- size = readlinkat (parentfd, name, buffer, linklen + 1);
+ size = readlinkat (parentfd, name, buffer, linklen);
Thanks for the bug report and patch. Although that patch fixes the immediate
problem, it would still have the bug that a race could cause 'tar' to create a
tar image of a symlink that never existed. Also, some file systems report an
incorrect st_size on symlinks so it's not wise to trust st_size.
I noticed some other problems while fixing this, and installed the attached set
of patches to fix what I found. The last patch should fix the bug you reported,
along with the untrustworthy st_size problem.
>From 3be1c38f48700ef5884d5d9c35fede85e2f445c3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Mon, 13 Jul 2015 09:29:51 -0700
Subject: [PATCH 1/4] tar: pacify GCC 5.1 -Wformat-signedness
* lib/wordsplit.c (struct wordsplit_node.flags):
Now unsigned, so that 'printf ("%x", p->flags)' doesn't provoke GCC.
* src/incremen.c (read_num, dumpdir_ok):
Don't printf an int with %x or %o.
---
lib/wordsplit.c | 2 +-
src/incremen.c | 18 +++++++++++-------
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/lib/wordsplit.c b/lib/wordsplit.c
index bda64d3..69db2e0 100644
--- a/lib/wordsplit.c
+++ b/lib/wordsplit.c
@@ -221,7 +221,7 @@ struct wordsplit_node
{
struct wordsplit_node *prev; /* Previous element */
struct wordsplit_node *next; /* Next element */
- int flags; /* Node flags */
+ unsigned flags; /* Node flags */
union
{
struct
diff --git a/src/incremen.c b/src/incremen.c
index b1b70ba..e549bbd 100644
--- a/src/incremen.c
+++ b/src/incremen.c
@@ -735,7 +735,7 @@ scan_directory (struct tar_stat_info *st)
savedir_error (dir);
info_attach_exclist (st);
-
+
tmp = xstrdup (dir);
zap_slashes (tmp);
@@ -1155,11 +1155,14 @@ read_num (FILE *fp, char const *fieldname,
}
if (c)
- FATAL_ERROR ((0, 0,
- _("%s: byte %s: %s %s followed by invalid byte 0x%02x"),
- quotearg_colon (listed_incremental_option),
- offtostr (ftello (fp), offbuf),
- fieldname, buf, c));
+ {
+ unsigned uc = c;
+ FATAL_ERROR ((0, 0,
+ _("%s: byte %s: %s %s followed by invalid byte 0x%02x"),
+ quotearg_colon (listed_incremental_option),
+ offtostr (ftello (fp), offbuf),
+ fieldname, buf, uc));
+ }
*pval = strtosysint (buf, NULL, min_val, max_val);
conversion_errno = errno;
@@ -1541,9 +1544,10 @@ dumpdir_ok (char *dumpdir)
{
if (expect && *p != expect)
{
+ unsigned char uc = *p;
ERROR ((0, 0,
_("Malformed dumpdir: expected '%c' but found %#3o"),
- expect, *p));
+ expect, uc));
return false;
}
switch (*p)
--
2.1.0
>From d1cfcd6f70ab54fa291b57fad949e5f872bfdc36 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Mon, 13 Jul 2015 09:32:46 -0700
Subject: [PATCH 2/4] tar: port to recent gnulib
* gnulib.modules: Remove 'acl' and add 'file-has-acl'.
---
gnulib.modules | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gnulib.modules b/gnulib.modules
index 0d2e76c..ec3dc90 100644
--- a/gnulib.modules
+++ b/gnulib.modules
@@ -18,7 +18,6 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-acl
alloca
argmatch
argp
@@ -37,6 +36,7 @@ fchownat
fcntl-h
fdopendir
fdutimensat
+file-has-acl
fileblocks
fnmatch-gnu
fprintftime
--
2.1.0
>From 88300c41f06a7f0e9cef5d1d5c0654e5ccf5fa22 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Mon, 13 Jul 2015 09:46:17 -0700
Subject: [PATCH 3/4] tar: port -d to longer symlinks
* src/compare.c (diff_symlink):
Don't use alloca on symlink length; it might be too big for the stack.
Don't assume that readlinkat's return value fits in 'int'.
Prefer memcmp to strncmp where either will do.
---
src/compare.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/compare.c b/src/compare.c
index d29cfdd..7ed4558 100644
--- a/src/compare.c
+++ b/src/compare.c
@@ -270,11 +270,12 @@ diff_link (void)
static void
diff_symlink (void)
{
+ char buf[1024];
size_t len = strlen (current_stat_info.link_name);
- char *linkbuf = alloca (len + 1);
+ char *linkbuf = len < sizeof buf ? buf : xmalloc (len + 1);
- int status = readlinkat (chdir_fd, current_stat_info.file_name,
- linkbuf, len + 1);
+ ssize_t status = readlinkat (chdir_fd, current_stat_info.file_name,
+ linkbuf, len + 1);
if (status < 0)
{
@@ -285,8 +286,11 @@ diff_symlink (void)
report_difference (¤t_stat_info, NULL);
}
else if (status != len
- || strncmp (current_stat_info.link_name, linkbuf, len) != 0)
+ || memcmp (current_stat_info.link_name, linkbuf, len) != 0)
report_difference (¤t_stat_info, _("Symlink differs"));
+
+ if (linkbuf != buf)
+ free (linkbuf);
}
#endif
--
2.1.0
>From 1fd3cbdebddcf7f31c92b6b967b955a41737c71e Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Mon, 13 Jul 2015 09:53:00 -0700
Subject: [PATCH 4/4] tar: fix symlink race and symlink transform bug
Problem reported by Tobias Stoeckmann in:
http://lists.gnu.org/archive/html/bug-tar/2015-07/msg00004.html
* gnulib.modules: Add areadlinkat-with-size.
* src/create.c: Include areadlink.h.
(dump_file0): Use areadlinkat_with_size, rather than trying to do
it by hand, incorrectly. This also avoids assumption that
the symlink contents fit on the stack. Also, use the transformed
link name, not the original link name, when deciding whether the
name is long enough to require writing a long link.
---
gnulib.modules | 1 +
src/create.c | 22 +++++++++-------------
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/gnulib.modules b/gnulib.modules
index ec3dc90..fe5ab73 100644
--- a/gnulib.modules
+++ b/gnulib.modules
@@ -19,6 +19,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
alloca
+areadlinkat-with-size
argmatch
argp
argp-version-etc
diff --git a/src/create.c b/src/create.c
index 1b08e0b..7cdc978 100644
--- a/src/create.c
+++ b/src/create.c
@@ -22,6 +22,7 @@
#include <system.h>
+#include <areadlink.h>
#include <quotearg.h>
#include "common.h"
@@ -1114,7 +1115,7 @@ dump_dir0 (struct tar_stat_info *st, char const *directory)
return;
info_attach_exclist (st);
-
+
if (incremental_option && archive_format != POSIX_FORMAT)
blk->header.typeflag = GNUTYPE_DUMPDIR;
else /* if (standard_option) */
@@ -1198,7 +1199,7 @@ dump_dir0 (struct tar_stat_info *st, char const *directory)
char const *entry;
size_t entry_len;
size_t name_len;
-
+
name_buf = xstrdup (st->orig_file_name);
name_size = name_len = strlen (name_buf);
@@ -1837,22 +1838,17 @@ dump_file0 (struct tar_stat_info *st, char const *name, char const *p)
#ifdef HAVE_READLINK
else if (S_ISLNK (st->stat.st_mode))
{
- char *buffer;
- int size;
- size_t linklen = st->stat.st_size;
- if (linklen != st->stat.st_size || linklen + 1 == 0)
- xalloc_die ();
- buffer = (char *) alloca (linklen + 1);
- size = readlinkat (parentfd, name, buffer, linklen + 1);
- if (size < 0)
+ st->link_name = areadlinkat_with_size (parentfd, name, st->stat.st_size);
+ if (!st->link_name)
{
+ if (errno == ENOMEM)
+ xalloc_die ();
file_removed_diag (p, top_level, readlink_diag);
return;
}
- buffer[size] = '\0';
- assign_string (&st->link_name, buffer);
transform_name (&st->link_name, XFORM_SYMLINK);
- if (NAME_FIELD_SIZE - (archive_format == OLDGNU_FORMAT) < size)
+ if (NAME_FIELD_SIZE - (archive_format == OLDGNU_FORMAT)
+ < strlen (st->link_name))
write_long_link (st);
xattrs_selinux_get (parentfd, name, st, 0);
--
2.1.0
