Handle kernel/FUSE disagreement over Linux attrs

Use long for the Linux attr type, so that it should work on all
little-endian systems, across both normal and FUSE-backed filesystems.

Disable Linux attr support for now on big-endian systems where
sizeof(long) > sizeof(int).  See the changes to bup-index.md and
_helpers.c for more information.

Eventually, we may want to add an argument to allow re-enabling
support on affected systems, when the user can guarantee a homogeneous
filesystem type -- or better yet, the kernel and FUSE will finally
sort out their issues.

Thanks to Tilo Schwarz <mail@tilo-schwarz.de> and daryl5@arcor.de for
reporting and pursuing the problems that lead to this patch.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>

diff --git a/Documentation/bup-index.md b/Documentation/bup-index.md
index c69c2aba8c61de2efb55c8c9f7df6e8ce8b223e2..280d9eb1d4fe361fc124eb07fceda9573c9f3f4c 100644 (file)
--- a/Documentation/bup-index.md
+++ b/Documentation/bup-index.md
@@ -42,6 +42,15 @@ need the same information).
 
 # NOTES
 
+At the moment, bup will ignore Linux attributes (cf. chattr(1) and
+lsattr(1)) on some systems (any big-endian systems where sizeof(long)
+< sizeof(int)).  This is because the Linux kernel and FUSE currently
+disagree over the type of the attr system call arguments, and so on
+big-endian systems there's no way to get the results without the risk
+of stack corruption (http://lwn.net/Articles/575846/).  In these
+situations, bup will print a warning the first time Linux attrs are
+relevant during any index/save/restore operation.
+
 bup makes accommodations for the expected "worst-case" filesystem
 timestamp resolution -- currently one second; examples include VFAT,
 ext2, ext3, small ext4, etc.  Since bup cannot know the filesystem

diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index 4d2f9fd14aee6885be851b20e0cf110ab00072af..65a8b6bafa8ef51efeab07c5d8cf7febb7b0787e 100644 (file)
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -815,11 +815,21 @@ static PyObject *fadvise_done(PyObject *self, PyObject *args)
 }
 
 
+// Currently the Linux kernel and FUSE disagree over the type for
+// FS_IOC_GETFLAGS and FS_IOC_SETFLAGS.  The kernel actually uses int,
+// but FUSE chose long (matching the declaration in linux/fs.h).  So
+// if you use int, and then traverse a FUSE filesystem, you may
+// corrupt the stack.  But if you use long, then you may get invalid
+// results on big-endian systems.
+//
+// For now, we just use long, and then disable Linux attrs entirely
+// (with a warning) in helpers.py on systems that are affected.
+
 #ifdef BUP_HAVE_FILE_ATTRS
 static PyObject *bup_get_linux_file_attr(PyObject *self, PyObject *args)
 {
     int rc;
-    unsigned int attr;
+    unsigned long attr;
     char *path;
     int fd;
 
@@ -830,8 +840,9 @@ static PyObject *bup_get_linux_file_attr(PyObject *self, PyObject *args)
     if (fd == -1)
         return PyErr_SetFromErrnoWithFilename(PyExc_OSError, path);
 
-    attr = 0;
+    attr = 0;  // Handle int/long mismatch (see above)
     rc = ioctl(fd, FS_IOC_GETFLAGS, &attr);
+    assert(attr <= UINT_MAX);  // Kernel type is actually int
     if (rc == -1)
     {
         close(fd);
@@ -839,7 +850,7 @@ static PyObject *bup_get_linux_file_attr(PyObject *self, PyObject *args)
     }
 
     close(fd);
-    return Py_BuildValue("I", attr);
+    return PyLong_FromUnsignedLong(attr);
 }
 #endif /* def BUP_HAVE_FILE_ATTRS */
 
@@ -849,7 +860,8 @@ static PyObject *bup_get_linux_file_attr(PyObject *self, PyObject *args)
 static PyObject *bup_set_linux_file_attr(PyObject *self, PyObject *args)
 {
     int rc;
-    unsigned int orig_attr, attr;
+    unsigned long orig_attr;
+    unsigned int attr;
     char *path;
     PyObject *py_attr;
     int fd;
@@ -873,13 +885,15 @@ static PyObject *bup_set_linux_file_attr(PyObject *self, PyObject *args)
     | FS_TOPDIR_FL | FS_NOCOW_FL;
 
     // The extents flag can't be removed, so don't (see chattr(1) and chattr.c).
+    orig_attr = 0; // Handle int/long mismatch (see above)
     rc = ioctl(fd, FS_IOC_GETFLAGS, &orig_attr);
+    assert(orig_attr <= UINT_MAX);  // Kernel type is actually int
     if (rc == -1)
     {
         close(fd);
         return PyErr_SetFromErrnoWithFilename(PyExc_OSError, path);
     }
-    attr |= (orig_attr & FS_EXTENT_FL);
+    attr |= ((unsigned int) orig_attr) & FS_EXTENT_FL;
 
     rc = ioctl(fd, FS_IOC_SETFLAGS, &attr);
     if (rc == -1)

diff --git a/lib/bup/metadata.py b/lib/bup/metadata.py
index b4cedf32005cd9960d7110a7537692a9c2980e68..685c6630495ef8f361390576d832f8036dfe6266 100644 (file)
--- a/lib/bup/metadata.py
+++ b/lib/bup/metadata.py
@@ -4,7 +4,7 @@
 #
 # This code is covered under the terms of the GNU Library General
 # Public License as described in the bup LICENSE file.
-import errno, os, sys, stat, time, pwd, grp, socket
+import errno, os, sys, stat, time, pwd, grp, socket, struct
 from cStringIO import StringIO
 from bup import vint, xstat
 from bup.drecurse import recursive_dirlist
@@ -42,7 +42,19 @@ except ImportError:
     # not on Linux, in which case files don't have any linux attrs anyway, so
     # lacking the functions isn't a problem.
     get_linux_file_attr = set_linux_file_attr = None
-    
+
+
+_suppress_linux_file_attr = \
+    sys.byteorder == 'big' and struct.calcsize('=l') > struct.calcsize('=i')
+
+def check_linux_file_attr_api():
+    global get_linux_file_attr, set_linux_file_attr
+    if not (get_linux_file_attr or set_linux_file_attr):
+        return
+    if _suppress_linux_file_attr:
+        log('Warning: Linux attr support disabled (see "bup help index").\n')
+        get_linux_file_attr = set_linux_file_attr = None
+
 
 # WARNING: the metadata encoding is *not* stable yet.  Caveat emptor!
 
@@ -554,6 +566,7 @@ class Metadata:
     ## Linux attributes (lsattr(1), chattr(1))
 
     def _add_linux_attr(self, path, st):
+        check_linux_file_attr_api()
         if not get_linux_file_attr: return
         if stat.S_ISREG(st.st_mode) or stat.S_ISDIR(st.st_mode):
             try:
@@ -585,6 +598,7 @@ class Metadata:
 
     def _apply_linux_attr_rec(self, path, restore_numeric_ids=False):
         if self.linux_attr:
+            check_linux_file_attr_api()
             if not set_linux_file_attr:
                 add_error("%s: can't restore linuxattrs: "
                           "linuxattr support missing.\n" % path)