[PATCH] Deny file rename if file has open streams
Jeremy Allison
jra at samba.org
Wed May 30 17:01:59 UTC 2018
On Wed, May 30, 2018 at 11:01:36AM +0200, Ralph Böhme wrote:
> 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!
Much better thanks ! RB+ and pushed (with the first 3 patches
from the previous patchset also, as required :-).
Cheers,
Jeremy.
> --
> 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
> 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