Package: dump
Version: 0.4b47-4+~tjw12r5
Severity: important
Dear Maintainer,
Many years ago I reported a bug with verifying attributes on files where
the count was sometimes wrong. You fixed this (Thanks!)
However, it turns out the same bug is present for directories too.
Unfortunately, the fix for files meant that directories *verified*
correctly but did not restore correctly.
The attached patch *replaces* the xattr_verify patch currently in the
package and includes fix for the directory attributes which now both
restore and verify successfully.
(I haven't actually tested failure cases for the verify yet - i.e.
adding, removing or modifying an attribute between backup and verify)
-- System Information:
Debian Release: 12.7
APT prefers stable-security
APT policy: (500, 'stable-security'), (500, 'stable')
Architecture: amd64 (x86_64)
Kernel: Linux 6.1.0-25-amd64 (SMP w/4 CPU threads; PREEMPT)
Kernel taint flags: TAINT_WARN
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: sysvinit (via /sbin/init)
Versions of packages dump depends on:
ii libblkid1 2.38.1-5+deb12u1
ii libbz2-1.0 1.0.8-5+b1
ii libc6 2.36-9+deb12u8
ii libcom-err2 1.47.0-2
ii libext2fs2 1.47.0-2
ii liblzo2-2 2.10-2
ii libreadline8 8.2-1.3
ii libselinux1 3.4-1+b6
ii tar 1.34+dfsg-1.2+deb12u1
ii zlib1g 1:1.2.13.dfsg-1
dump recommends no packages.
dump suggests no packages.
-- no debconf information
diff -urN dump-0.4b47.orig/restore/dirs.c dump-0.4b47/restore/dirs.c
--- dump-0.4b47.orig/restore/dirs.c 2022-05-03 10:02:27.000000000 +0000
+++ dump-0.4b47/restore/dirs.c 2022-05-03 10:02:27.000000000 +0000
@@ -181,7 +181,7 @@
struct inotab *itp;
struct direct nulldir;
int fd;
- char xattr[XATTR_MAXSIZE];
+ char xattr[XATTR_MAXSIZE*2] = {0};
int xattr_found = 0;
_Bool is_tmp = command != 'r' && command != 'R';
dump_ino_t ino;
@@ -248,7 +248,7 @@
skipfile();
break;
case EXT_XATTR:
- if (readxattr(xattr) == GOOD)
+ if (readxattr(xattr+xattr_found*XATTR_MAXSIZE)
== GOOD)
xattr_found = 1;
break;
}
@@ -667,14 +667,14 @@
}
clearerr(mf);
for (;;) {
- char xattr[XATTR_MAXSIZE];
+ char xattr[XATTR_MAXSIZE*2];
if (fread((char *)&node, sizeof(struct modeinfo), 1, mf) != 1) {
if (feof(mf))
break;
err(1, "fread");
}
if (node.xattr) {
- if (fread(xattr, XATTR_MAXSIZE, 1, mf) != 1) {
+ if (fread(xattr, XATTR_MAXSIZE*2, 1, mf) != 1) {
if (feof(mf))
break;
err(1, "fread");
@@ -701,8 +701,13 @@
warn("chown");
(void) chmod(cp, node.mode);
utimes(cp, node.timep);
- if (node.xattr)
+ if (node.xattr) {
xattr_extract(cp, xattr);
+ char* xattr2 = xattr + XATTR_MAXSIZE;
+ if (*(uint32_t*)xattr2 != 0) {
+ xattr_extract(cp, xattr2);
+ }
+ }
ep->e_flags &= ~NEW;
if (node.flags)
#ifdef __linux__
@@ -746,14 +751,14 @@
}
clearerr(mf);
for (;;) {
- char xattr[XATTR_MAXSIZE];
+ char xattr[XATTR_MAXSIZE*2];
if (fread((char *)&node, sizeof(struct modeinfo), 1, mf) != 1) {
if (feof(mf))
break;
err(1, "fread");
}
if (node.xattr) {
- if (fread(xattr, XATTR_MAXSIZE, 1, mf) != 1) {
+ if (fread(xattr, XATTR_MAXSIZE*2, 1, mf) != 1) {
if (feof(mf))
break;
err(1, "fread");
@@ -805,14 +810,32 @@
}
}
#endif
+ int xattr_count_all = 0;
if (node.xattr) {
+ int xattr_count_thisblock;
if (xattr_compare(cp, xattr) == FAIL)
do_compare_error;
+ xattr_count(xattr, &xattr_count_thisblock);
+ xattr_count_all += xattr_count_thisblock;
+ char* xattr2 = xattr + XATTR_MAXSIZE;
+ if (*(uint32_t*)xattr2 != 0) {
+ xattr_extract(cp, xattr2);
+ xattr_count(xattr2,
&xattr_count_thisblock);
+ xattr_count_all +=
xattr_count_thisblock;
+ }
}
else {
if (xattr_compare(cp, NULL) == FAIL)
do_compare_error;
}
+ int count_file = xattr_count_file(cp);
+ if (count_file < 0) {
+ do_compare_error;
+ } else if (count_file != xattr_count_all) {
+ fprintf(stderr, "%s: directory does not have
the same number of attributes (%d) as the tape (%d).\n",
+ cp, count_file,
xattr_count_all );
+ do_compare_error;
+ }
ep->e_flags &= ~NEW;
}
}
@@ -925,7 +948,7 @@
if ( fwrite((char *)&node, sizeof(struct modeinfo), 1, mf) != 1 )
err(1,"cannot write to file %s", modefile);
if (xattr)
- if ( fwrite(xattr, XATTR_MAXSIZE, 1, mf) != 1 )
+ if ( fwrite(xattr, XATTR_MAXSIZE*2, 1, mf) != 1 )
err(1,"cannot write to file %s", modefile);
}
diff -urN dump-0.4b47.orig/restore/extern.h dump-0.4b47/restore/extern.h
--- dump-0.4b47.orig/restore/extern.h 2022-05-03 10:02:27.000000000 +0000
+++ dump-0.4b47/restore/extern.h 2022-05-03 10:02:27.000000000 +0000
@@ -154,3 +154,5 @@
int readxattr (char *);
int xattr_compare (char *, char *);
int xattr_extract (char *, char *);
+int xattr_count_file (char *);
+int xattr_count (char *, int *);
diff -urN dump-0.4b47.orig/restore/tape.c dump-0.4b47/restore/tape.c
--- dump-0.4b47.orig/restore/tape.c 2022-05-03 10:02:27.000000000 +0000
+++ dump-0.4b47/restore/tape.c 2022-05-03 10:02:27.000000000 +0000
@@ -1711,39 +1711,63 @@
}
#endif /* !COMPARE_ONTHEFLY */
+/* collect both extended attribute tape blocks, then
+ compare the attributes */
static void
compareattr(char *name)
{
int xattr_done = 0;
+ int xattr_count_all = 0;
while (spcl.c_flags & DR_EXTATTRIBUTES) {
- switch (spcl.c_extattributes) {
- case EXT_MACOSFNDRINFO:
- msg("MacOSX not supported for comparision in this
version, skipping\n");
- skipfile();
- break;
- case EXT_MACOSRESFORK:
- msg("MacOSX not supported for comparision in this
version, skipping\n");
- skipfile();
- break;
- case EXT_XATTR: {
- char xattr[XATTR_MAXSIZE];
+ switch (spcl.c_extattributes) {
+ case EXT_MACOSFNDRINFO:
+ msg("MacOSX not supported for
comparision in this version, skipping\n");
+ skipfile();
+ break;
+ case EXT_MACOSRESFORK:
+ msg("MacOSX not supported for
comparision in this version, skipping\n");
+ skipfile();
+ break;
+ case EXT_XATTR: {
+ char xattr[XATTR_MAXSIZE];
+ int xattr_count_thisblock = 0;
- if (readxattr(xattr) == GOOD) {
- if (xattr_compare(name, xattr) == FAIL)
- do_compare_error;
- xattr_done = 1;
- }
- else
- do_compare_error;
- break;
- }
+ /* in-inode xattr record comes
first, if present at all; then block xattr */
+ if (readxattr(xattr) == GOOD) {
+ /* this returns fail if
the buffer contains different attributes
+ than
the file on disk; note that it mustn't return fail if the file on disk has more
attributes! */
+ if (xattr_compare(name,
xattr) == FAIL)
+
do_compare_error;
+
+ xattr_count(xattr,
&xattr_count_thisblock);
+ xattr_count_all +=
xattr_count_thisblock;
+ xattr_done = 1;
+ }
+ else
+ do_compare_error;
+ break;
+ }
default:
msg("unexpected inode extension %ld, skipping\n",
spcl.c_extattributes);
skipfile();
break;
}
}
+
+ /* cover the case of two xattr sources, the contents of which matched
the file on disk:
+ the only uncovered case left is if there re more
attributes on disk than observed on tape */
+ if (xattr_done) {
+ int count_file = xattr_count_file(name);
+ if (count_file < 0) {
+ do_compare_error;
+ } else if (count_file != xattr_count_all) {
+ fprintf(stderr, "%s: file does not have the
same number of attributes (%d) as the tape (%d).\n",
+ name,
count_file, xattr_count_all );
+ do_compare_error;
+ }
+ }
+ /* no attrs whatsoever? file must have none then as well */
if (!xattr_done && xattr_compare(name, NULL) == FAIL)
do_compare_error;
}
diff -urN dump-0.4b47.orig/restore/xattr.c dump-0.4b47/restore/xattr.c
--- dump-0.4b47.orig/restore/xattr.c 2022-05-03 10:02:27.000000000 +0000
+++ dump-0.4b47/restore/xattr.c 2022-05-03 10:02:27.000000000 +0000
@@ -123,7 +123,7 @@
static int xattr_cb_set (char *, char *, int, int, void *);
static int xattr_cb_compare (char *, char *, int, int, void *);
static int xattr_verify (char *);
-static int xattr_count (char *, int *);
+int xattr_count (char *, int *);
static int xattr_walk (char *, int (*)(char *, char *, int, int, void *), void
*);
#define POSIX_ACL_XATTR_VERSION 0x0002
@@ -373,7 +373,7 @@
#endif
valuesz = lgetxattr(path, name, valuef, XATTR_MAXSIZE);
if (valuesz < 0) {
- warn("%s: EA compare lgetxattr failed\n", path);
+ warn("%s: EA compare lgetxattr for %s failed\n", path,
name);
return FAIL;
}
#ifdef TRANSSELINUX /*GAN6May06 SELinux MLS */
@@ -381,8 +381,9 @@
#endif
if (valuesz != valuelen || memcmp(value, valuef, valuelen)) {
+ valuef[valuesz] = '\0';
/* GAN24May06: show name and new value for user to compare */
- fprintf(stderr, "%s: EA %s:%s value changed to %s\n", path,
name, value, valuef);
+ fprintf(stderr, "%s: EA %s value changed from %s on tape to %s
on disk\n", path, name, value, valuef);
return FAIL;
}
@@ -429,7 +430,7 @@
return GOOD;
}
-static int
+int
xattr_count(char *buffer, int *count)
{
struct ext2_xattr_entry *entry;
@@ -493,6 +494,7 @@
size = entry->e_value_size;
memcpy(value, buffer + VALUE_OFFSET(buffer, entry), size);
+ value[size]='\0';
if (convertacl) {
struct posix_acl *acl;
@@ -540,43 +542,58 @@
return GOOD;
}
-int
-xattr_compare(char *path, char *buffer)
+/* return count of the xattrs on the given file or FAIL if failed */
+int xattr_count_file(char *path)
{
- int countf, countt;
+ int size, count = 0;
char *names = NULL, *end_names, *name;
- countf = llistxattr(path, NULL, 0);
- if (countf < 0) {
- warn("%s: llistxattr failed", path);
- return FAIL;
+ /* return number of bytes a/v */
+ size = llistxattr(path, NULL, 0);
+ if (size < 0) {
+ warn("%s: llistxattr failed", path);
+ return FAIL;
}
- names = malloc(countf + 1);
- if (!names) {
- warn("%s: llistxattr failed", path);
- return FAIL;
+ names = malloc(size + 1);
+ if (!names)
+ {
+ warn("%s: malloc failed", path);
+ return FAIL;
}
- countf = llistxattr(path, names, countf);
- if (countf < 0) {
- warn("%s: llistxattr failed", path);
- free(names);
- return FAIL;
+ /* return nr of bytes and park data in names array */
+ size = llistxattr(path, names, size);
+ if (size < 0)
+ {
+ warn("%s: llistxattr failed", path);
+ free(names);
+ return FAIL;
}
- names[countf] = '\0';
- end_names = names + countf;
+ names[size] = '\0';
+ end_names = names + size;
- countf = 0;
- for (name = names; name != end_names; name = strchr(name, '\0') + 1) {
- if (!*name)
- continue;
- countf++;
+ for (name = names; name != end_names; name = strchr(name, '\0') + 1)
+ {
+ if (!*name)
+ continue;
+ count++;
}
-
free(names);
+ return count;
+}
+
+int
+xattr_compare(char *path, char *buffer)
+{
+ int countf, countt;
+
+ countf = xattr_count_file(path); /* already warns on failure */
+ if (countf < 0)
+ return FAIL;
+
if (buffer) {
if (xattr_verify(buffer) == FAIL)
return FAIL;
@@ -587,10 +604,20 @@
else
countt = 0;
- if (countf != countt) {
- fprintf(stderr, "%s: EA count changed from %d to %d\n", path,
countt, countf);
+ /* with ext4 there may be two tape blocks for attrs: one from the
actual inode, plus
+ one extra filesystem block. in this case countf must be at
least >= countt;
+ verification of that aspect is delayed and left to compareattr
+ which loops over the different blocks */
+
+ /* nothing on tape but something on disk? sure fail. ditto for more on
tape than on disk. */
+ if (countf > 0 && !countt)
+ {
+ fprintf(stderr, "%s: file does not have the same number of
attributes (%d) as the tape (%d).\n",
+ path, countf, countt);
return FAIL;
}
+ if (countf < countt)
+ return FAIL;
/* warning left to compareattr */
if (!buffer)
return GOOD;
