Current SMB2 leases patchset (WIP).
Stefan (metze) Metzmacher
metze at samba.org
Thu Oct 30 09:15:03 MDT 2014
Hi Jeremy,
here's a first independent patchset set that can go to master now.
Mostly the strict rename patches.
The rest of the patches is in the following branch:
https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-leases
I'll post more review details on the rest once I looked at it more closely.
metze
Am 29.10.2014 um 04:30 schrieb Jeremy Allison:
> On Tue, Oct 28, 2014 at 04:14:13PM -0700, Jeremy Allison wrote:
>>
>> Scratch that previous patchset/wip. I missed a couple of UINT16_MAX -> UINT32_MAX
>> changes.
>>
>> This is a corrected one with lease_idx now uint32_t.
>
> Discovered the root cause of the oplock problem
> that causes the samba3.smb2.oplock test to fail.
>
> It's the code added in:
>
> "s3: leases - oplock break errors"
> commit 0626052a4fdfe03f9fb137b9630b5111b129ef7b.
>
> Which I initially put as part of the patchset,
> but probably shouldn't have.
>
> Volker told me this is code created whilst he was
> at Redmond to get closer to passing the Microsoft
> oplock torture tests - and in this case what
> they're testing is if we give the correct error
> message when a client responds to a break message
> when it shouldn't. So the test is failing because
> we respond NT_STATUS_INVALID_DEVICE_STATE instead
> of what the test expects, NT_STATUS_INVALID_OPLOCK_PROTOCOL.
>
> Big whoop, just not important - we're sending an
> error message in the insane client case, and that's
> what matters. We can work on passing the MS test
> cases later as it's only testing returning a specific
> error code when the client does something insane, this
> isn't a blocking factor in getting the leases code in.
>
> So here is the patchset minus the "s3: leases - oplock break errors"
> change. I'll test tomorrow that it passes a full
> make test (late here tonight).
>
> Cheers,
>
> Jeremy
>
-------------- next part --------------
From 1c489458fafe26fc2ee7f34772a83ff31a06e05f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 23 Sep 2014 05:18:54 +0200
Subject: [PATCH 1/8] s3:locking: Rename share_mode_forall->share_entry_forall
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
source3/locking/proto.h | 4 ++--
source3/locking/share_mode_lock.c | 4 ++--
source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 8 ++++----
source3/utils/status.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index 46eec2a..ce8c25cf 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -190,8 +190,8 @@ bool is_delete_on_close_set(struct share_mode_lock *lck, uint32_t name_hash);
bool set_sticky_write_time(struct file_id fileid, struct timespec write_time);
bool set_write_time(struct file_id fileid, struct timespec write_time);
struct timespec get_share_mode_write_time(struct share_mode_lock *lck);
-int share_mode_forall(void (*fn)(const struct share_mode_entry *, const char *,
- const char *, void *),
+int share_entry_forall(void (*fn)(const struct share_mode_entry *, const char *,
+ const char *, void *),
void *private_data);
bool share_mode_cleanup_disconnected(struct file_id id,
uint64_t open_persistent_id);
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 12f499b..53039eb 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -499,8 +499,8 @@ static int traverse_fn(struct db_record *rec, void *_state)
share mode system.
********************************************************************/
-int share_mode_forall(void (*fn)(const struct share_mode_entry *, const char *,
- const char *, void *),
+int share_entry_forall(void (*fn)(const struct share_mode_entry *,
+ const char *, const char *, void *),
void *private_data)
{
struct forall_state state;
diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
index 855b8c7..d2f05f3 100644
--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -166,7 +166,7 @@ static WERROR net_enum_files(TALLOC_CTX *ctx,
f_enum_cnt.username = username;
f_enum_cnt.ctr3 = *ctr3;
- share_mode_forall( enum_file_fn, (void *)&f_enum_cnt );
+ share_entry_forall( enum_file_fn, (void *)&f_enum_cnt );
*ctr3 = f_enum_cnt.ctr3;
@@ -867,7 +867,7 @@ static void net_count_files_for_all_sess(struct srvsvc_NetSessCtr1 *ctr1,
s_file_info.resume_handle = resume_handle;
s_file_info.num_entries = num_entries;
- share_mode_forall(count_sess_files_fn, &s_file_info);
+ share_entry_forall(count_sess_files_fn, &s_file_info);
}
/*******************************************************************
@@ -984,7 +984,7 @@ static void count_share_opens(struct srvsvc_NetConnInfo1 *arr,
sfs.resp_entries = resp_entries;
sfs.total_entries = total_entries;
- share_mode_forall(share_file_fn, &sfs);
+ share_entry_forall(share_file_fn, &sfs);
}
/****************************************************************************
@@ -2744,7 +2744,7 @@ WERROR _srvsvc_NetFileClose(struct pipes_struct *p,
r->out.result = WERR_BADFILE;
state.r = r;
state.msg_ctx = p->msg_ctx;
- share_mode_forall(enum_file_close_fn, &state);
+ share_entry_forall(enum_file_close_fn, &state);
return r->out.result;
}
diff --git a/source3/utils/status.c b/source3/utils/status.c
index 7bbcea5..2c850bb 100644
--- a/source3/utils/status.c
+++ b/source3/utils/status.c
@@ -526,7 +526,7 @@ int main(int argc, const char *argv[])
goto done;
}
- result = share_mode_forall(print_share_mode, NULL);
+ result = share_entry_forall(print_share_mode, NULL);
if (result == 0) {
d_printf("No locked files\n");
--
1.9.1
From 25c31da9410393ed375fe6e7c05a6b0dcdd6cc56 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 23 Sep 2014 05:45:49 +0200
Subject: [PATCH 2/8] s3:locking: Introduce share_mode_forall
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
source3/locking/proto.h | 4 ++
source3/locking/share_mode_lock.c | 92 +++++++++++++++++++++++++++------------
2 files changed, 68 insertions(+), 28 deletions(-)
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index ce8c25cf..a5c46c8 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -190,6 +190,10 @@ bool is_delete_on_close_set(struct share_mode_lock *lck, uint32_t name_hash);
bool set_sticky_write_time(struct file_id fileid, struct timespec write_time);
bool set_write_time(struct file_id fileid, struct timespec write_time);
struct timespec get_share_mode_write_time(struct share_mode_lock *lck);
+int share_mode_forall(int (*fn)(struct file_id fid,
+ const struct share_mode_data *data,
+ void *private_data),
+ void *private_data);
int share_entry_forall(void (*fn)(const struct share_mode_entry *, const char *,
const char *, void *),
void *private_data);
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 53039eb..c2f3402 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -440,30 +440,33 @@ struct share_mode_lock *fetch_share_mode_unlocked(TALLOC_CTX *mem_ctx,
return lck;
}
-struct forall_state {
- void (*fn)(const struct share_mode_entry *entry,
- const char *sharepath,
- const char *fname,
- void *private_data);
+struct share_mode_forall_state {
+ int (*fn)(struct file_id fid, const struct share_mode_data *data,
+ void *private_data);
void *private_data;
};
-static int traverse_fn(struct db_record *rec, void *_state)
+static int share_mode_traverse_fn(struct db_record *rec, void *_state)
{
- struct forall_state *state = (struct forall_state *)_state;
+ struct share_mode_forall_state *state =
+ (struct share_mode_forall_state *)_state;
uint32_t i;
TDB_DATA key;
TDB_DATA value;
DATA_BLOB blob;
enum ndr_err_code ndr_err;
struct share_mode_data *d;
+ struct file_id fid;
+ int ret;
key = dbwrap_record_get_key(rec);
value = dbwrap_record_get_value(rec);
/* Ensure this is a locking_key record. */
- if (key.dsize != sizeof(struct file_id))
+ if (key.dsize != sizeof(fid)) {
return 0;
+ }
+ memcpy(&fid, key.dptr, sizeof(fid));
d = talloc(talloc_tos(), struct share_mode_data);
if (d == NULL) {
@@ -485,11 +488,58 @@ static int traverse_fn(struct db_record *rec, void *_state)
}
for (i=0; i<d->num_share_modes; i++) {
d->share_modes[i].stale = false; /* [skip] in idl */
- state->fn(&d->share_modes[i],
- d->servicepath, d->base_name,
- state->private_data);
}
+
+ ret = state->fn(fid, d, state->private_data);
+
TALLOC_FREE(d);
+ return ret;
+}
+
+int share_mode_forall(int (*fn)(struct file_id fid,
+ const struct share_mode_data *data,
+ void *private_data),
+ void *private_data)
+{
+ struct share_mode_forall_state state = {
+ .fn = fn,
+ .private_data = private_data
+ };
+ NTSTATUS status;
+ int count;
+
+ if (lock_db == NULL) {
+ return 0;
+ }
+
+ status = dbwrap_traverse_read(lock_db, share_mode_traverse_fn,
+ &state, &count);
+ if (!NT_STATUS_IS_OK(status)) {
+ return -1;
+ }
+
+ return count;
+}
+
+struct share_entry_forall_state {
+ void (*fn)(const struct share_mode_entry *e,
+ const char *service_path, const char *base_name,
+ void *private_data);
+ void *private_data;
+};
+
+static int share_entry_traverse_fn(struct file_id fid,
+ const struct share_mode_data *data,
+ void *private_data)
+{
+ struct share_entry_forall_state *state = private_data;
+ uint32_t i;
+
+ for (i=0; i<data->num_share_modes; i++) {
+ state->fn(&data->share_modes[i],
+ data->servicepath, data->base_name,
+ state->private_data);
+ }
return 0;
}
@@ -503,24 +553,10 @@ int share_entry_forall(void (*fn)(const struct share_mode_entry *,
const char *, const char *, void *),
void *private_data)
{
- struct forall_state state;
- NTSTATUS status;
- int count;
-
- if (lock_db == NULL)
- return 0;
-
- state.fn = fn;
- state.private_data = private_data;
+ struct share_entry_forall_state state = {
+ .fn = fn, .private_data = private_data };
- status = dbwrap_traverse_read(lock_db, traverse_fn, (void *)&state,
- &count);
-
- if (!NT_STATUS_IS_OK(status)) {
- return -1;
- } else {
- return count;
- }
+ return share_mode_forall(share_entry_traverse_fn, &state);
}
bool share_mode_cleanup_disconnected(struct file_id fid,
--
1.9.1
From 6ee97816451e7b5f89c69e55738703a8e5f3386b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 24 Sep 2014 20:46:15 +0200
Subject: [PATCH 3/8] s3:locking: allow early return for share_entry_forall()
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
source3/locking/proto.h | 4 +--
source3/locking/share_mode_lock.c | 23 ++++++++++------
source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 46 +++++++++++++++++--------------
source3/utils/status.c | 12 ++++----
4 files changed, 49 insertions(+), 36 deletions(-)
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index a5c46c8..44f3ba1 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -194,8 +194,8 @@ int share_mode_forall(int (*fn)(struct file_id fid,
const struct share_mode_data *data,
void *private_data),
void *private_data);
-int share_entry_forall(void (*fn)(const struct share_mode_entry *, const char *,
- const char *, void *),
+int share_entry_forall(int (*fn)(const struct share_mode_entry *, const char *,
+ const char *, void *),
void *private_data);
bool share_mode_cleanup_disconnected(struct file_id id,
uint64_t open_persistent_id);
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index c2f3402..da16d1a 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -522,9 +522,9 @@ int share_mode_forall(int (*fn)(struct file_id fid,
}
struct share_entry_forall_state {
- void (*fn)(const struct share_mode_entry *e,
- const char *service_path, const char *base_name,
- void *private_data);
+ int (*fn)(const struct share_mode_entry *e,
+ const char *service_path, const char *base_name,
+ void *private_data);
void *private_data;
};
@@ -536,9 +536,14 @@ static int share_entry_traverse_fn(struct file_id fid,
uint32_t i;
for (i=0; i<data->num_share_modes; i++) {
- state->fn(&data->share_modes[i],
- data->servicepath, data->base_name,
- state->private_data);
+ int ret;
+
+ ret = state->fn(&data->share_modes[i],
+ data->servicepath, data->base_name,
+ state->private_data);
+ if (ret != 0) {
+ return ret;
+ }
}
return 0;
@@ -549,9 +554,9 @@ static int share_entry_traverse_fn(struct file_id fid,
share mode system.
********************************************************************/
-int share_entry_forall(void (*fn)(const struct share_mode_entry *,
- const char *, const char *, void *),
- void *private_data)
+int share_entry_forall(int (*fn)(const struct share_mode_entry *,
+ const char *, const char *, void *),
+ void *private_data)
{
struct share_entry_forall_state state = {
.fn = fn, .private_data = private_data };
diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
index d2f05f3..eaa70e7 100644
--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -79,9 +79,9 @@ struct share_conn_stat {
/*******************************************************************
********************************************************************/
-static void enum_file_fn( const struct share_mode_entry *e,
- const char *sharepath, const char *fname,
- void *private_data )
+static int enum_file_fn(const struct share_mode_entry *e,
+ const char *sharepath, const char *fname,
+ void *private_data)
{
struct file_enum_count *fenum =
(struct file_enum_count *)private_data;
@@ -98,21 +98,21 @@ static void enum_file_fn( const struct share_mode_entry *e,
/* If the pid was not found delete the entry from connections.tdb */
if ( !process_exists(e->pid) ) {
- return;
+ return 0;
}
username = uidtoname(e->uid);
if ((fenum->username != NULL)
&& !strequal(username, fenum->username)) {
- return;
+ return 0;
}
f = talloc_realloc(fenum->ctx, fenum->ctr3->array,
struct srvsvc_NetFileInfo3, i+1);
if ( !f ) {
DEBUG(0,("conn_enum_fn: realloc failed for %d items\n", i+1));
- return;
+ return 0;
}
fenum->ctr3->array = f;
@@ -133,7 +133,7 @@ static void enum_file_fn( const struct share_mode_entry *e,
sharepath, fname );
}
if (!fullpath) {
- return;
+ return 0;
}
string_replace( fullpath, '/', '\\' );
@@ -150,6 +150,8 @@ static void enum_file_fn( const struct share_mode_entry *e,
fenum->ctr3->array[i].user = username;
fenum->ctr3->count++;
+
+ return 0;
}
/*******************************************************************
@@ -826,9 +828,9 @@ static WERROR init_srv_sess_info_0(struct pipes_struct *p,
* find out the session on which this file is open and bump up its count
**********************************************************************/
-static void count_sess_files_fn(const struct share_mode_entry *e,
- const char *sharepath, const char *fname,
- void *data)
+static int count_sess_files_fn(const struct share_mode_entry *e,
+ const char *sharepath, const char *fname,
+ void *data)
{
struct sess_file_info *info = data;
uint32_t rh = info->resume_handle;
@@ -846,9 +848,10 @@ static void count_sess_files_fn(const struct share_mode_entry *e,
serverid_equal(&e->pid, &sess->pid)) {
info->ctr->array[i].num_open++;
- return;
+ return 0;
}
}
+ return 0;
}
/*******************************************************************
@@ -950,9 +953,9 @@ static WERROR init_srv_sess_info_1(struct pipes_struct *p,
find the share connection on which this open exists.
********************************************************************/
-static void share_file_fn(const struct share_mode_entry *e,
- const char *sharepath, const char *fname,
- void *data)
+static int share_file_fn(const struct share_mode_entry *e,
+ const char *sharepath, const char *fname,
+ void *data)
{
struct share_file_stat *sfs = data;
uint32_t i;
@@ -962,10 +965,11 @@ static void share_file_fn(const struct share_mode_entry *e,
for (i=0; i < sfs->resp_entries; i++) {
if (serverid_equal(&e->pid, &sfs->svrid_arr[offset + i])) {
sfs->netconn_arr[i].num_open ++;
- return;
+ return 0;
}
}
}
+ return 0;
}
/*******************************************************************
@@ -2690,9 +2694,9 @@ struct enum_file_close_state {
struct messaging_context *msg_ctx;
};
-static void enum_file_close_fn( const struct share_mode_entry *e,
- const char *sharepath, const char *fname,
- void *private_data )
+static int enum_file_close_fn(const struct share_mode_entry *e,
+ const char *sharepath, const char *fname,
+ void *private_data)
{
char msg[MSG_SMB_SHARE_MODE_ENTRY_SIZE];
struct enum_file_close_state *state =
@@ -2700,11 +2704,11 @@ static void enum_file_close_fn( const struct share_mode_entry *e,
uint32_t fid = (((uint32_t)(procid_to_pid(&e->pid))<<16) | e->share_file_id);
if (fid != state->r->in.fid) {
- return; /* Not this file. */
+ return 0; /* Not this file. */
}
if (!process_exists(e->pid) ) {
- return;
+ return 0;
}
/* Ok - send the close message. */
@@ -2718,6 +2722,8 @@ static void enum_file_close_fn( const struct share_mode_entry *e,
messaging_send_buf(state->msg_ctx,
e->pid, MSG_SMB_CLOSE_FILE,
(uint8 *)msg, sizeof(msg)));
+
+ return 0;
}
/********************************************************************
diff --git a/source3/utils/status.c b/source3/utils/status.c
index 2c850bb..b589813 100644
--- a/source3/utils/status.c
+++ b/source3/utils/status.c
@@ -115,10 +115,10 @@ static bool Ucrit_addPid( struct server_id pid )
return True;
}
-static void print_share_mode(const struct share_mode_entry *e,
- const char *sharepath,
- const char *fname,
- void *dummy)
+static int print_share_mode(const struct share_mode_entry *e,
+ const char *sharepath,
+ const char *fname,
+ void *dummy)
{
static int count;
@@ -135,7 +135,7 @@ static void print_share_mode(const struct share_mode_entry *e,
if (do_checks && !serverid_exists(&e->pid)) {
/* the process for this entry does not exist any more */
- return;
+ return 0;
}
if (Ucrit_checkPid(e->pid)) {
@@ -183,6 +183,8 @@ static void print_share_mode(const struct share_mode_entry *e,
d_printf(" %s %s %s",sharepath, fname, time_to_asc((time_t)e->time.tv_sec));
}
+
+ return 0;
}
static void print_brl(struct file_id id,
--
1.9.1
From 70806faa109a76c8d5059ed38e3a2d718322b76d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 24 Oct 2014 13:57:04 -0700
Subject: [PATCH 4/8] s3:param: Add new option "strict rename".
Control whether smbd can rename directories containing
open files. Defaults to "no" (meaning we *can* do
such renames).
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
docs-xml/smbdotconf/tuning/strictrename.xml | 25 +++++++++++++++++++++++++
lib/param/param_table.c | 9 +++++++++
source3/param/loadparm.c | 1 +
3 files changed, 35 insertions(+)
create mode 100644 docs-xml/smbdotconf/tuning/strictrename.xml
diff --git a/docs-xml/smbdotconf/tuning/strictrename.xml b/docs-xml/smbdotconf/tuning/strictrename.xml
new file mode 100644
index 0000000..5478863
--- /dev/null
+++ b/docs-xml/smbdotconf/tuning/strictrename.xml
@@ -0,0 +1,25 @@
+<samba:parameter name="strict rename"
+ context="S"
+ type="boolean"
+ xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
+<description>
+ <para>By default a Windows SMB server prevents directory
+ renames when there are open file or directory handles below
+ it in the filesystem hierarchy. Historically Samba has always
+ allowed this as POSIX filesystem semantics require it.</para>
+
+ <para>This boolean parameter allows Samba to match the Windows
+ behavior. Setting this to "yes" is a very expensive change,
+ as it forces Samba to travers the entire open file handle
+ database on every directory rename request. In a clustered
+ Samba system the cost is even greater than the non-clustered
+ case.</para>
+
+ <para>For this reason the default is "no", and it is recommended
+ to be left that way unless a specific Windows application requires
+ it to be changed.</para>
+
+</description>
+
+<value type="default">no</value>
+</samba:parameter>
diff --git a/lib/param/param_table.c b/lib/param/param_table.c
index 15ffa8c..7a13e6f 100644
--- a/lib/param/param_table.c
+++ b/lib/param/param_table.c
@@ -1884,6 +1884,15 @@ struct parm_struct parm_table[] = {
.flags = FLAG_ADVANCED | FLAG_SHARE,
},
{
+ .label = "strict rename",
+ .type = P_BOOL,
+ .p_class = P_LOCAL,
+ .offset = LOCAL_VAR(strict_rename),
+ .special = NULL,
+ .enum_list = NULL,
+ .flags = FLAG_ADVANCED | FLAG_SHARE,
+ },
+ {
.label = "strict sync",
.type = P_BOOL,
.p_class = P_LOCAL,
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 52ffbcc..d2afac7 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -206,6 +206,7 @@ static struct loadparm_service sDefault =
.follow_symlinks = true,
.sync_always = false,
.strict_allocate = false,
+ .strict_rename = false,
.strict_sync = false,
.mangling_char = '~',
.copymap = NULL,
--
1.9.1
From dc3482d8a491ddb4e2534ab035ff9e6cc3ed0d18 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 24 Oct 2014 13:57:04 -0700
Subject: [PATCH 5/8] selftest:Samba3: use "strict rename = yes"
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
selftest/target/Samba3.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 1963321..48b8164 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1076,6 +1076,7 @@ sub provision($$$$$$)
store dos attributes = yes
create mask = 755
dos filemode = yes
+ strict rename = yes
vfs objects = acl_xattr fake_acls xattr_tdb streams_depot
printing = vlp
--
1.9.1
From 8591587aea245066844ffe14eef1ce6a3069874a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 25 Sep 2014 01:30:33 +0200
Subject: [PATCH 6/8] s3:smbd: Don't rename a dir with files open underneath
This is an EXPENSIVE check. We'll have to guard this with an option
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
source3/smbd/dir.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 120 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index e60bc2c..36d95d5 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -25,6 +25,7 @@
#include "libcli/security/security.h"
#include "lib/util/bitmap.h"
#include "../lib/util/memcache.h"
+#include "../librpc/gen_ndr/open_files.h"
/*
This module implements directory related functions for Samba.
@@ -1809,6 +1810,113 @@ bool SearchDir(struct smb_Dir *dirp, const char *name, long *poffset)
return False;
}
+struct files_below_forall_state {
+ char *dirpath;
+ size_t dirpath_len;
+ int (*fn)(struct file_id fid, const struct share_mode_data *data,
+ void *private_data);
+ void *private_data;
+};
+
+static int files_below_forall_fn(struct file_id fid,
+ const struct share_mode_data *data,
+ void *private_data)
+{
+ struct files_below_forall_state *state = private_data;
+ char tmpbuf[PATH_MAX];
+ char *fullpath, *to_free;
+ size_t len;
+
+ len = full_path_tos(data->servicepath, data->base_name,
+ tmpbuf, sizeof(tmpbuf),
+ &fullpath, &to_free);
+ if (len == -1) {
+ return 0;
+ }
+ if (state->dirpath_len >= len) {
+ /*
+ * Filter files above dirpath
+ */
+ return 0;
+ }
+ if (fullpath[state->dirpath_len] != '/') {
+ /*
+ * Filter file that don't have a path separator at the end of
+ * dirpath's length
+ */
+ return 0;
+ }
+
+ if (memcmp(state->dirpath, fullpath, len) != 0) {
+ /*
+ * Not a parent
+ */
+ return 0;
+ }
+
+ return state->fn(fid, data, private_data);
+}
+
+static int files_below_forall(connection_struct *conn,
+ const struct smb_filename *dir_name,
+ int (*fn)(struct file_id fid,
+ const struct share_mode_data *data,
+ void *private_data),
+ void *private_data)
+{
+ struct files_below_forall_state state = {};
+ int ret;
+ char tmpbuf[PATH_MAX];
+ char *to_free;
+
+ state.dirpath_len = full_path_tos(conn->connectpath,
+ dir_name->base_name,
+ tmpbuf, sizeof(tmpbuf),
+ &state.dirpath, &to_free);
+ if (state.dirpath_len == -1) {
+ return -1;
+
+ }
+
+ ret = share_mode_forall(files_below_forall_fn, &state);
+ TALLOC_FREE(to_free);
+ return ret;
+}
+
+struct have_file_open_below_state {
+ bool found_one;
+};
+
+static int have_file_open_below_fn(struct file_id fid,
+ const struct share_mode_data *data,
+ void *private_data)
+{
+ struct have_file_open_below_state *state = private_data;
+ state->found_one = true;
+ return 1;
+}
+
+static bool have_file_open_below(connection_struct *conn,
+ const struct smb_filename *name)
+{
+ struct have_file_open_below_state state = {};
+ int ret;
+
+ if (!VALID_STAT(name->st)) {
+ return false;
+ }
+ if (!S_ISDIR(name->st.st_ex_mode)) {
+ return false;
+ }
+
+ ret = files_below_forall(conn, name, have_file_open_below_fn, &state);
+ if (ret == -1) {
+ return false;
+ }
+
+ return state.found_one;
+}
+
/*****************************************************************
Is this directory empty ?
*****************************************************************/
@@ -1854,5 +1962,16 @@ NTSTATUS can_delete_directory_fsp(files_struct *fsp)
TALLOC_FREE(talloced);
TALLOC_FREE(dir_hnd);
- return status;
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
+ }
+
+ if (!lp_posix_pathnames() &&
+ lp_strict_rename(SNUM(conn)) &&
+ have_file_open_below(fsp->conn, fsp->fsp_name))
+ {
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ return NT_STATUS_OK;
}
--
1.9.1
From e9712a9e1c1eb249785f0f17df3432f212540087 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 25 Sep 2014 01:32:00 +0200
Subject: [PATCH 7/8] s4:torture/smb2: test rename dir deny with open files
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
source4/torture/smb2/rename.c | 97 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)
diff --git a/source4/torture/smb2/rename.c b/source4/torture/smb2/rename.c
index 9d0f4e1..07bdabd 100644
--- a/source4/torture/smb2/rename.c
+++ b/source4/torture/smb2/rename.c
@@ -928,6 +928,99 @@ done:
return ret;
}
+static bool torture_smb2_rename_dir_openfile(struct torture_context *torture,
+ struct smb2_tree *tree1)
+{
+ bool ret = true;
+ NTSTATUS status;
+ union smb_open io;
+ union smb_close cl;
+ union smb_setfileinfo sinfo;
+ struct smb2_handle d1, h1;
+
+ smb2_deltree(tree1, BASEDIR);
+ smb2_util_rmdir(tree1, BASEDIR);
+
+ torture_comment(torture, "Creating base directory\n");
+
+ ZERO_STRUCT(io.smb2);
+ io.generic.level = RAW_OPEN_SMB2;
+ io.smb2.in.create_flags = 0;
+ io.smb2.in.desired_access = 0x0017019f;
+ io.smb2.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
+ io.smb2.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY;
+ io.smb2.in.share_access = 0;
+ io.smb2.in.alloc_size = 0;
+ io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE;
+ io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+ io.smb2.in.security_flags = 0;
+ io.smb2.in.fname = BASEDIR;
+
+ status = smb2_create(tree1, torture, &(io.smb2));
+ CHECK_STATUS(status, NT_STATUS_OK);
+ d1 = io.smb2.out.file.handle;
+
+ torture_comment(torture, "Creating test file\n");
+
+ ZERO_STRUCT(io.smb2);
+ io.generic.level = RAW_OPEN_SMB2;
+ io.smb2.in.create_flags = 0;
+ io.smb2.in.desired_access = 0x0017019f;
+ io.smb2.in.create_options = NTCREATEX_OPTIONS_NON_DIRECTORY_FILE;
+ io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+ io.smb2.in.share_access = 0;
+ io.smb2.in.alloc_size = 0;
+ io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE;
+ io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+ io.smb2.in.security_flags = 0;
+ io.smb2.in.fname = BASEDIR "\\file.txt";
+
+ status = smb2_create(tree1, torture, &(io.smb2));
+ CHECK_STATUS(status, NT_STATUS_OK);
+ h1 = io.smb2.out.file.handle;
+
+ torture_comment(torture, "Renaming directory\n");
+
+ ZERO_STRUCT(sinfo);
+ sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION;
+ sinfo.rename_information.in.file.handle = d1;
+ sinfo.rename_information.in.overwrite = 0;
+ sinfo.rename_information.in.root_fid = 0;
+ sinfo.rename_information.in.new_name =
+ BASEDIR "-new";
+ status = smb2_setinfo_file(tree1, &sinfo);
+ CHECK_STATUS(status, NT_STATUS_ACCESS_DENIED);
+
+ torture_comment(torture, "Closing directory\n");
+
+ ZERO_STRUCT(cl.smb2);
+ cl.smb2.level = RAW_CLOSE_SMB2;
+ cl.smb2.in.file.handle = d1;
+ status = smb2_close(tree1, &(cl.smb2));
+ CHECK_STATUS(status, NT_STATUS_OK);
+ ZERO_STRUCT(d1);
+
+ torture_comment(torture, "Closing test file\n");
+
+ cl.smb2.in.file.handle = h1;
+ status = smb2_close(tree1, &(cl.smb2));
+ CHECK_STATUS(status, NT_STATUS_OK);
+ ZERO_STRUCT(h1);
+
+done:
+
+ torture_comment(torture, "Cleaning up\n");
+
+ if (h1.data) {
+ ZERO_STRUCT(cl.smb2);
+ cl.smb2.level = RAW_CLOSE_SMB2;
+ cl.smb2.in.file.handle = h1;
+ status = smb2_close(tree1, &(cl.smb2));
+ }
+ smb2_deltree(tree1, BASEDIR);
+ return ret;
+}
+
struct rename_one_dir_cycle_state {
struct tevent_context *ev;
struct smb2_tree *tree;
@@ -1336,6 +1429,10 @@ struct torture_suite *torture_smb2_rename_init(void)
"msword",
torture_smb2_rename_msword);
+ torture_suite_add_1smb2_test(
+ suite, "rename_dir_openfile",
+ torture_smb2_rename_dir_openfile);
+
torture_suite_add_1smb2_test(suite,
"rename_dir_bench",
torture_smb2_rename_dir_bench);
--
1.9.1
From d7562df3abeb630a22ba0e5f80f6b67957ee9b71 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 28 Oct 2014 15:20:26 -0700
Subject: [PATCH 8/8] s3:locking: Change from ndr_pull_struct_blob() to
ndr_pull_struct_blob_all() so we fail if not all bytes are consumed.
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
source3/locking/share_mode_lock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index da16d1a..65409ac 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -133,7 +133,7 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx,
blob.data = dbuf.dptr;
blob.length = dbuf.dsize;
- ndr_err = ndr_pull_struct_blob(
+ ndr_err = ndr_pull_struct_blob_all(
&blob, d, d, (ndr_pull_flags_fn_t)ndr_pull_share_mode_data);
if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
DEBUG(1, ("ndr_pull_share_mode_lock failed: %s\n",
@@ -476,7 +476,7 @@ static int share_mode_traverse_fn(struct db_record *rec, void *_state)
blob.data = value.dptr;
blob.length = value.dsize;
- ndr_err = ndr_pull_struct_blob(
+ ndr_err = ndr_pull_struct_blob_all(
&blob, d, d, (ndr_pull_flags_fn_t)ndr_pull_share_mode_data);
if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
DEBUG(1, ("ndr_pull_share_mode_lock failed\n"));
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20141030/dff0f314/attachment.pgp>
More information about the samba-technical
mailing list