From 1667926c9645a7030835ead982629a6191366da5 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 12 Jan 2017 01:01:24 +0100 Subject: [PATCH 01/21] fix bad parsing of wrong syntax this was like whack-a-mole: fix one regex -> another issue pops up --- borg/helpers.py | 12 +++++++++--- borg/testsuite/helpers.py | 5 +++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/borg/helpers.py b/borg/helpers.py index 7c4a0fa91..2d1ec5100 100644 --- a/borg/helpers.py +++ b/borg/helpers.py @@ -823,11 +823,17 @@ class Location: """ # path must not contain :: (it ends at :: or string end), but may contain single colons. - # to avoid ambiguities with other regexes, it must also not start with ":". + # to avoid ambiguities with other regexes, it must also not start with ":" nor with "//" nor with "ssh://". path_re = r""" - (?!:) # not starting with ":" + (?!(:|//|ssh://)) # not starting with ":" or // or ssh:// (?P([^:]|(:(?!:)))+) # any chars, but no "::" """ + # abs_path must not contain :: (it ends at :: or string end), but may contain single colons. + # it must start with a / and that slash is part of the path. + abs_path_re = r""" + (?P(/([^:]|(:(?!:)))+)) # start with /, then any chars, but no "::" + """ + # optional ::archive_name at the end, archive name must not contain "/". # borg mount's FUSE filesystem creates one level of directories from # the archive names and of course "/" is not valid in a directory name. @@ -842,7 +848,7 @@ class Location: (?Pssh):// # ssh:// """ + optional_user_re + r""" # user@ (optional) (?P[^:/]+)(?::(?P\d+))? # host or host:port - """ + path_re + optional_archive_re, re.VERBOSE) # path or path::archive + """ + abs_path_re + optional_archive_re, re.VERBOSE) # path or path::archive file_re = re.compile(r""" (?Pfile):// # file:// diff --git a/borg/testsuite/helpers.py b/borg/testsuite/helpers.py index 3f9c70969..dcf1859ec 100644 --- a/borg/testsuite/helpers.py +++ b/borg/testsuite/helpers.py @@ -134,6 +134,11 @@ class TestLocationWithoutEnv: location_time2 = Location('/some/path::archive{now:%s}') assert location_time1.archive != location_time2.archive + def test_bad_syntax(self): + with pytest.raises(ValueError): + # this is invalid due to the 2nd colon, correct: 'ssh://user@host/path' + Location('ssh://user@host:/path') + class TestLocationWithEnv: def test_ssh(self, monkeypatch): From cdffd93139cef65e0c8f0ba441fbf4317d40904c Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 17 Jan 2017 02:09:28 +0100 Subject: [PATCH 02/21] pyinstaller: use fixed AND freshly compiled bootloader, fixes #2002 --- Vagrantfile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index 0f1bcee2a..f0caae042 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -283,9 +283,10 @@ def install_pyinstaller(bootloader) . ~/.bash_profile cd /vagrant/borg . borg-env/bin/activate - git clone https://github.com/pyinstaller/pyinstaller.git + git clone https://github.com/thomaswaldmann/pyinstaller.git cd pyinstaller - git checkout v3.1.1 + # develop branch, with fixed / freshly rebuilt bootloaders + git checkout fresh-bootloader EOF if bootloader script += <<-EOF From 2b6e8a19e37636c093948cd16517378dc34a68ae Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 19 Jan 2017 18:58:14 +0100 Subject: [PATCH 03/21] use python 3.5.3 to build binaries, fixes #2078 --- Vagrantfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index f0caae042..1b9786839 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -226,7 +226,7 @@ def install_pythons(boxname) pyenv install 3.4.0 # tests pyenv install 3.5.0 # tests pyenv install 3.6.0 # tests - pyenv install 3.5.2 # binary build, use latest 3.5.x release + pyenv install 3.5.3 # binary build, use latest 3.5.x release pyenv rehash EOF end @@ -244,8 +244,8 @@ def build_pyenv_venv(boxname) . ~/.bash_profile cd /vagrant/borg # use the latest 3.5 release - pyenv global 3.5.2 - pyenv virtualenv 3.5.2 borg-env + pyenv global 3.5.3 + pyenv virtualenv 3.5.3 borg-env ln -s ~/.pyenv/versions/borg-env . EOF end From 7b9ff759606d9c5284a751cf668c0a18d147a6ee Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 19 Jan 2017 19:02:13 +0100 Subject: [PATCH 04/21] use osxfuse 3.5.4 for tests / to build binaries --- Vagrantfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index 1b9786839..63a31764b 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -65,9 +65,9 @@ def packages_darwin # install all the (security and other) updates sudo softwareupdate --install --all # get osxfuse 3.x release code from github: - curl -s -L https://github.com/osxfuse/osxfuse/releases/download/osxfuse-3.5.3/osxfuse-3.5.3.dmg >osxfuse.dmg + curl -s -L https://github.com/osxfuse/osxfuse/releases/download/osxfuse-3.5.4/osxfuse-3.5.4.dmg >osxfuse.dmg MOUNTDIR=$(echo `hdiutil mount osxfuse.dmg | tail -1 | awk '{$1="" ; print $0}'` | xargs -0 echo) \ - && sudo installer -pkg "${MOUNTDIR}/Extras/FUSE for macOS 3.5.3.pkg" -target / + && sudo installer -pkg "${MOUNTDIR}/Extras/FUSE for macOS 3.5.4.pkg" -target / sudo chown -R vagrant /usr/local # brew must be able to create stuff here ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)" brew update From 74c33463dcd01502cb3886ba59d4c7f4b337fe87 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 20 Jan 2017 02:59:36 +0100 Subject: [PATCH 05/21] vagrant freebsd: some fixes, fixes #2067 - use -RELEASE, it can be updated via binaries - more RAM, otherwise the 4 workers run out of memory. - do not install / use fakeroot, it seems broken. - set a hostname, this VM has none --- Vagrantfile | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index 63a31764b..d8721baf7 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -83,11 +83,13 @@ end def packages_freebsd return <<-EOF + # VM has no hostname set + hostname freebsd # install all the (security and other) updates, base system freebsd-update --not-running-from-cron fetch install # for building borgbackup and dependencies: pkg install -y openssl liblz4 fusefs-libs pkgconf - pkg install -y fakeroot git bash + pkg install -y git bash # for building python: pkg install -y sqlite3 # make bash default / work: @@ -468,11 +470,11 @@ Vagrant.configure(2) do |config| end # BSD - # note: the FreeBSD-10.3-STABLE box needs "vagrant up" twice to start. + # note: the FreeBSD-10.3-RELEASE box needs "vagrant up" twice to start. config.vm.define "freebsd64" do |b| - b.vm.box = "freebsd/FreeBSD-10.3-STABLE" + b.vm.box = "freebsd/FreeBSD-10.3-RELEASE" b.vm.provider :virtualbox do |v| - v.memory = 768 + v.memory = 1536 end b.ssh.shell = "sh" b.vm.provision "install system packages", :type => :shell, :inline => packages_freebsd From ddd9d77e5d2a2a7475ca51bcfbcc39129f5bf3b9 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 21 Jan 2017 05:36:56 +0100 Subject: [PATCH 06/21] Manifest.in: simplify, exclude *.{so,dll,orig}, fixes #2066 --- MANIFEST.in | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/MANIFEST.in b/MANIFEST.in index 8a4123fea..307ccc708 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,10 +1,8 @@ include README.rst AUTHORS LICENSE CHANGES.rst MANIFEST.in -recursive-include borg *.pyx -recursive-include docs * -recursive-exclude docs *.pyc -recursive-exclude docs *.pyo -prune docs/_build +exclude .coveragerc .gitattributes .gitignore .travis.yml Vagrantfile prune .travis prune .github -exclude .coveragerc .gitattributes .gitignore .travis.yml Vagrantfile -include borg/_version.py +graft borg +graft docs +prune docs/_build +global-exclude *.py[co] *.orig *.so *.dll From 90ae9076a4856e13110da56afb63228b7f8d3969 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sat, 21 Jan 2017 15:04:01 +0100 Subject: [PATCH 07/21] hashindex: detect mingw byte order --- borg/_hashindex.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/borg/_hashindex.c b/borg/_hashindex.c index bfa3ef09b..a86f12769 100644 --- a/borg/_hashindex.c +++ b/borg/_hashindex.c @@ -13,10 +13,12 @@ #endif #if (defined(BYTE_ORDER)&&(BYTE_ORDER == BIG_ENDIAN)) || \ + (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)) || \ (defined(_BIG_ENDIAN)&&defined(__SVR4)&&defined(__sun)) #define _le32toh(x) __builtin_bswap32(x) #define _htole32(x) __builtin_bswap32(x) #elif (defined(BYTE_ORDER)&&(BYTE_ORDER == LITTLE_ENDIAN)) || \ + (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)) || \ (defined(_LITTLE_ENDIAN)&&defined(__SVR4)&&defined(__sun)) #define _le32toh(x) (x) #define _htole32(x) (x) From fafd5e03997713ae9bec00368522f6698034e4b1 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sat, 21 Jan 2017 15:04:42 +0100 Subject: [PATCH 08/21] hashindex: separate endian-dependent defs from endian detection also make macro style consistent with other macros in the codebase. --- borg/_hashindex.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/borg/_hashindex.c b/borg/_hashindex.c index a86f12769..f0e8ba939 100644 --- a/borg/_hashindex.c +++ b/borg/_hashindex.c @@ -12,20 +12,26 @@ #include #endif -#if (defined(BYTE_ORDER)&&(BYTE_ORDER == BIG_ENDIAN)) || \ +#if (defined(BYTE_ORDER) && (BYTE_ORDER == BIG_ENDIAN)) || \ (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)) || \ - (defined(_BIG_ENDIAN)&&defined(__SVR4)&&defined(__sun)) -#define _le32toh(x) __builtin_bswap32(x) -#define _htole32(x) __builtin_bswap32(x) -#elif (defined(BYTE_ORDER)&&(BYTE_ORDER == LITTLE_ENDIAN)) || \ + (defined(_BIG_ENDIAN) && defined(__SVR4)&&defined(__sun)) +#define BORG_BIG_ENDIAN 1 +#elif (defined(BYTE_ORDER) && (BYTE_ORDER == LITTLE_ENDIAN)) || \ (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)) || \ - (defined(_LITTLE_ENDIAN)&&defined(__SVR4)&&defined(__sun)) -#define _le32toh(x) (x) -#define _htole32(x) (x) + (defined(_LITTLE_ENDIAN) && defined(__SVR4)&&defined(__sun)) +#define BORG_BIG_ENDIAN 0 #else #error Unknown byte order #endif +#if BORG_BIG_ENDIAN +#define _le32toh(x) __builtin_bswap32(x) +#define _htole32(x) __builtin_bswap32(x) +#else +#define _le32toh(x) (x) +#define _htole32(x) (x) +#endif + #define MAGIC "BORG_IDX" #define MAGIC_LEN 8 From d350e3a2e1447c0d3a4d4a1adee87b2825a5a345 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sun, 22 Jan 2017 02:21:26 +0100 Subject: [PATCH 09/21] create: don't create hard link refs to failed files --- borg/archive.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/borg/archive.py b/borg/archive.py index 26ad16455..4d436c860 100644 --- a/borg/archive.py +++ b/borg/archive.py @@ -660,8 +660,6 @@ Number of files: {0.stats.nfiles}'''.format( self.add_item(item) status = 'h' # regular file, hardlink (to already seen inodes) return status - else: - self.hard_links[st.st_ino, st.st_dev] = safe_path is_special_file = is_special(st.st_mode) if not is_special_file: path_hash = self.key.id_hash(os.path.join(self.cwd, path).encode('utf-8', 'surrogateescape')) @@ -709,6 +707,9 @@ Number of files: {0.stats.nfiles}'''.format( item[b'mode'] = stat.S_IFREG | stat.S_IMODE(item[b'mode']) self.stats.nfiles += 1 self.add_item(item) + if st.st_nlink > 1 and source is None: + # Add the hard link reference *after* the file has been added to the archive. + self.hard_links[st.st_ino, st.st_dev] = safe_path return status @staticmethod From fc8be58b636b1fbb8e21720990811e269ada8b8b Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 22 Jan 2017 16:54:06 +0100 Subject: [PATCH 10/21] SyncFile: fix use of fd object after close --- borg/repository.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/borg/repository.py b/borg/repository.py index dd49b368b..2cf000108 100644 --- a/borg/repository.py +++ b/borg/repository.py @@ -829,8 +829,9 @@ class LoggedIO: # tell the OS that it does not need to cache what we just wrote, # avoids spoiling the cache for the OS and other processes. os.posix_fadvise(self._write_fd.fileno(), 0, 0, os.POSIX_FADV_DONTNEED) + dirname = os.path.dirname(self._write_fd.name) self._write_fd.close() - sync_dir(os.path.dirname(self._write_fd.name)) + sync_dir(dirname) self._write_fd = None From 8fe047ec8d8281b1738d87bf548af194337074c3 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sun, 22 Jan 2017 02:29:54 +0100 Subject: [PATCH 11/21] mount: handle invalid hard link refs --- borg/fuse.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/borg/fuse.py b/borg/fuse.py index f92f76900..25c935229 100644 --- a/borg/fuse.py +++ b/borg/fuse.py @@ -128,15 +128,21 @@ class FuseOperations(llfuse.Operations): else: self.items[inode] = item continue - segments = prefix + os.fsencode(os.path.normpath(item[b'path'])).split(b'/') - del item[b'path'] + path = item.pop(b'path') + segments = prefix + os.fsencode(os.path.normpath(path)).split(b'/') num_segments = len(segments) parent = 1 for i, segment in enumerate(segments, 1): # Leaf segment? if i == num_segments: if b'source' in item and stat.S_ISREG(item[b'mode']): - inode = self._find_inode(item[b'source'], prefix) + try: + inode = self._find_inode(item[b'source'], prefix) + except KeyError: + file = path.decode(errors='surrogateescape') + source = item[b'source'].decode(errors='surrogateescape') + logger.warning('Skipping broken hard link: %s -> %s', file, source) + continue item = self.cache.get(inode) item[b'nlink'] = item.get(b'nlink', 1) + 1 self.items[inode] = item From 6996fa6dc0ba1feb762e6ec3a012cad2c219cc21 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 23 Jan 2017 20:47:33 +0100 Subject: [PATCH 12/21] creating a new segment: use "xb" mode, fixes #2099 "ab" seems to make no sense here (if there is already a (crap, but non-empty) segment file, we would write a MAGIC right into the middle of the resulting file) and cause #2099. --- borg/repository.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/borg/repository.py b/borg/repository.py index dd49b368b..058e894e3 100644 --- a/borg/repository.py +++ b/borg/repository.py @@ -670,7 +670,8 @@ class LoggedIO: if not os.path.exists(dirname): os.mkdir(dirname) sync_dir(os.path.join(self.path, 'data')) - self._write_fd = open(self.segment_filename(self.segment), 'ab') + # play safe: fail if file exists (do not overwrite existing contents, do not append) + self._write_fd = open(self.segment_filename(self.segment), 'xb') self._write_fd.write(MAGIC) self.offset = MAGIC_LEN return self._write_fd From fbaefc98c9842291578f54b55a4399b44dd0a109 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Fri, 27 Jan 2017 11:54:20 +0100 Subject: [PATCH 13/21] docs: add CVE numbers for issues fixed in 1.0.9 https://www.cvedetails.com/product/35461/Borg-Borg.html?vendor_id=16008 --- docs/changes.rst | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 24e360606..e0efe3f57 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -5,8 +5,8 @@ This section is used for infos about security and corruption issues. .. _tam_vuln: -Pre-1.0.9 manifest spoofing vulnerability ------------------------------------------ +Pre-1.0.9 manifest spoofing vulnerability (CVE-2016-10099) +---------------------------------------------------------- A flaw in the cryptographic authentication scheme in Borg allowed an attacker to spoof the manifest. The attack requires an attacker to be able to @@ -54,7 +54,9 @@ Vulnerability time line: * 2016-11-14: Vulnerability and fix discovered during review of cryptography by Marian Beermann (@enkore) * 2016-11-20: First patch -* 2016-12-18: Released fixed versions: 1.0.9, 1.1.0b3 +* 2016-12-20: Released fixed version 1.0.9 +* 2017-01-02: CVE was assigned +* 2017-01-15: Released fixed version 1.1.0b3 (fix was previously only available from source) .. _attic013_check_corruption: @@ -183,10 +185,14 @@ Security fixes: - A flaw in the cryptographic authentication scheme in Borg allowed an attacker to spoof the manifest. See :ref:`tam_vuln` above for the steps you should take. + + CVE-2016-10099 was assigned to this vulnerability. - borg check: When rebuilding the manifest (which should only be needed very rarely) duplicate archive names would be handled on a "first come first serve" basis, allowing an attacker to apparently replace archives. + CVE-2016-10100 was assigned to this vulnerability. + Bug fixes: - borg check: From 2cfaf03f840d67a0daa3b94ffb9572ff39b2a668 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Tue, 24 Jan 2017 14:18:25 +0100 Subject: [PATCH 14/21] mount: umount on SIGINT/^C when in foreground --- borg/archiver.py | 7 +++++++ borg/fuse.py | 4 +++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/borg/archiver.py b/borg/archiver.py index 667a8e90b..c2626beba 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -1546,6 +1546,13 @@ class Archiver: - allow_damaged_files: by default damaged files (where missing chunks were replaced with runs of zeros by borg check --repair) are not readable and return EIO (I/O error). Set this option to read such files. + + When the daemonized process receives a signal or crashes, it does not unmount. + Unmounting in these cases could cause an active rsync or similar process + to unintentionally delete data. + + When running in the foreground ^C/SIGINT unmounts cleanly, but other + signals or crashes do not. """) subparser = subparsers.add_parser('mount', parents=[common_parser], description=self.do_mount.__doc__, diff --git a/borg/fuse.py b/borg/fuse.py index f92f76900..710611004 100644 --- a/borg/fuse.py +++ b/borg/fuse.py @@ -6,6 +6,7 @@ import os import stat import tempfile import time +from signal import SIGINT from distutils.version import LooseVersion import msgpack @@ -98,7 +99,8 @@ class FuseOperations(llfuse.Operations): umount = False try: signal = fuse_main() - umount = (signal is None) # no crash and no signal -> umount request + # no crash and no signal (or it's ^C and we're in the foreground) -> umount request + umount = (signal is None or (signal == SIGINT and foreground)) finally: llfuse.close(umount) From f74b533d6dce141ebab200aafa864399b1f085fe Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 28 Jan 2017 14:44:42 +0100 Subject: [PATCH 15/21] vagrant: improve darwin64 VM settings somehow without these cpuid settings it does not work for everybody. also nice if we can get away without the extensions pack, which is proprietary. do not update iTunes, we just want the OS security / bugfix updates --- Vagrantfile | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Vagrantfile b/Vagrantfile index d8721baf7..f26066e79 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -63,6 +63,8 @@ end def packages_darwin return <<-EOF # install all the (security and other) updates + sudo softwareupdate --ignore iTunesX + sudo softwareupdate --ignore iTunes sudo softwareupdate --install --all # get osxfuse 3.x release code from github: curl -s -L https://github.com/osxfuse/osxfuse/releases/download/osxfuse-3.5.4/osxfuse-3.5.4.dmg >osxfuse.dmg @@ -458,6 +460,16 @@ Vagrant.configure(2) do |config| # OS X config.vm.define "darwin64" do |b| b.vm.box = "jhcook/yosemite-clitools" + b.vm.provider :virtualbox do |v| + v.customize ['modifyvm', :id, '--ostype', 'MacOS1010_64'] + v.customize ['modifyvm', :id, '--paravirtprovider', 'default'] + # Adjust CPU settings according to + # https://github.com/geerlingguy/macos-virtualbox-vm + v.customize ['modifyvm', :id, '--cpuidset', + '00000001', '000306a9', '00020800', '80000201', '178bfbff'] + # Disable USB variant requiring Virtualbox proprietary extension pack + v.customize ["modifyvm", :id, '--usbehci', 'off', '--usbxhci', 'off'] + end b.vm.provision "packages darwin", :type => :shell, :privileged => false, :inline => packages_darwin b.vm.provision "install pyenv", :type => :shell, :privileged => false, :inline => install_pyenv("darwin64") b.vm.provision "fix pyenv", :type => :shell, :privileged => false, :inline => fix_pyenv_darwin("darwin64") From 5a39d5c4f8a86a9c94449061c293eaaa0ce90bb8 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 24 Jan 2017 23:45:54 +0100 Subject: [PATCH 16/21] make LoggedIO.close_segment reentrant if anything blows up in the middle of a (first) invocation of close_segment() and an exception gets raised, it could happen that close_segment() gets called again (e.g. in Repository.__del__ or elsewhere). As the self._write_fd was set to None rather late, it would re-enter the if-block then. The new code gets the value of self._write_fd and also sets it to None in one tuple assignment, so re-entrance does not happen. Also, it uses try/finally to make sure the important parts (fd.close()) gets executed, even if there are exceptions in the other parts. --- borg/repository.py | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/borg/repository.py b/borg/repository.py index ddaf01439..4c7df2174 100644 --- a/borg/repository.py +++ b/borg/repository.py @@ -821,19 +821,25 @@ class LoggedIO: self.close_segment() # after-commit fsync() def close_segment(self): - if self._write_fd: - self.segment += 1 - self.offset = 0 - self._write_fd.flush() - os.fsync(self._write_fd.fileno()) - if hasattr(os, 'posix_fadvise'): # only on UNIX - # tell the OS that it does not need to cache what we just wrote, - # avoids spoiling the cache for the OS and other processes. - os.posix_fadvise(self._write_fd.fileno(), 0, 0, os.POSIX_FADV_DONTNEED) - dirname = os.path.dirname(self._write_fd.name) - self._write_fd.close() - sync_dir(dirname) - self._write_fd = None + # set self._write_fd to None early to guard against reentry from error handling code pathes: + fd, self._write_fd = self._write_fd, None + if fd is not None: + dirname = None + try: + self.segment += 1 + self.offset = 0 + dirname = os.path.dirname(fd.name) + fd.flush() + fileno = fd.fileno() + os.fsync(fileno) + if hasattr(os, 'posix_fadvise'): # only on UNIX + # tell the OS that it does not need to cache what we just wrote, + # avoids spoiling the cache for the OS and other processes. + os.posix_fadvise(fileno, 0, 0, os.POSIX_FADV_DONTNEED) + finally: + fd.close() + if dirname: + sync_dir(dirname) MAX_DATA_SIZE = MAX_OBJECT_SIZE - LoggedIO.put_header_fmt.size From add38e8cdeb4242a64e5939c764ff6a314d85b5b Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 27 Jan 2017 21:09:55 +0100 Subject: [PATCH 17/21] ignore posix_fadvise errors in repository.py, work around #2095 note: we also ignore the call's return value in _chunker.c. both is harmless as the call not working does not cause incorrect function, just worse performance due to constant flooding of the cache (as if we would not issue the call). --- borg/repository.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/borg/repository.py b/borg/repository.py index 4c7df2174..e85c33d6f 100644 --- a/borg/repository.py +++ b/borg/repository.py @@ -833,9 +833,19 @@ class LoggedIO: fileno = fd.fileno() os.fsync(fileno) if hasattr(os, 'posix_fadvise'): # only on UNIX - # tell the OS that it does not need to cache what we just wrote, - # avoids spoiling the cache for the OS and other processes. - os.posix_fadvise(fileno, 0, 0, os.POSIX_FADV_DONTNEED) + try: + # tell the OS that it does not need to cache what we just wrote, + # avoids spoiling the cache for the OS and other processes. + os.posix_fadvise(fileno, 0, 0, os.POSIX_FADV_DONTNEED) + except OSError: + # usually, posix_fadvise can't fail for us, but there seem to + # be failures when running borg under docker on ARM, likely due + # to a bug outside of borg. + # also, there is a python wrapper bug, always giving errno = 0. + # https://github.com/borgbackup/borg/issues/2095 + # as this call is not critical for correct function (just to + # optimize cache usage), we ignore these errors. + pass finally: fd.close() if dirname: From dc3492642dfd987b37ef6e80aca7f657c2aee070 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 21 Jan 2017 06:09:25 +0100 Subject: [PATCH 18/21] update CHANGES (1.0-maint) --- docs/changes.rst | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index e0efe3f57..129d3e2d9 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -128,8 +128,8 @@ The best check that everything is ok is to run a dry-run extraction:: Changelog ========= -Version 1.0.10rc1 (not released yet) ------------------------------------- +Version 1.0.10rc1 (2017-01-29) +------------------------------ Bug fixes: @@ -144,9 +144,16 @@ Bug fixes: - Fixed change-passphrase crashing with unencrypted repositories, #1978 - Fixed "borg check repo::archive" indicating success if "archive" does not exist, #1997 - borg check: print non-exit-code warning if --last or --prefix aren't fulfilled +- fix bad parsing of wrong repo location syntax +- create: don't create hard link refs to failed files, + mount: handle invalid hard link refs, #2092 +- detect mingw byte order, #2073 +- creating a new segment: use "xb" mode, #2099 +- mount: umount on SIGINT/^C when in foreground, #2082 Other changes: +- binary: use fixed AND freshly compiled pyinstaller bootloader, #2002 - xattr: ignore empty names returned by llistxattr(2) et al - Enable the fault handler: install handlers for the SIGSEGV, SIGFPE, SIGABRT, SIGBUS and SIGILL signals to dump the Python traceback. @@ -156,8 +163,11 @@ Other changes: - tests: - vagrant / travis / tox: add Python 3.6 based testing - - vagrant: fix openbsd repo, fixes #2042 - - vagrant: fix the freebsd64 machine, #2037 + - vagrant: fix openbsd repo, #2042 + - vagrant: fix the freebsd64 machine, #2037 #2067 + - vagrant: use python 3.5.3 to build binaries, #2078 + - vagrant: use osxfuse 3.5.4 for tests / to build binaries + vagrant: improve darwin64 VM settings - travis: fix osxfuse install (fixes OS X testing on Travis CI) - travis: require succeeding OS X tests, #2028 - travis: use latest pythons for OS X based testing @@ -169,12 +179,18 @@ Other changes: - language clarification - VM backup FAQ - borg create: document how to backup stdin, #2013 - borg upgrade: fix incorrect title levels + - add CVE numbers for issues fixed in 1.0.9, #2106 - fix typos (taken from Debian package patch) - remote: include data hexdump in "unexpected RPC data" error message - remote: log SSH command line at debug level - API_VERSION: use numberspaces, #2023 - remove .github from pypi package, #2051 - add pip and setuptools to requirements file, #2030 +- SyncFile: fix use of fd object after close (cosmetic) +- Manifest.in: simplify, exclude *.{so,dll,orig}, #2066 +- ignore posix_fadvise errors in repository.py, #2095 + (works around issues with docker on ARM) +- make LoggedIO.close_segment reentrant, avoid reentrance Version 1.0.9 (2016-12-20) From e6c1931d476de51010dc5bd482bf1e591eb1ebba Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 28 Jan 2017 23:36:56 +0100 Subject: [PATCH 19/21] ran build_usage --- docs/usage/create.rst.inc | 8 ++++++-- docs/usage/mount.rst.inc | 9 ++++++++- docs/usage/upgrade.rst.inc | 4 ++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/docs/usage/create.rst.inc b/docs/usage/create.rst.inc index 56133ef7e..a911252ca 100644 --- a/docs/usage/create.rst.inc +++ b/docs/usage/create.rst.inc @@ -84,8 +84,12 @@ Description ~~~~~~~~~~~ This command creates a backup archive containing all files found while recursively -traversing all paths specified. The archive will consume almost no disk space for -files or parts of files that have already been stored in other archives. +traversing all paths specified. When giving '-' as path, borg will read data +from standard input and create a file 'stdin' in the created archive from that +data. + +The archive will consume almost no disk space for files or parts of files that +have already been stored in other archives. The archive name needs to be unique. It must not end in '.checkpoint' or '.checkpoint.N' (with N being a number), because these names are used for diff --git a/docs/usage/mount.rst.inc b/docs/usage/mount.rst.inc index 6deef307b..898d9c5b4 100644 --- a/docs/usage/mount.rst.inc +++ b/docs/usage/mount.rst.inc @@ -11,7 +11,7 @@ borg mount [--remote-path PATH] [-f] [-o OPTIONS] REPOSITORY_OR_ARCHIVE MOUNTPOINT - Mount archive or an entire repository as a FUSE fileystem + Mount archive or an entire repository as a FUSE filesystem positional arguments: REPOSITORY_OR_ARCHIVE @@ -54,3 +54,10 @@ supported by borg: - allow_damaged_files: by default damaged files (where missing chunks were replaced with runs of zeros by borg check --repair) are not readable and return EIO (I/O error). Set this option to read such files. + +When the daemonized process receives a signal or crashes, it does not unmount. +Unmounting in these cases could cause an active rsync or similar process +to unintentionally delete data. + +When running in the foreground ^C/SIGINT unmounts cleanly, but other +signals or crashes do not. diff --git a/docs/usage/upgrade.rst.inc b/docs/usage/upgrade.rst.inc index b4793c2cc..b566cf118 100644 --- a/docs/usage/upgrade.rst.inc +++ b/docs/usage/upgrade.rst.inc @@ -46,7 +46,7 @@ Description Upgrade an existing Borg repository. Borg 1.x.y upgrades -------------------- ++++++++++++++++++++ Use ``borg upgrade --tam REPO`` to require manifest authentication introduced with Borg 1.0.9 to address security issues. This means @@ -68,7 +68,7 @@ https://borgbackup.readthedocs.io/en/stable/changes.html#pre-1-0-9-manifest-spoo for details. Attic and Borg 0.xx to Borg 1.x -------------------------------- ++++++++++++++++++++++++++++++++ This currently supports converting an Attic repository to Borg and also helps with converting Borg 0.xx to 1.0. From e19537ff6ff1ad21d3fcc3ad7ea47eb36687e593 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 29 Jan 2017 05:54:25 +0100 Subject: [PATCH 20/21] ran build_usage --- docs/usage/mount.rst.inc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/usage/mount.rst.inc b/docs/usage/mount.rst.inc index ee63cb42b..e15f25afa 100644 --- a/docs/usage/mount.rst.inc +++ b/docs/usage/mount.rst.inc @@ -61,3 +61,10 @@ The BORG_MOUNT_DATA_CACHE_ENTRIES environment variable is meant for advanced use to tweak the performance. It sets the number of cached data chunks; additional memory usage can be up to ~8 MiB times this number. The default is the number of CPU cores. + +When the daemonized process receives a signal or crashes, it does not unmount. +Unmounting in these cases could cause an active rsync or similar process +to unintentionally delete data. + +When running in the foreground ^C/SIGINT unmounts cleanly, but other +signals or crashes do not. From 7f2a108c949116c6d3a3edb6ebceee8126b788a5 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Mon, 30 Jan 2017 03:11:42 +0100 Subject: [PATCH 21/21] fixup: do not access os.POSIX_FADV_* early before we know posix_fadvise support exists on the platform. --- src/borg/platform/base.py | 3 ++- src/borg/platform/linux.pyx | 4 ++-- src/borg/repository.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/borg/platform/base.py b/src/borg/platform/base.py index 06d1c2723..0d2fb51b8 100644 --- a/src/borg/platform/base.py +++ b/src/borg/platform/base.py @@ -65,6 +65,7 @@ def sync_dir(path): def safe_fadvise(fd, offset, len, advice): if hasattr(os, 'posix_fadvise'): + advice = getattr(os, 'POSIX_FADV_' + advice) try: os.posix_fadvise(fd, offset, len, advice) except OSError: @@ -120,7 +121,7 @@ class SyncFile: platform.fdatasync(self.fileno) # tell the OS that it does not need to cache what we just wrote, # avoids spoiling the cache for the OS and other processes. - safe_fadvise(self.fileno, 0, 0, os.POSIX_FADV_DONTNEED) + safe_fadvise(self.fileno, 0, 0, 'DONTNEED') def close(self): """sync() and close.""" diff --git a/src/borg/platform/linux.pyx b/src/borg/platform/linux.pyx index 3658e4179..e87983c7d 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -217,7 +217,7 @@ cdef _sync_file_range(fd, offset, length, flags): assert length & PAGE_MASK == 0, "length %d not page-aligned" % length if sync_file_range(fd, offset, length, flags) != 0: raise OSError(errno.errno, os.strerror(errno.errno)) - safe_fadvise(fd, offset, length, os.POSIX_FADV_DONTNEED) + safe_fadvise(fd, offset, length, 'DONTNEED') cdef unsigned PAGE_MASK = resource.getpagesize() - 1 @@ -254,7 +254,7 @@ class SyncFile(BaseSyncFile): os.fdatasync(self.fileno) # tell the OS that it does not need to cache what we just wrote, # avoids spoiling the cache for the OS and other processes. - safe_fadvise(self.fileno, 0, 0, os.POSIX_FADV_DONTNEED) + safe_fadvise(self.fileno, 0, 0, 'DONTNEED') def umount(mountpoint): diff --git a/src/borg/repository.py b/src/borg/repository.py index 142e44e9c..9d4d604dd 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -909,7 +909,7 @@ class LoggedIO: self.fds = None # Just to make sure we're disabled def close_fd(self, fd): - safe_fadvise(fd.fileno(), 0, 0, os.POSIX_FADV_DONTNEED) + safe_fadvise(fd.fileno(), 0, 0, 'DONTNEED') fd.close() def segment_iterator(self, segment=None, reverse=False):