receive: strip root subvol path during process_clone
diff mbox

Message ID 20160614225541.18140-1-benedikt.morbach@googlemail.com
State New
Headers show

Commit Message

Benedikt Morbach June 14, 2016, 10:55 p.m. UTC
otherwise we get

    ERROR: cannot open <subvol_path>: No such file or directory

because <full_root_path>/<subvol_path> doesn't exist, so openat() will fail below.

Signed-off-by: Benedikt Morbach <benedikt.morbach@googlemail.com>
---
 cmds-receive.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Benedikt Morbach June 14, 2016, 11:06 p.m. UTC | #1
Hi all,

this fixes http://thread.gmane.org/gmane.comp.file-systems.btrfs/56902 
for me.
I got to this via gdb + good old debug printf and tbh I'm not entirely
clear about the semantics of process_clone, so some error handling
might be missing here?

Cheers
Benedikt

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba June 17, 2016, 5:18 p.m. UTC | #2
Hi,

On Wed, Jun 15, 2016 at 01:06:46AM +0200, Benedikt Morbach wrote:
> this fixes http://thread.gmane.org/gmane.comp.file-systems.btrfs/56902 
> for me.
> I got to this via gdb + good old debug printf and tbh I'm not entirely
> clear about the semantics of process_clone, so some error handling
> might be missing here?

thanks for the fix. The mail thread holds enough information to
reproduce the issue, I'll get to that on Monday. I'm not sure about the
use of strstr to match the subvolume name so I want to write the tests
first and craft some paths to see how it works.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benedikt Morbach July 11, 2016, 2:07 p.m. UTC | #3
Hi David,

On Fri, Jun 17, 2016 at 7:18 PM, David Sterba <dsterba@suse.cz> wrote:
> thanks for the fix. The mail thread holds enough information to
> reproduce the issue, I'll get to that on Monday. I'm not sure about the
> use of strstr to match the subvolume name so I want to write the tests
> first and craft some paths to see how it works.

any update on this? I've been running this patch for a few weeks now
with my hourly backups.
I could also take a stab at writing a test case but I think I'd need
some pointers for that. ;-)

I took a look at the send/receive patches that Filipe sent earlier and
afaict this isn't fixed in those.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba July 13, 2016, 3:46 p.m. UTC | #4
On Mon, Jul 11, 2016 at 04:07:25PM +0200, Benedikt Morbach wrote:
> Hi David,
> 
> On Fri, Jun 17, 2016 at 7:18 PM, David Sterba <dsterba@suse.cz> wrote:
> > thanks for the fix. The mail thread holds enough information to
> > reproduce the issue, I'll get to that on Monday. I'm not sure about the
> > use of strstr to match the subvolume name so I want to write the tests
> > first and craft some paths to see how it works.
> 
> any update on this? I've been running this patch for a few weeks now
> with my hourly backups.
> I could also take a stab at writing a test case but I think I'd need
> some pointers for that. ;-)

Sorry, I got distracted. The things with paths are a bit complicated,
there's chroot support, explicit mountpoint (-m), so stripping the
root_path would not work in all cases.

Apparently the full_clone_path is wrong there as it's not relative to
root_path and thus openat(mnt_fd, ...) will fail, so instead of
stripping the path, we would need to provide the clone path relative to
root_path directly.

> I took a look at the send/receive patches that Filipe sent earlier and
> afaict this isn't fixed in those.

No, this is not related to kernel.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/cmds-receive.c b/cmds-receive.c
index f4a3a4f..a975fdd 100644
--- a/cmds-receive.c
+++ b/cmds-receive.c
@@ -753,6 +753,17 @@  static int process_clone(const char *path, u64 offset, u64 len,
 		subvol_path = strdup(si->path);
 	}
 
+	/* strip the subvolume that we are receiving to from the start of subvol_path */
+	if (r->full_root_path &&
+		strstr(subvol_path, r->full_root_path) == subvol_path) {
+		size_t root_len = strlen(r->full_root_path);
+		size_t sub_len = strlen(subvol_path);
+
+		memmove(subvol_path,
+			subvol_path + root_len + 1,
+			sub_len - root_len);
+	}
+
 	ret = path_cat_out(full_clone_path, subvol_path, clone_path);
 	if (ret < 0) {
 		error("clone: target path invalid: %s", clone_path);