[PATCH] Deny file rename if file has open streams
Ralph Böhme
slow at samba.org
Wed May 30 09:01:36 UTC 2018
On Tue, May 29, 2018 at 02:12:40PM -0700, Jeremy Allison wrote:
> On Sun, May 27, 2018 at 04:57:54PM +0200, Ralph Böhme via samba-technical wrote:
> > Hi!
> >
> > Just stumpled across this one: Windows denies renaming a file that has open
> > streams with NT_STATUS_ACCESS_DENIED.
> >
> > Please review & push if happy.
>
> Only one comment, you've dupicated:
>
> +#define NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN 0x0008
>
> with the existing:
>
> /* Private options for printer support */
> #define NTCREATEX_OPTIONS_PRIVATE_DELETE_ON_CLOSE 0x0008
>
> Was this intentional ? They shouldn't conflict as
> NTCREATEX_OPTIONS_PRIVATE_DELETE_ON_CLOSE is only used for
> printer handles, but I'm thinking it'd be safer to set
> NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN to be 0x0010 instead.
heavens! Sorry, no, this was not intentional. Thanks for spotting this!
Attached is an updated patchset. I've also removed the unneeded check for
e->share_file==fsp->fh->gen_id from the second patch from the for loop in
file_has_open_streams().
Please review & push if ok. Thanks!
-slow
--
Ralph Boehme, Samba Team https://samba.org/
Samba Developer, SerNet GmbH https://sernet.de/en/samba/
GPG Key Fingerprint: FAE2 C608 8A24 2520 51C5
59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From 099ec01d1ba11d0fad366b14d809b8368ad36021 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 27 May 2018 13:01:50 +0200
Subject: [PATCH 1/3] s3:smbd: add private option
NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN
This will be used to mark basefile opens of streams opens. This is
needed to later implement a function that can determine if a file has
stream opens.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/include/smb.h | 3 +++
source3/smbd/open.c | 7 ++++++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/source3/include/smb.h b/source3/include/smb.h
index 3316f09d94f..5e83ee90afe 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -419,6 +419,9 @@ Offset Data length.
/* Private options for printer support */
#define NTCREATEX_OPTIONS_PRIVATE_DELETE_ON_CLOSE 0x0008
+/* Private option for streams support */
+#define NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN 0x0010
+
/* Flag for NT transact rename call. */
#define RENAME_REPLACE_IF_EXISTS 1
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 61a42e29a10..5bb42e09d8c 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -5144,6 +5144,7 @@ static NTSTATUS create_file_unixpath(connection_struct *conn,
&& (!(private_flags & NTCREATEX_OPTIONS_PRIVATE_STREAM_DELETE))) {
uint32_t base_create_disposition;
struct smb_filename *smb_fname_base = NULL;
+ uint32_t base_privflags;
if (create_options & FILE_DIRECTORY_FILE) {
status = NT_STATUS_NOT_A_DIRECTORY;
@@ -5194,13 +5195,17 @@ static NTSTATUS create_file_unixpath(connection_struct *conn,
}
}
+ base_privflags = NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN;
+
/* Open the base file. */
status = create_file_unixpath(conn, NULL, smb_fname_base, 0,
FILE_SHARE_READ
| FILE_SHARE_WRITE
| FILE_SHARE_DELETE,
base_create_disposition,
- 0, 0, 0, NULL, 0, 0, NULL, NULL,
+ 0, 0, 0, NULL, 0,
+ base_privflags,
+ NULL, NULL,
&base_fsp, NULL);
TALLOC_FREE(smb_fname_base);
--
2.13.6
From 2cb192430d21efc31155255880179ad44c81adcd Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 27 May 2018 13:03:25 +0200
Subject: [PATCH 2/3] s3:locking: add file_has_open_streams()
This can be used to check if a file opened by fsp also has stream opens.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/locking/locking.c | 31 +++++++++++++++++++++++++++++++
source3/locking/proto.h | 1 +
2 files changed, 32 insertions(+)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 791878c6383..e962fee89ab 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -1316,3 +1316,34 @@ struct timespec get_share_mode_write_time(struct share_mode_lock *lck)
}
return d->old_write_time;
}
+
+bool file_has_open_streams(files_struct *fsp)
+{
+ struct share_mode_lock *lock = NULL;
+ struct share_mode_data *d = NULL;
+ uint32_t i;
+
+ lock = get_existing_share_mode_lock(talloc_tos(), fsp->file_id);
+ if (lock == NULL) {
+ return false;
+ }
+ d = lock->data;
+
+ for (i = 0; i < d->num_share_modes; i++) {
+ struct share_mode_entry *e = &d->share_modes[i];
+
+ if (share_mode_stale_pid(d, i)) {
+ continue;
+ }
+
+ if (e->private_options &
+ NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN)
+ {
+ TALLOC_FREE(lock);
+ return true;
+ }
+ }
+
+ TALLOC_FREE(lock);
+ return false;
+}
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index afd5373772a..403729c934a 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -207,6 +207,7 @@ 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);
+bool file_has_open_streams(files_struct *fsp);
int share_mode_forall(int (*fn)(struct file_id fid,
const struct share_mode_data *data,
void *private_data),
--
2.13.6
From 50d77d327fe3c6a849d948f15034bb2f44ef3146 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 26 May 2018 18:32:21 +0200
Subject: [PATCH 3/3] s3:smbd: don't allow renaming basefile if streams are
open
Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451
Signed-off-by: Ralph Boehme <slow at samba.org>
---
selftest/knownfail.d/samba3.smb2.streams | 2 --
selftest/knownfail.d/samba3.vfs.fruit | 3 ---
source3/smbd/reply.c | 4 ++++
3 files changed, 4 insertions(+), 5 deletions(-)
delete mode 100644 selftest/knownfail.d/samba3.smb2.streams
diff --git a/selftest/knownfail.d/samba3.smb2.streams b/selftest/knownfail.d/samba3.smb2.streams
deleted file mode 100644
index 26d40a67bda..00000000000
--- a/selftest/knownfail.d/samba3.smb2.streams
+++ /dev/null
@@ -1,2 +0,0 @@
-samba3.smb2.streams.basefile-rename-with-open-stream\(.*\)
-samba3.smb2.streams streams_xattr.basefile-rename-with-open-stream\(nt4_dc\)
diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
index bf97dbc5822..8df25bccb79 100644
--- a/selftest/knownfail.d/samba3.vfs.fruit
+++ b/selftest/knownfail.d/samba3.vfs.fruit
@@ -1,4 +1 @@
^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.read open rsrc after rename\(nt4_dc\)
-^samba3.vfs.fruit metadata_stream.read open rsrc after rename\(nt4_dc\)
-^samba3.vfs.fruit streams_depot.read open rsrc after rename\(nt4_dc\)
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index fc56e3234be..1b99664cbd8 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -6643,6 +6643,10 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
return status;
}
+ if (file_has_open_streams(fsp)) {
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
/* Make a copy of the dst smb_fname structs */
smb_fname_dst = cp_smb_filename(ctx, smb_fname_dst_in);
--
2.13.6
More information about the samba-technical
mailing list