Questions about smbd option "strict rename"
Ralph Boehme
rb at sernet.de
Fri Nov 27 17:52:59 UTC 2015
On Wed, Nov 25, 2015 at 09:31:14AM -0800, Jeremy Allison wrote:
> On Wed, Nov 25, 2015 at 06:15:19PM +0100, Ralph Boehme wrote:
> > On Wed, Nov 25, 2015 at 08:57:52AM -0800, Jeremy Allison wrote:
> > > On Wed, Nov 25, 2015 at 03:50:25PM +0100, Ralph Boehme wrote:
> > > > On Tue, Nov 24, 2015 at 10:01:11AM -0800, Jeremy Allison wrote:
> > > > > On Tue, Nov 24, 2015 at 06:50:47PM +0100, Ralph Boehme wrote:
> > > > > > Jeremy, how to proceed wrt that attaching POSIX rename behaviour to
> > > > > > POSIX pathnames is wrong imo. We need a seperate flag for this.
> > > > >
> > > > > I don't think it is wrong. We have behaved that way
> > > > > for a *very* long time and in the same way we
> > > > > attach POSIX delete behavior to POSIX pathnames
> > > > > too. Changing that will break UNIX extensions.
> > > >
> > > > sorry if you got the impression I would want to change semantics, I
> > > > don't! I merely want to tweak the internal handling by allowing more
> > > > fine grained control *without* changing existing CIFS UNIX extensions.
> > > >
> > > > > Now you might want an additional flag in order to
> > > > > get POSIX-rename (and maybe POSIX-delete) behavior
> > > > > for MacOSX clients that don't currently negotiate
> > > > > UNIX extensions, but that's different from changing
> > > > > the current smbd behavior.
> > > >
> > > > Again, I don't want to change existing behaviour.
> > > >
> > > > Maybe the attached patch makes clearer what I have in mind.
> > >
> > > Oh, that makes much more sense, thanks !
> >
> > :)
> >
> > > I still have adding fields into files_struct
> > > as a kitchen sink for underlying behavior.
> >
> > Maybe add a uint64_t for flags, ie "uint64_t posix_flags" ? Just
> > thinking loud.
>
> Oh that's a nice idea. I think for the specific
> issue at hand (OSX clients wanting some but
> not all POSIX behavior) that's a perfect
> solution.
>
> When POSIX_OPEN is passed in, we set all
> flags to ge the current behavior and then
> the OSX client can unset (or set) flags
> at will inside the fruit VFS.
patch attached. What do you think?
I'm not fully happy with the flag names yet, I think we should prefix
them with VFS_ so they won't collide with flags we may add later on
for SMB2/3 UNIX extensions.
-Ralph
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From 8ee18ff52320bfc802c42564d8280a4d2b91ab31 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 27 Nov 2015 18:29:55 +0100
Subject: [PATCH 1/4] s3:smbd: convert file_struct.posix_open to a bitmap with
flags
This is in preperation of a more fine grained control of POSIX behaviour
in the SMB and VFS layers.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11065
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/include/vfs.h | 7 ++++++-
source3/locking/locking.c | 3 ++-
source3/modules/vfs_acl_common.c | 4 ++--
source3/param/loadparm.c | 3 ++-
source3/smbd/close.c | 6 ++++--
source3/smbd/fileio.c | 8 ++++----
source3/smbd/open.c | 10 ++++++----
source3/smbd/reply.c | 2 +-
source3/smbd/smb2_close.c | 2 +-
source3/smbd/vfs.c | 4 ++--
10 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 9945375..3e01e52 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -255,7 +255,7 @@ typedef struct files_struct {
bool aio_write_behind;
bool initial_delete_on_close; /* Only set at NTCreateX if file was created. */
bool delete_on_close;
- bool posix_open;
+ uint64_t posix_flags;
bool is_sparse;
bool backup_intent; /* Handle was successfully opened with backup intent
and opener has privilege to do so. */
@@ -297,6 +297,11 @@ typedef struct files_struct {
struct tevent_req *deferred_close;
} files_struct;
+#define POSIX_FLAGS_OPEN UINT64_C(0x00000001)
+
+#define POSIX_FLAGS_ALL \
+ (POSIX_FLAGS_OPEN)
+
struct vuid_cache_entry {
struct auth_session_info *session_info;
uint64_t vuid; /* SMB2 compat */
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 7b8bc84..3b8276c 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -847,7 +847,8 @@ bool set_share_mode(struct share_mode_lock *lck, struct files_struct *fsp,
e->id = fsp->file_id;
e->share_file_id = fsp->fh->gen_id;
e->uid = (uint32_t)uid;
- e->flags = fsp->posix_open ? SHARE_MODE_FLAG_POSIX_OPEN : 0;
+ e->flags = (fsp->posix_flags & POSIX_FLAGS_OPEN) ?
+ SHARE_MODE_FLAG_POSIX_OPEN : 0;
e->name_hash = fsp->name_hash;
return true;
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 625e7cb..532464e 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -1089,7 +1089,7 @@ static int chmod_acl_module_common(struct vfs_handle_struct *handle,
static int fchmod_acl_module_common(struct vfs_handle_struct *handle,
struct files_struct *fsp, mode_t mode)
{
- if (fsp->posix_open) {
+ if (fsp->posix_flags & POSIX_FLAGS_OPEN) {
/* Only allow this on POSIX opens. */
return SMB_VFS_NEXT_FCHMOD(handle, fsp, mode);
}
@@ -1109,7 +1109,7 @@ static int chmod_acl_acl_module_common(struct vfs_handle_struct *handle,
static int fchmod_acl_acl_module_common(struct vfs_handle_struct *handle,
struct files_struct *fsp, mode_t mode)
{
- if (fsp->posix_open) {
+ if (fsp->posix_flags & POSIX_FLAGS_OPEN) {
/* Only allow this on POSIX opens. */
return SMB_VFS_NEXT_FCHMOD_ACL(handle, fsp, mode);
}
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 9f40e65..be0dd16 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -4316,7 +4316,8 @@ enum brl_flavour lp_posix_cifsu_locktype(files_struct *fsp)
if (posix_default_lock_was_set) {
return posix_cifsx_locktype;
} else {
- return fsp->posix_open ? POSIX_LOCK : WINDOWS_LOCK;
+ return (fsp->posix_flags & POSIX_FLAGS_OPEN) ?
+ POSIX_LOCK : WINDOWS_LOCK;
}
}
diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index 0e75bf0..d77941b 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -327,7 +327,7 @@ static NTSTATUS close_remove_share_mode(files_struct *fsp,
if (e->name_hash != fsp->name_hash) {
continue;
}
- if (fsp->posix_open
+ if ((fsp->posix_flags & POSIX_FLAGS_OPEN)
&& (e->flags & SHARE_MODE_FLAG_POSIX_OPEN)) {
continue;
}
@@ -1103,7 +1103,9 @@ static NTSTATUS close_directory(struct smb_request *req, files_struct *fsp,
struct share_mode_entry *e = &lck->data->share_modes[i];
if (is_valid_share_mode_entry(e) &&
e->name_hash == fsp->name_hash) {
- if (fsp->posix_open && (e->flags & SHARE_MODE_FLAG_POSIX_OPEN)) {
+ if ((fsp->posix_flags & POSIX_FLAGS_OPEN) &&
+ (e->flags & SHARE_MODE_FLAG_POSIX_OPEN))
+ {
continue;
}
if (serverid_equal(&self, &e->pid) &&
diff --git a/source3/smbd/fileio.c b/source3/smbd/fileio.c
index 2fe0432..2e8c30b 100644
--- a/source3/smbd/fileio.c
+++ b/source3/smbd/fileio.c
@@ -193,7 +193,7 @@ void trigger_write_time_update(struct files_struct *fsp)
{
int delay;
- if (fsp->posix_open) {
+ if (fsp->posix_flags & POSIX_FLAGS_OPEN) {
/* Don't use delayed writes on POSIX files. */
return;
}
@@ -238,7 +238,7 @@ void trigger_write_time_update_immediate(struct files_struct *fsp)
{
struct smb_file_time ft;
- if (fsp->posix_open) {
+ if (fsp->posix_flags & POSIX_FLAGS_OPEN) {
/* Don't use delayed writes on POSIX files. */
return;
}
@@ -284,7 +284,7 @@ void mark_file_modified(files_struct *fsp)
}
trigger_write_time_update(fsp);
- if (fsp->posix_open) {
+ if (fsp->posix_flags & POSIX_FLAGS_OPEN) {
return;
}
if (!(lp_store_dos_attributes(SNUM(fsp->conn)) ||
@@ -1047,7 +1047,7 @@ NTSTATUS sync_file(connection_struct *conn, files_struct *fsp, bool write_throug
int fsp_stat(files_struct *fsp)
{
if (fsp->fh->fd == -1) {
- if (fsp->posix_open) {
+ if (fsp->posix_flags & POSIX_FLAGS_OPEN) {
return SMB_VFS_LSTAT(fsp->conn, fsp->fsp_name);
} else {
return SMB_VFS_STAT(fsp->conn, fsp->fsp_name);
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index c34742e..f682fd5 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -362,7 +362,7 @@ NTSTATUS fd_open(struct connection_struct *conn,
* client should be doing this.
*/
- if (fsp->posix_open || !lp_follow_symlinks(SNUM(conn))) {
+ if ((fsp->posix_flags & POSIX_FLAGS_OPEN) || !lp_follow_symlinks(SNUM(conn))) {
flags |= O_NOFOLLOW;
}
#endif
@@ -945,7 +945,7 @@ static NTSTATUS open_file(files_struct *fsp,
access_mask);
if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) &&
- fsp->posix_open &&
+ (fsp->posix_flags & POSIX_FLAGS_OPEN) &&
S_ISLNK(smb_fname->st.st_ex_mode)) {
/* This is a POSIX stat open for delete
* or rename on a symlink that points
@@ -2703,7 +2703,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
fsp->access_mask = open_access_mask; /* We change this to the
* requested access_mask after
* the open is done. */
- fsp->posix_open = posix_open;
+ fsp->posix_flags |= posix_open ? POSIX_FLAGS_OPEN : 0;
if (timeval_is_zero(&request_time)) {
request_time = fsp->open_time;
@@ -3569,7 +3569,9 @@ static NTSTATUS open_directory(connection_struct *conn,
fsp->oplock_type = NO_OPLOCK;
fsp->sent_oplock_break = NO_BREAK_SENT;
fsp->is_directory = True;
- fsp->posix_open = (file_attributes & FILE_FLAG_POSIX_SEMANTICS) ? True : False;
+ if (file_attributes & FILE_FLAG_POSIX_SEMANTICS) {
+ fsp->posix_flags |= POSIX_FLAGS_ALL;
+ }
status = fsp_set_smb_fname(fsp, smb_dname);
if (!NT_STATUS_IS_OK(status)) {
file_free(req, fsp);
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index efef613..ed3bbd7 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -2669,7 +2669,7 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp,
}
if (S_ISDIR(fsp->fsp_name->st.st_ex_mode)) {
- if (fsp->posix_open) {
+ if (fsp->posix_flags & POSIX_FLAGS_OPEN) {
return NT_STATUS_OK;
}
diff --git a/source3/smbd/smb2_close.c b/source3/smbd/smb2_close.c
index ed53e1b..875bc23 100644
--- a/source3/smbd/smb2_close.c
+++ b/source3/smbd/smb2_close.c
@@ -231,7 +231,7 @@ static NTSTATUS smbd_smb2_close(struct smbd_smb2_request *req,
return NT_STATUS_NO_MEMORY;
}
- posix_open = fsp->posix_open;
+ posix_open = (fsp->posix_flags & POSIX_FLAGS_OPEN);
smb_fname = cp_smb_filename(talloc_tos(), fsp->fsp_name);
if (smb_fname == NULL) {
return NT_STATUS_NO_MEMORY;
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 9f3ba6d..991ffdb 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -1309,7 +1309,7 @@ NTSTATUS vfs_stat_fsp(files_struct *fsp)
int ret;
if(fsp->fh->fd == -1) {
- if (fsp->posix_open) {
+ if (fsp->posix_flags & POSIX_FLAGS_OPEN) {
ret = SMB_VFS_LSTAT(fsp->conn, fsp->fsp_name);
} else {
ret = SMB_VFS_STAT(fsp->conn, fsp->fsp_name);
@@ -1939,7 +1939,7 @@ NTSTATUS vfs_chown_fsp(files_struct *fsp, uid_t uid, gid_t gid)
path = fsp->fsp_name->base_name;
}
- if (fsp->posix_open || as_root) {
+ if ((fsp->posix_flags & POSIX_FLAGS_OPEN) || as_root) {
ret = SMB_VFS_LCHOWN(fsp->conn,
path,
uid, gid);
--
2.5.0
From 711b32047111c78c039de2a58e0423968f4bb2b2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 22 Jan 2015 10:00:15 +0100
Subject: [PATCH 2/4] s3:smbd: file_struct: seperate POSIX directory rename cap
from posix_open
We need more fine grained control over which POSIX semantics we'd like
to enable per file handle. Currently POSIX_FLAGS_OPEN is a kitchensink
for all kinds of stuff like:
- POSIX unlink
- POSIX byte-range locks
- POSIX rename
- delayed writetime update
- more...
This is in preperation of SMB2 UNIX extensions, where we may want to be
more fine grained then in the CIFS UNIX extensions, and for OS X clients
with AAPL.
For CIFS UNIX extensions we use POSIX_FLAGS_ALL so semantics are
preserved.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11065
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/include/vfs.h | 4 +++-
source3/smbd/reply.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 3e01e52..de44c44 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -298,9 +298,11 @@ typedef struct files_struct {
} files_struct;
#define POSIX_FLAGS_OPEN UINT64_C(0x00000001)
+#define POSIX_FLAGS_RENAME UINT64_C(0x00000002)
#define POSIX_FLAGS_ALL \
- (POSIX_FLAGS_OPEN)
+ (POSIX_FLAGS_OPEN | \
+ POSIX_FLAGS_RENAME)
struct vuid_cache_entry {
struct auth_session_info *session_info;
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index ed3bbd7..b0c3981 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -2669,7 +2669,7 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp,
}
if (S_ISDIR(fsp->fsp_name->st.st_ex_mode)) {
- if (fsp->posix_flags & POSIX_FLAGS_OPEN) {
+ if (fsp->posix_flags & POSIX_FLAGS_RENAME) {
return NT_STATUS_OK;
}
--
2.5.0
From f1b7bdde9b3e1f53670c77976e54899e69013cb9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 25 Nov 2015 09:12:55 +0100
Subject: [PATCH 3/4] vfs_fruit: add a flag that tracks whether use of AAPL was
negotiated
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11065
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_fruit.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index bd71ff1..a80015f 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -125,7 +125,8 @@ struct fruit_config_data {
enum fruit_meta meta;
enum fruit_locking locking;
enum fruit_encoding encoding;
- bool use_aapl;
+ bool use_aapl; /* config from smb.conf */
+ bool nego_aapl; /* client negotiated AAPL */
bool use_copyfile;
bool readdir_attr_enabled;
bool unix_info_enabled;
@@ -3386,6 +3387,10 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle,
if (!NT_STATUS_IS_OK(status)) {
return status;
}
+ if (!config->nego_aapl) {
+ return status;
+ }
+
fsp = *result;
if (config->copyfile_enabled) {
--
2.5.0
From dc424def2c3aa8b2343c8b970aa78218baca082b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 22 Jan 2015 10:07:56 +0100
Subject: [PATCH 4/4] vfs_fruit: enable POSIX directory rename semantics
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11065
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_fruit.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index a80015f..74677bd 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -3419,6 +3419,14 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle,
}
}
+ if (fsp->is_directory) {
+ /*
+ * Enable POSIX directory rename behaviour
+ */
+ fsp->posix_flags |= POSIX_FLAGS_RENAME;
+ return status;
+ }
+
if (is_ntfs_stream_smb_fname(smb_fname)
|| fsp->is_directory) {
return status;
--
2.5.0
More information about the samba-technical
mailing list