[PATCH] Bunch of options for vfs_fileid

[Ralph Böhme]

Hi Christian,

On Sat, Jan 06, 2018 at 08:53:01AM +0100, Christian Ambach wrote:
> > +	char hostname[HOST_NAME_MAX+1];
> [...]
> > +
> > +	rc = gethostname(hostname, HOST_NAME_MAX+1);
> > +	if (rc != 0) {
> > +		DBG_ERR("gethostname failed\n");
> > +		return UINT64_MAX;
> > +	}
> man gethostname on Linux notes a discrepancy between Linux and POSIX:
> in POSIX gethostname is allowed to return up to 255 characters, while
> HOST_NAME_MAX is just 64 characters on Linux. Not sure how this looks
> like on other platforms. Maybe it makes more sense to use an array of
> size 256 to be on the safe side?

Why? As long as the array size matches the size passed to gethostname()
everything's fine.

> The error path that returns the same number for all devices might be
> problematic too, as it will then generate the same fileid for files
> with the same inode number on different filesystems.

Yeah, that's a bit problematic in theory. In practice, afaict gethostname() will
only fail if the passed in buffer size is too small, but this can't happen when
using HOST_NAME_MAX.

> > +	TALLOC_FREE(devname);
> > +
> > +	id = fileid_uint64_hash((uint8_t *)devname, devname_len);
> 
> Isn't this a use-after-free?

You bet it is. Damn! Thanks for catching this. Patch attached.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
-------------- next part --------------
From e05f315cb4fd93ec2924a0d528f90bc601445a8e Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 6 Jan 2018 16:13:52 +0100
Subject: [PATCH] vfs_fileid: fix a use after free

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fileid.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source3/modules/vfs_fileid.c b/source3/modules/vfs_fileid.c
index 98cc32d62d5..c890876c998 100644
--- a/source3/modules/vfs_fileid.c
+++ b/source3/modules/vfs_fileid.c
@@ -233,9 +233,11 @@ static uint64_t fileid_device_mapping_hostname(struct fileid_handle_data *data,
 		return UINT64_MAX;
 	}
 	devname_len = talloc_array_length(devname) - 1;
-	TALLOC_FREE(devname);
 
 	id = fileid_uint64_hash((uint8_t *)devname, devname_len);
+
+	TALLOC_FREE(devname);
+
 	return id;
 }
 
-- 
2.13.6