From 17c77a5dc51064dfbd4a0ec6129a911ab59279a3 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 11 Aug 2016 15:30:50 +0200 Subject: [PATCH 1/9] xattr: dynamically grow result buffer until it fits, fixes #1462 this also fixes the race condition seen in #1462 because there is only 1 call now. either it succeeds, then we get the correct length as result and truncate the result value to that length. or it fails with ERANGE, then we grow the buffer to double size and repeat. or it fails with some other error, then we throw OSError. --- borg/testsuite/xattr.py | 22 ++++++++++++- borg/xattr.py | 68 ++++++++++++++++++++++++++++++----------- 2 files changed, 72 insertions(+), 18 deletions(-) diff --git a/borg/testsuite/xattr.py b/borg/testsuite/xattr.py index df0130c90e..ecc92fa188 100644 --- a/borg/testsuite/xattr.py +++ b/borg/testsuite/xattr.py @@ -2,7 +2,7 @@ import tempfile import unittest -from ..xattr import is_enabled, getxattr, setxattr, listxattr +from ..xattr import is_enabled, getxattr, setxattr, listxattr, get_buffer from . import BaseTestCase @@ -38,3 +38,23 @@ def test(self): self.assert_equal(getxattr(self.tmpfile.fileno(), 'user.foo'), b'bar') self.assert_equal(getxattr(self.symlink, 'user.foo'), b'bar') self.assert_equal(getxattr(self.tmpfile.name, 'user.empty'), None) + + def test_listxattr_buffer_growth(self): + # make it work even with ext4, which imposes rather low limits + get_buffer(size=64, init=True) + # xattr raw key list will be size 9 * (10 + 1), which is > 64 + keys = ['user.attr%d' % i for i in range(9)] + for key in keys: + setxattr(self.tmpfile.name, key, b'x') + got_keys = listxattr(self.tmpfile.name) + self.assert_equal_se(got_keys, keys) + self.assert_equal(len(get_buffer()), 128) + + def test_getxattr_buffer_growth(self): + # make it work even with ext4, which imposes rather low limits + get_buffer(size=64, init=True) + value = b'x' * 126 + setxattr(self.tmpfile.name, 'user.big', value) + got_value = getxattr(self.tmpfile.name, 'user.big') + self.assert_equal(value, got_value) + self.assert_equal(len(get_buffer()), 128) diff --git a/borg/xattr.py b/borg/xattr.py index e88d7ce8c9..146f3ae769 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -6,6 +6,7 @@ import subprocess import sys import tempfile +import threading from ctypes import CDLL, create_string_buffer, c_ssize_t, c_size_t, c_char_p, c_int, c_uint32, get_errno from ctypes.util import find_library from distutils.version import LooseVersion @@ -14,6 +15,18 @@ logger = create_logger() +def get_buffer(size=None, init=False): + if size is not None: + size = int(size) + if init or len(thread_local.buffer) < size: + thread_local.buffer = create_string_buffer(size) + return thread_local.buffer + + +thread_local = threading.local() +get_buffer(size=4096, init=True) + + def is_enabled(path=None): """Determine if xattr is enabled on the filesystem """ @@ -78,9 +91,17 @@ def get_all(path, follow_symlinks=True): raise Exception(msg) +class BufferTooSmallError(Exception): + """the buffer given to an xattr function was too small for the result""" + + def _check(rv, path=None): if rv < 0: - raise OSError(get_errno(), path) + e = get_errno() + if e == errno.ERANGE: + raise BufferTooSmallError + else: + raise OSError(e, path) return rv if sys.platform.startswith('linux'): # pragma: linux only @@ -106,14 +127,20 @@ def listxattr(path, *, follow_symlinks=True): func = libc.listxattr else: func = libc.llistxattr - n = _check(func(path, None, 0), path) - if n == 0: - return [] - namebuf = create_string_buffer(n) - n2 = _check(func(path, namebuf, n), path) - if n2 != n: - raise Exception('listxattr failed') - return [os.fsdecode(name) for name in namebuf.raw.split(b'\0')[:-1] if not name.startswith(b'system.posix_acl_')] + size = len(get_buffer()) + while True: + buf = get_buffer(size) + try: + n = _check(func(path, buf, size), path) + except BufferTooSmallError: + size *= 2 + else: + if n == 0: + return [] + names = buf.raw[:n] + return [os.fsdecode(name) + for name in names.split(b'\0')[:-1] + if not name.startswith(b'system.posix_acl_')] def getxattr(path, name, *, follow_symlinks=True): name = os.fsencode(name) @@ -125,14 +152,17 @@ def getxattr(path, name, *, follow_symlinks=True): func = libc.getxattr else: func = libc.lgetxattr - n = _check(func(path, name, None, 0)) - if n == 0: - return - valuebuf = create_string_buffer(n) - n2 = _check(func(path, name, valuebuf, n), path) - if n2 != n: - raise Exception('getxattr failed') - return valuebuf.raw + size = len(get_buffer()) + while True: + buf = get_buffer(size) + try: + n = _check(func(path, name, buf, size), path) + except BufferTooSmallError: + size *= 2 + else: + if n == 0: + return + return buf.raw[:n] def setxattr(path, name, value, *, follow_symlinks=True): name = os.fsencode(name) @@ -172,6 +202,7 @@ def listxattr(path, *, follow_symlinks=True): func = libc.flistxattr elif not follow_symlinks: flags = XATTR_NOFOLLOW + # TODO: fix, see linux n = _check(func(path, None, 0, flags), path) if n == 0: return [] @@ -191,6 +222,7 @@ def getxattr(path, name, *, follow_symlinks=True): func = libc.fgetxattr elif not follow_symlinks: flags = XATTR_NOFOLLOW + # TODO: fix, see linux n = _check(func(path, name, None, 0, 0, flags)) if n == 0: return @@ -244,6 +276,7 @@ def listxattr(path, *, follow_symlinks=True): func = libc.extattr_list_file else: func = libc.extattr_list_link + # TODO: fix, see linux n = _check(func(path, ns, None, 0), path) if n == 0: return [] @@ -269,6 +302,7 @@ def getxattr(path, name, *, follow_symlinks=True): func = libc.extattr_get_file else: func = libc.extattr_get_link + # TODO: fix, see linux n = _check(func(path, EXTATTR_NAMESPACE_USER, name, None, 0)) if n == 0: return From 67c6c1898cbaf0daf591a8c00749b068b225ac56 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 11 Aug 2016 19:47:04 +0200 Subject: [PATCH 2/9] xattr: refactor code, deduplicate this code would be otherwise duplicated 3 times for linux, freebsd, darwin. --- borg/xattr.py | 282 ++++++++++++++++++++++++++------------------------ 1 file changed, 145 insertions(+), 137 deletions(-) diff --git a/borg/xattr.py b/borg/xattr.py index 146f3ae769..94df6b05c6 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -46,6 +46,7 @@ def get_all(path, follow_symlinks=True): if e.errno in (errno.ENOTSUP, errno.EPERM): return {} + libc_name = find_library('c') if libc_name is None: # find_library didn't work, maybe we are on some minimal system that misses essential @@ -104,6 +105,46 @@ def _check(rv, path=None): raise OSError(e, path) return rv + +def _listxattr_inner(func, path): + if isinstance(path, str): + path = os.fsencode(path) + size = len(get_buffer()) + while True: + buf = get_buffer(size) + try: + n = _check(func(path, buf, size), path) + except BufferTooSmallError: + size *= 2 + assert size < 2 ** 20 + else: + return n, buf.raw + + +def _getxattr_inner(func, path, name): + if isinstance(path, str): + path = os.fsencode(path) + name = os.fsencode(name) + size = len(get_buffer()) + while True: + buf = get_buffer(size) + try: + n = _check(func(path, name, buf, size), path) + except BufferTooSmallError: + size *= 2 + else: + return n, buf.raw + + +def _setxattr_inner(func, path, name, value): + if isinstance(path, str): + path = os.fsencode(path) + name = os.fsencode(name) + value = value and os.fsencode(value) + size = len(value) if value else 0 + _check(func(path, name, value, size), path) + + if sys.platform.startswith('linux'): # pragma: linux only libc.llistxattr.argtypes = (c_char_p, c_char_p, c_size_t) libc.llistxattr.restype = c_ssize_t @@ -119,63 +160,49 @@ def _check(rv, path=None): libc.fgetxattr.restype = c_ssize_t def listxattr(path, *, follow_symlinks=True): - if isinstance(path, str): - path = os.fsencode(path) - if isinstance(path, int): - func = libc.flistxattr - elif follow_symlinks: - func = libc.listxattr - else: - func = libc.llistxattr - size = len(get_buffer()) - while True: - buf = get_buffer(size) - try: - n = _check(func(path, buf, size), path) - except BufferTooSmallError: - size *= 2 + def func(path, buf, size): + if isinstance(path, int): + return libc.flistxattr(path, buf, size) else: - if n == 0: - return [] - names = buf.raw[:n] - return [os.fsdecode(name) - for name in names.split(b'\0')[:-1] - if not name.startswith(b'system.posix_acl_')] + if follow_symlinks: + return libc.listxattr(path, buf, size) + else: + return libc.llistxattr(path, buf, size) + + n, buf = _listxattr_inner(func, path) + if n == 0: + return [] + names = buf[:n].split(b'\0')[:-1] + return [os.fsdecode(name) for name in names + if not name.startswith(b'system.posix_acl_')] def getxattr(path, name, *, follow_symlinks=True): - name = os.fsencode(name) - if isinstance(path, str): - path = os.fsencode(path) - if isinstance(path, int): - func = libc.fgetxattr - elif follow_symlinks: - func = libc.getxattr - else: - func = libc.lgetxattr - size = len(get_buffer()) - while True: - buf = get_buffer(size) - try: - n = _check(func(path, name, buf, size), path) - except BufferTooSmallError: - size *= 2 + def func(path, name, buf, size): + if isinstance(path, int): + return libc.fgetxattr(path, name, buf, size) else: - if n == 0: - return - return buf.raw[:n] + if follow_symlinks: + return libc.getxattr(path, name, buf, size) + else: + return libc.lgetxattr(path, name, buf, size) + + n, buf = _getxattr_inner(func, path, name) + if n == 0: + return + return buf[:n] def setxattr(path, name, value, *, follow_symlinks=True): - name = os.fsencode(name) - value = value and os.fsencode(value) - if isinstance(path, str): - path = os.fsencode(path) - if isinstance(path, int): - func = libc.fsetxattr - elif follow_symlinks: - func = libc.setxattr - else: - func = libc.lsetxattr - _check(func(path, name, value, len(value) if value else 0, 0), path) + def func(path, name, value, size): + flags = 0 + if isinstance(path, int): + return libc.fsetxattr(path, name, value, size, flags) + else: + if follow_symlinks: + return libc.setxattr(path, name, value, size, flags) + else: + return libc.lsetxattr(path, name, value, size, flags) + + _setxattr_inner(func, path, name, value) elif sys.platform == 'darwin': # pragma: darwin only libc.listxattr.argtypes = (c_char_p, c_char_p, c_size_t, c_int) @@ -191,62 +218,53 @@ def setxattr(path, name, value, *, follow_symlinks=True): libc.fgetxattr.argtypes = (c_int, c_char_p, c_char_p, c_size_t, c_uint32, c_int) libc.fgetxattr.restype = c_ssize_t + XATTR_NOFLAGS = 0x0000 XATTR_NOFOLLOW = 0x0001 def listxattr(path, *, follow_symlinks=True): - func = libc.listxattr - flags = 0 - if isinstance(path, str): - path = os.fsencode(path) - if isinstance(path, int): - func = libc.flistxattr - elif not follow_symlinks: - flags = XATTR_NOFOLLOW - # TODO: fix, see linux - n = _check(func(path, None, 0, flags), path) + def func(path, buf, size): + if isinstance(path, int): + return libc.flistxattr(path, buf, size, XATTR_NOFLAGS) + else: + if follow_symlinks: + return libc.listxattr(path, buf, size, XATTR_NOFLAGS) + else: + return libc.listxattr(path, buf, size, XATTR_NOFOLLOW) + + n, buf = _listxattr_inner(func, path) if n == 0: return [] - namebuf = create_string_buffer(n) - n2 = _check(func(path, namebuf, n, flags), path) - if n2 != n: - raise Exception('listxattr failed') - return [os.fsdecode(name) for name in namebuf.raw.split(b'\0')[:-1]] + names = buf[:n].split(b'\0')[:-1] + return [os.fsdecode(name) for name in names] def getxattr(path, name, *, follow_symlinks=True): - name = os.fsencode(name) - func = libc.getxattr - flags = 0 - if isinstance(path, str): - path = os.fsencode(path) - if isinstance(path, int): - func = libc.fgetxattr - elif not follow_symlinks: - flags = XATTR_NOFOLLOW - # TODO: fix, see linux - n = _check(func(path, name, None, 0, 0, flags)) + def func(path, name, buf, size): + if isinstance(path, int): + return libc.fgetxattr(path, name, buf, size, 0, XATTR_NOFLAGS) + else: + if follow_symlinks: + return libc.getxattr(path, name, buf, size, 0, XATTR_NOFLAGS) + else: + return libc.getxattr(path, name, buf, size, 0, XATTR_NOFOLLOW) + + n, buf = _getxattr_inner(func, path, name) if n == 0: return - valuebuf = create_string_buffer(n) - n2 = _check(func(path, name, valuebuf, n, 0, flags), path) - if n2 != n: - raise Exception('getxattr failed') - return valuebuf.raw + return buf[:n] def setxattr(path, name, value, *, follow_symlinks=True): - name = os.fsencode(name) - value = value and os.fsencode(value) - func = libc.setxattr - flags = 0 - if isinstance(path, str): - path = os.fsencode(path) - if isinstance(path, int): - func = libc.fsetxattr - elif not follow_symlinks: - flags = XATTR_NOFOLLOW - _check(func(path, name, value, len(value) if value else 0, 0, flags), path) + def func(path, name, value, size): + if isinstance(path, int): + return libc.fsetxattr(path, name, value, size, 0, XATTR_NOFLAGS) + else: + if follow_symlinks: + return libc.setxattr(path, name, value, size, 0, XATTR_NOFLAGS) + else: + return libc.setxattr(path, name, value, size, 0, XATTR_NOFOLLOW) + + _setxattr_inner(func, path, name, value) elif sys.platform.startswith('freebsd'): # pragma: freebsd only - EXTATTR_NAMESPACE_USER = 0x0001 libc.extattr_list_fd.argtypes = (c_int, c_int, c_char_p, c_size_t) libc.extattr_list_fd.restype = c_ssize_t libc.extattr_list_link.argtypes = (c_char_p, c_int, c_char_p, c_size_t) @@ -265,27 +283,23 @@ def setxattr(path, name, value, *, follow_symlinks=True): libc.extattr_set_link.restype = c_int libc.extattr_set_file.argtypes = (c_char_p, c_int, c_char_p, c_char_p, c_size_t) libc.extattr_set_file.restype = c_int + ns = EXTATTR_NAMESPACE_USER = 0x0001 def listxattr(path, *, follow_symlinks=True): - ns = EXTATTR_NAMESPACE_USER - if isinstance(path, str): - path = os.fsencode(path) - if isinstance(path, int): - func = libc.extattr_list_fd - elif follow_symlinks: - func = libc.extattr_list_file - else: - func = libc.extattr_list_link - # TODO: fix, see linux - n = _check(func(path, ns, None, 0), path) + def func(path, buf, size): + if isinstance(path, int): + return libc.extattr_list_fd(path, ns, buf, size) + else: + if follow_symlinks: + return libc.extattr_list_file(path, ns, buf, size) + else: + return libc.extattr_list_link(path, ns, buf, size) + + n, buf = _listxattr_inner(func, path) if n == 0: return [] - namebuf = create_string_buffer(n) - n2 = _check(func(path, ns, namebuf, n), path) - if n2 != n: - raise Exception('listxattr failed') names = [] - mv = memoryview(namebuf.raw) + mv = memoryview(buf) while mv: length = mv[0] names.append(os.fsdecode(bytes(mv[1:1 + length]))) @@ -293,37 +307,31 @@ def listxattr(path, *, follow_symlinks=True): return names def getxattr(path, name, *, follow_symlinks=True): - name = os.fsencode(name) - if isinstance(path, str): - path = os.fsencode(path) - if isinstance(path, int): - func = libc.extattr_get_fd - elif follow_symlinks: - func = libc.extattr_get_file - else: - func = libc.extattr_get_link - # TODO: fix, see linux - n = _check(func(path, EXTATTR_NAMESPACE_USER, name, None, 0)) + def func(path, name, buf, size): + if isinstance(path, int): + return libc.extattr_get_fd(path, ns, name, buf, size) + else: + if follow_symlinks: + return libc.extattr_get_file(path, ns, name, buf, size) + else: + return libc.extattr_get_link(path, ns, name, buf, size) + + n, buf = _getxattr_inner(func, path, name) if n == 0: return - valuebuf = create_string_buffer(n) - n2 = _check(func(path, EXTATTR_NAMESPACE_USER, name, valuebuf, n), path) - if n2 != n: - raise Exception('getxattr failed') - return valuebuf.raw + return buf[:n] def setxattr(path, name, value, *, follow_symlinks=True): - name = os.fsencode(name) - value = value and os.fsencode(value) - if isinstance(path, str): - path = os.fsencode(path) - if isinstance(path, int): - func = libc.extattr_set_fd - elif follow_symlinks: - func = libc.extattr_set_file - else: - func = libc.extattr_set_link - _check(func(path, EXTATTR_NAMESPACE_USER, name, value, len(value) if value else 0), path) + def func(path, name, value, size): + if isinstance(path, int): + return libc.extattr_set_fd(path, ns, name, value, size) + else: + if follow_symlinks: + return libc.extattr_set_file(path, ns, name, value, size) + else: + return libc.extattr_set_link(path, ns, name, value, size) + + _setxattr_inner(func, path, name, value) else: # pragma: unknown platform only def listxattr(path, *, follow_symlinks=True): From 09dbec99a05040244f1e4aca908552ba07a42052 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 11 Aug 2016 20:11:36 +0200 Subject: [PATCH 3/9] raise OSError including the error message derived from errno also: deal with path being a integer FD --- borg/xattr.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/borg/xattr.py b/borg/xattr.py index 94df6b05c6..82a3afb4e9 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -102,7 +102,13 @@ def _check(rv, path=None): if e == errno.ERANGE: raise BufferTooSmallError else: - raise OSError(e, path) + try: + msg = os.strerror(e) + except ValueError: + msg = '' + if isinstance(path, int): + path = '' % path + raise OSError(e, msg, path) return rv From 418794f66f9b577de67a9ba15618738aa81ba626 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 11 Aug 2016 21:52:48 +0200 Subject: [PATCH 4/9] xattr: errno ERANGE has different meanings --- borg/xattr.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/borg/xattr.py b/borg/xattr.py index 82a3afb4e9..2649eadb08 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -18,6 +18,7 @@ def get_buffer(size=None, init=False): if size is not None: size = int(size) + assert size < 2 ** 24 if init or len(thread_local.buffer) < size: thread_local.buffer = create_string_buffer(size) return thread_local.buffer @@ -96,10 +97,12 @@ class BufferTooSmallError(Exception): """the buffer given to an xattr function was too small for the result""" -def _check(rv, path=None): +def _check(rv, path=None, detect_buffer_too_small=False): if rv < 0: e = get_errno() - if e == errno.ERANGE: + if detect_buffer_too_small and e == errno.ERANGE: + # listxattr and getxattr signal with ERANGE that they need a bigger result buffer. + # setxattr signals this way that e.g. a xattr key name is too long / inacceptable. raise BufferTooSmallError else: try: @@ -119,10 +122,9 @@ def _listxattr_inner(func, path): while True: buf = get_buffer(size) try: - n = _check(func(path, buf, size), path) + n = _check(func(path, buf, size), path, detect_buffer_too_small=True) except BufferTooSmallError: size *= 2 - assert size < 2 ** 20 else: return n, buf.raw @@ -135,7 +137,7 @@ def _getxattr_inner(func, path, name): while True: buf = get_buffer(size) try: - n = _check(func(path, name, buf, size), path) + n = _check(func(path, name, buf, size), path, detect_buffer_too_small=True) except BufferTooSmallError: size *= 2 else: @@ -148,7 +150,7 @@ def _setxattr_inner(func, path, name, value): name = os.fsencode(name) value = value and os.fsencode(value) size = len(value) if value else 0 - _check(func(path, name, value, size), path) + _check(func(path, name, value, size), path, detect_buffer_too_small=False) if sys.platform.startswith('linux'): # pragma: linux only From 4eac66fe2aa7c18569703ef10149c9171dc784a7 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 11 Aug 2016 22:47:12 +0200 Subject: [PATCH 5/9] xattr: fix race condition in get_all(), see issue #906 --- borg/xattr.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/borg/xattr.py b/borg/xattr.py index 2649eadb08..0439b193c1 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -15,6 +15,13 @@ logger = create_logger() +try: + ENOATTR = errno.ENOATTR +except AttributeError: + # on some platforms, ENOATTR is missing, use ENODATA there + ENOATTR = errno.ENODATA + + def get_buffer(size=None, init=False): if size is not None: size = int(size) @@ -41,8 +48,17 @@ def is_enabled(path=None): def get_all(path, follow_symlinks=True): try: - return dict((name, getxattr(path, name, follow_symlinks=follow_symlinks)) - for name in listxattr(path, follow_symlinks=follow_symlinks)) + result = {} + names = listxattr(path, follow_symlinks=follow_symlinks) + for name in names: + try: + result[name] = getxattr(path, name, follow_symlinks=follow_symlinks) + except OSError as e: + # if we get ENOATTR, a race has happened: xattr names were deleted after list. + # we just ignore the now missing ones. if you want consistency, do snapshots. + if e.errno != ENOATTR: + raise + return result except OSError as e: if e.errno in (errno.ENOTSUP, errno.EPERM): return {} From 7ea052a5e80ece5fd70640c918d6bcb8f50d3a5a Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 12 Aug 2016 02:55:49 +0200 Subject: [PATCH 6/9] xattr: buffer full check for freebsd freebsd 10.2: it does not give rc < 0 and errno == ERANGE if the buffer was too small, like linux or mac OS X does. rv == buffer len might be a signal of truncation. rv > buffer len would be even worse not sure if some implementation returns the total length of the data, not just the amount put into the buffer. but as we use the returned length to "truncate" the buffer, we better make sure it is not longer than the buffer. also: freebsd listxattr memoryview len bugfix --- borg/xattr.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/borg/xattr.py b/borg/xattr.py index 0439b193c1..99f7657bb1 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -128,6 +128,12 @@ def _check(rv, path=None, detect_buffer_too_small=False): if isinstance(path, int): path = '' % path raise OSError(e, msg, path) + if detect_buffer_too_small and rv >= len(get_buffer()): + # freebsd does not error with ERANGE if the buffer is too small, + # it just fills the buffer, truncates and returns. + # so, we play sure and just assume that result is truncated if + # it happens to be a full buffer. + raise BufferTooSmallError return rv @@ -323,7 +329,7 @@ def func(path, buf, size): if n == 0: return [] names = [] - mv = memoryview(buf) + mv = memoryview(buf)[:n] while mv: length = mv[0] names.append(os.fsdecode(bytes(mv[1:1 + length]))) From b6ead3dce2cabd07b5fdf5f50a228f8e76108c5c Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 12 Aug 2016 04:17:27 +0200 Subject: [PATCH 7/9] xattr: use some string processing functions, dedup, simplify --- borg/xattr.py | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/borg/xattr.py b/borg/xattr.py index 99f7657bb1..0099a2ad96 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -109,6 +109,22 @@ def get_all(path, follow_symlinks=True): raise Exception(msg) +def split_string0(buf): + """split a list of zero-terminated strings into python not-zero-terminated bytes""" + return buf.split(b'\0')[:-1] + + +def split_lstring(buf): + """split a list of length-prefixed strings into python not-length-prefixed bytes""" + result = [] + mv = memoryview(buf) + while mv: + length = mv[0] + result.append(bytes(mv[1:1 + length])) + mv = mv[1 + length:] + return result + + class BufferTooSmallError(Exception): """the buffer given to an xattr function was too small for the result""" @@ -200,10 +216,7 @@ def func(path, buf, size): return libc.llistxattr(path, buf, size) n, buf = _listxattr_inner(func, path) - if n == 0: - return [] - names = buf[:n].split(b'\0')[:-1] - return [os.fsdecode(name) for name in names + return [os.fsdecode(name) for name in split_string0(buf[:n]) if not name.startswith(b'system.posix_acl_')] def getxattr(path, name, *, follow_symlinks=True): @@ -217,9 +230,7 @@ def func(path, name, buf, size): return libc.lgetxattr(path, name, buf, size) n, buf = _getxattr_inner(func, path, name) - if n == 0: - return - return buf[:n] + return buf[:n] or None def setxattr(path, name, value, *, follow_symlinks=True): def func(path, name, value, size): @@ -262,10 +273,7 @@ def func(path, buf, size): return libc.listxattr(path, buf, size, XATTR_NOFOLLOW) n, buf = _listxattr_inner(func, path) - if n == 0: - return [] - names = buf[:n].split(b'\0')[:-1] - return [os.fsdecode(name) for name in names] + return [os.fsdecode(name) for name in split_string0(buf[:n])] def getxattr(path, name, *, follow_symlinks=True): def func(path, name, buf, size): @@ -278,9 +286,7 @@ def func(path, name, buf, size): return libc.getxattr(path, name, buf, size, 0, XATTR_NOFOLLOW) n, buf = _getxattr_inner(func, path, name) - if n == 0: - return - return buf[:n] + return buf[:n] or None def setxattr(path, name, value, *, follow_symlinks=True): def func(path, name, value, size): @@ -326,15 +332,7 @@ def func(path, buf, size): return libc.extattr_list_link(path, ns, buf, size) n, buf = _listxattr_inner(func, path) - if n == 0: - return [] - names = [] - mv = memoryview(buf)[:n] - while mv: - length = mv[0] - names.append(os.fsdecode(bytes(mv[1:1 + length]))) - mv = mv[1 + length:] - return names + return [os.fsdecode(name) for name in split_lstring(buf[:n])] def getxattr(path, name, *, follow_symlinks=True): def func(path, name, buf, size): @@ -347,9 +345,7 @@ def func(path, name, buf, size): return libc.extattr_get_link(path, ns, name, buf, size) n, buf = _getxattr_inner(func, path, name) - if n == 0: - return - return buf[:n] + return buf[:n] or None def setxattr(path, name, value, *, follow_symlinks=True): def func(path, name, value, size): From 8630ebf3f0bd1a5743f9d8507b12d6661c1b875b Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 12 Aug 2016 04:21:21 +0200 Subject: [PATCH 8/9] xattr: fix module docstring --- borg/xattr.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/borg/xattr.py b/borg/xattr.py index 0099a2ad96..7a4a7ae929 100644 --- a/borg/xattr.py +++ b/borg/xattr.py @@ -1,5 +1,5 @@ -"""A basic extended attributes (xattr) implementation for Linux and MacOS X -""" +"""A basic extended attributes (xattr) implementation for Linux, FreeBSD and MacOS X.""" + import errno import os import re From 3c7dddcb99586c2e7ac791177598fa99e2fb459f Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 12 Aug 2016 18:00:50 +0200 Subject: [PATCH 9/9] update changelog --- docs/changes.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index 305be063ca..4972dc3828 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -57,6 +57,10 @@ Security fixes: - fix security issue with remote repository access, #1428 + +Version 1.0.7rc2 (not released yet) +----------------------------------- + Bug fixes: - do not write objects to repository that are bigger than the allowed size, @@ -64,6 +68,11 @@ Bug fixes: IMPORTANT: if you created archives with many millions of files or directories, please verify if you can open them successfully, e.g. try a "borg list REPO::ARCHIVE". +- fixed a race condition in extended attributes querying that led to the + entire file not being backed up (while logging the error, exit code = 1), + #1469 +- fixed a race condition in extended attributes querying that led to a crash, + #1462 Version 1.0.7rc1 (2016-08-05)