Attachment 7644 Details for Bug 8995 – Tested patches for master

[patch] Tested patches for master

tmp.diff (text/plain), 8.40 KB, created by Stefan Metzmacher on 2012-06-14 12:18:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2012-06-14 12:18:51 UTC

Size: 8.40 KB

patch

obsolete

>From 91dbf8cd26490550666809efeb6747c7ec7eded4 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 13 Jun 2012 12:11:51 +0200
>Subject: [PATCH 1/4] s3:smbd: try to make fsp->fh->gen_id as globally unique as possible
>
>This makes sure the value is never 0, it's between 1 and UINT32_MAX.
>
>While fsp->fh->gen_id is 'unsigned long' currently (which might by 8 bytes),
>there's some oplock code which truncates it to uint32_t (using IVAL()).
>
>Which means we could reuse fsp->fh->gen_id as persistent file id
>until we have a final fix, which uses database.
>
>See bug #8995 for more details.
>
>Based on code from Ira Cooper. Ensure fsp->fh->gen_id starts from
>a random point. We will use this as the SMB2 persistent_id.
>
>metze
>---
> source3/smbd/files.c |   20 +++++++++++++++++++-
> 1 files changed, 19 insertions(+), 1 deletions(-)
>
>diff --git a/source3/smbd/files.c b/source3/smbd/files.c
>index 97db348..fb9dacc 100644
>--- a/source3/smbd/files.c
>+++ b/source3/smbd/files.c
>@@ -28,12 +28,26 @@
> #define FILE_HANDLE_OFFSET 0x1000
> 
> /****************************************************************************
>- Return a unique number identifying this fsp over the life of this pid.
>+ Return a unique number identifying this fsp over the life of this pid,
>+ and try to make it as globally unique as possible.
>+ See bug #8995 for the details.
> ****************************************************************************/
> 
> static unsigned long get_gen_count(struct smbd_server_connection *sconn)
> {
>+	/*
>+	 * While fsp->fh->gen_id is 'unsigned long' currently
>+	 * (which might by 8 bytes),
>+	 * there's some oplock code which truncates it to
>+	 * uint32_t(using IVAL()).
>+	 */
>+	if (sconn->file_gen_counter == 0) {
>+		sconn->file_gen_counter = generate_random();
>+	}
> 	sconn->file_gen_counter += 1;
>+	if (sconn->file_gen_counter >= UINT32_MAX) {
>+		sconn->file_gen_counter = 0;
>+	}
> 	if (sconn->file_gen_counter == 0) {
> 		sconn->file_gen_counter += 1;
> 	}
>@@ -315,6 +329,10 @@ files_struct *file_find_dif(struct smbd_server_connection *sconn,
> 	int count=0;
> 	files_struct *fsp;
> 
>+	if (gen_id == 0) {
>+		return NULL;
>+	}
>+
> 	for (fsp=sconn->files; fsp; fsp=fsp->next,count++) {
> 		/* We can have a fsp->fh->fd == -1 here as it could be a stat open. */
> 		if (file_id_equal(&fsp->file_id, &id) &&
>-- 
>1.7.4.1
>
>
>From 7633a0fe968d5300629613ed15f025cd3023f855 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 13 Jun 2012 15:40:23 +0200
>Subject: [PATCH 2/4] s3:smbd: only set fsp->fh->gen_id for a client connection
>
>For faked connections, like dfs and printing, we leave it as 0.
>
>metze
>---
> source3/smbd/files.c |    3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
>diff --git a/source3/smbd/files.c b/source3/smbd/files.c
>index fb9dacc..3fc36d5 100644
>--- a/source3/smbd/files.c
>+++ b/source3/smbd/files.c
>@@ -119,7 +119,6 @@ NTSTATUS file_new(struct smb_request *req, connection_struct *conn,
> 
> 	fsp->fnum = -1;
> 	fsp->conn = conn;
>-	fsp->fh->gen_id = get_gen_count(sconn);
> 	GetTimeOfDay(&fsp->open_time);
> 
> 	if (sconn->file_bmap != NULL) {
>@@ -129,6 +128,8 @@ NTSTATUS file_new(struct smb_request *req, connection_struct *conn,
> 
> 		fsp->fnum = i + FILE_HANDLE_OFFSET;
> 		SMB_ASSERT(fsp->fnum < 65536);
>+
>+		fsp->fh->gen_id = get_gen_count(sconn);
> 	}
> 
> 	DLIST_ADD(sconn->files, fsp);
>-- 
>1.7.4.1
>
>
>From 34b5c9390840779702ec2cb0596c1f5423f64c58 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 14 Jun 2012 10:32:43 +0200
>Subject: [PATCH 3/4] s3:smbd: add a fsp_persistent_id() function
>
>This calculates a 64-bit value that most likely uniquely identifies
>the files_struct globally to the server.
>
>* 32-bit random gen_id
>* 16-bit truncated open_time
>* 16-bit fnum (valatile_id)
>
>Based on code from Ira Cooper. Use fsp->fh->gen_id as the persistent
>fileid in SMB2.
>
>Pair-Programmed-With: Michael Adam <obnox@samba.org>
>
>metze
>---
> source3/smbd/files.c |   24 ++++++++++++++++++++++++
> source3/smbd/proto.h |    1 +
> 2 files changed, 25 insertions(+), 0 deletions(-)
>
>diff --git a/source3/smbd/files.c b/source3/smbd/files.c
>index 3fc36d5..1833080 100644
>--- a/source3/smbd/files.c
>+++ b/source3/smbd/files.c
>@@ -597,6 +597,30 @@ files_struct *file_fsp(struct smb_request *req, uint16 fid)
> 	return fsp;
> }
> 
>+uint64_t fsp_persistent_id(const struct files_struct *fsp)
>+{
>+	uint64_t persistent_id;
>+
>+	/*
>+	 * This calculates a number that is most likely
>+	 * globally unique. In future we will have a database
>+	 * to make it completely unique.
>+	 *
>+	 * 32-bit random gen_id
>+	 * 16-bit truncated open_time
>+	 * 16-bit fnum (valatile_id)
>+	 */
>+	persistent_id = fsp->fh->gen_id & UINT32_MAX;
>+	persistent_id <<= 16;
>+	persistent_id &= 0x0000FFFFFFFF0000LLU;
>+	persistent_id |= fsp->open_time.tv_usec & UINT16_MAX;
>+	persistent_id <<= 16;
>+	persistent_id &= 0xFFFFFFFFFFFF0000LLU;
>+	persistent_id |= fsp->fnum & UINT16_MAX;
>+
>+	return persistent_id;
>+}
>+
> struct files_struct *file_fsp_smb2(struct smbd_smb2_request *smb2req,
> 				   uint64_t persistent_id,
> 				   uint64_t volatile_id)
>diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
>index e7bc3eb..53d9e2dd 100644
>--- a/source3/smbd/proto.h
>+++ b/source3/smbd/proto.h
>@@ -387,6 +387,7 @@ bool file_find_subpath(files_struct *dir_fsp);
> void file_sync_all(connection_struct *conn);
> void file_free(struct smb_request *req, files_struct *fsp);
> files_struct *file_fsp(struct smb_request *req, uint16 fid);
>+uint64_t fsp_persistent_id(const struct files_struct *fsp);
> struct files_struct *file_fsp_smb2(struct smbd_smb2_request *smb2req,
> 				   uint64_t persistent_id,
> 				   uint64_t volatile_id);
>-- 
>1.7.4.1
>
>
>From 0a87f75b93bfcd2a76e8b7371a3a0a556c8ad427 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 13 Jun 2012 12:13:01 +0200
>Subject: [PATCH 4/4] s3:smbd: use fsp_persistent_id() as persistent_file_id part for SMB2 (bug #8995)
>
>It seems to be important to have unique persistent file ids,
>because windows clients seem to index files by server_guid + persistent_file_id.
>Which may break, if we just have a 16-bit range per connection
>and the client connects multiple times.
>
>Based on code from Ira Cooper. Use fsp->fh->gen_id as the persistent
>fileid in SMB2.
>
>metze
>---
> source3/smbd/files.c       |   10 ++++++----
> source3/smbd/smb2_break.c  |    3 ++-
> source3/smbd/smb2_create.c |    2 +-
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
>diff --git a/source3/smbd/files.c b/source3/smbd/files.c
>index 1833080..d410083 100644
>--- a/source3/smbd/files.c
>+++ b/source3/smbd/files.c
>@@ -626,15 +626,12 @@ struct files_struct *file_fsp_smb2(struct smbd_smb2_request *smb2req,
> 				   uint64_t volatile_id)
> {
> 	struct files_struct *fsp;
>+	uint64_t fsp_persistent;
> 
> 	if (smb2req->compat_chain_fsp != NULL) {
> 		return smb2req->compat_chain_fsp;
> 	}
> 
>-	if (persistent_id != volatile_id) {
>-		return NULL;
>-	}
>-
> 	if (volatile_id > UINT16_MAX) {
> 		return NULL;
> 	}
>@@ -643,6 +640,11 @@ struct files_struct *file_fsp_smb2(struct smbd_smb2_request *smb2req,
> 	if (fsp == NULL) {
> 		return NULL;
> 	}
>+	fsp_persistent = fsp_persistent_id(fsp);
>+
>+	if (persistent_id != fsp_persistent) {
>+		return NULL;
>+	}
> 
> 	if (smb2req->tcon == NULL) {
> 		return NULL;
>diff --git a/source3/smbd/smb2_break.c b/source3/smbd/smb2_break.c
>index 75505e5..9318068 100644
>--- a/source3/smbd/smb2_break.c
>+++ b/source3/smbd/smb2_break.c
>@@ -237,6 +237,7 @@ void send_break_message_smb2(files_struct *fsp, int level)
> 				SMB2_OPLOCK_LEVEL_II :
> 				SMB2_OPLOCK_LEVEL_NONE;
> 	NTSTATUS status;
>+	uint64_t fsp_persistent = fsp_persistent_id(fsp);
> 
> 	DEBUG(10,("send_break_message_smb2: sending oplock break "
> 		"for file %s, fnum = %d, smb2 level %u\n",
>@@ -245,7 +246,7 @@ void send_break_message_smb2(files_struct *fsp, int level)
> 		(unsigned int)smb2_oplock_level ));
> 
> 	status = smbd_smb2_send_oplock_break(fsp->conn->sconn,
>-					(uint64_t)fsp->fnum,
>+					fsp_persistent,
> 					(uint64_t)fsp->fnum,
> 					smb2_oplock_level);
> 	if (!NT_STATUS_IS_OK(status)) {
>diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c
>index 2aedfb6..9881ed2 100644
>--- a/source3/smbd/smb2_create.c
>+++ b/source3/smbd/smb2_create.c
>@@ -822,7 +822,7 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx,
> 	if (state->out_file_attributes == 0) {
> 		state->out_file_attributes = FILE_ATTRIBUTE_NORMAL;
> 	}
>-	state->out_file_id_persistent = result->fnum;
>+	state->out_file_id_persistent = fsp_persistent_id(result);
> 	state->out_file_id_volatile = result->fnum;
> 	state->out_context_blobs = out_context_blobs;
> 
>-- 
>1.7.4.1
>

Actions: View

Attachments on bug 8995: 7640 | 7641 | 7643 | 7644 | 7651