[PATCH] A few smaller patches

Volker Lendecke Volker.Lendecke at SerNet.DE
Thu Sep 13 18:17:44 UTC 2018


Hi!

Review appreciated!

Thanks, Volker

-- 
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 at sernet.de

Meet us at Storage Developer Conference (SDC)
Santa Clara, CA USA, September 24th-27th 2018
-------------- next part --------------
From 853096db39ee5793005e98762a3a389d4b7c29bf Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 10 Sep 2018 14:53:37 +0200
Subject: [PATCH 01/12] smbd: Add some structure protection for durable
 reconnect

We should consume all data, and the ndr_pull function fills in all
fields. Thus the ZERO_STRUCT(cookie) is not required.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/durable.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c
index 80392e2c6db..1f6113e4cc9 100644
--- a/source3/smbd/durable.c
+++ b/source3/smbd/durable.c
@@ -546,10 +546,11 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn,
 	 * call below.
 	 */
 
-	ZERO_STRUCT(cookie);
-
-	ndr_err = ndr_pull_struct_blob(&old_cookie, talloc_tos(), &cookie,
-			(ndr_pull_flags_fn_t)ndr_pull_vfs_default_durable_cookie);
+	ndr_err = ndr_pull_struct_blob_all(
+		&old_cookie,
+		talloc_tos(),
+		&cookie,
+		(ndr_pull_flags_fn_t)ndr_pull_vfs_default_durable_cookie);
 	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
 		status = ndr_map_error2ntstatus(ndr_err);
 		return status;
-- 
2.11.0


From 603f14f88b85706826919ac768180a391ae3c436 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 16 Aug 2018 13:18:14 +0200
Subject: [PATCH 02/12] dbwrap_tdb: Avoid double-call to talloc_get_type_abort

We've already retrieved "ctx" in the callers of db_tdb_fetch_locked_internal().

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/dbwrap/dbwrap_tdb.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/dbwrap/dbwrap_tdb.c b/lib/dbwrap/dbwrap_tdb.c
index d695f3bd732..00d283c66e6 100644
--- a/lib/dbwrap/dbwrap_tdb.c
+++ b/lib/dbwrap/dbwrap_tdb.c
@@ -111,10 +111,11 @@ static int db_tdb_fetchlock_parse(TDB_DATA key, TDB_DATA data,
 }
 
 static struct db_record *db_tdb_fetch_locked_internal(
-	struct db_context *db, TALLOC_CTX *mem_ctx, TDB_DATA key)
+	struct db_context *db,
+	struct db_tdb_ctx *ctx,
+	TALLOC_CTX *mem_ctx,
+	TDB_DATA key)
 {
-	struct db_tdb_ctx *ctx = talloc_get_type_abort(db->private_data,
-						       struct db_tdb_ctx);
 	struct tdb_fetch_locked_state state;
 	int ret;
 
@@ -162,7 +163,7 @@ static struct db_record *db_tdb_fetch_locked(
 		DEBUG(3, ("tdb_chainlock failed\n"));
 		return NULL;
 	}
-	return db_tdb_fetch_locked_internal(db, mem_ctx, key);
+	return db_tdb_fetch_locked_internal(db, ctx, mem_ctx, key);
 }
 
 static struct db_record *db_tdb_try_fetch_locked(
@@ -176,7 +177,7 @@ static struct db_record *db_tdb_try_fetch_locked(
 		DEBUG(3, ("tdb_chainlock_nonblock failed\n"));
 		return NULL;
 	}
-	return db_tdb_fetch_locked_internal(db, mem_ctx, key);
+	return db_tdb_fetch_locked_internal(db, ctx, mem_ctx, key);
 }
 
 static NTSTATUS db_tdb_do_locked(struct db_context *db, TDB_DATA key,
-- 
2.11.0


From 09e977fe0d1935fb55ad6181ed37d1fec65d7c6b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 13 Sep 2018 18:56:54 +0200
Subject: [PATCH 03/12] smbd: Add a paranoia check for leases

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/oplock.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 2faad788d46..827dbeb4ef2 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -347,6 +347,12 @@ static void lease_timeout_handler(struct tevent_context *ctx,
 		return;
 	}
 
+	/*
+	 * Paranoia check: There can only be one fsp_lease per lease
+	 * key
+	 */
+	SMB_ASSERT(fsp->lease == lease);
+
 	lck = get_existing_share_mode_lock(
 			talloc_tos(), fsp->file_id);
 	if (lck == NULL) {
-- 
2.11.0


From 61a22bdad396a1ae21136a037e500332acd559cd Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 13 Sep 2018 19:36:47 +0200
Subject: [PATCH 04/12] smbd: Avoid casts in DEBUG statements

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/locking.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 91e32ced304..a31a4c2b170 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -998,8 +998,9 @@ NTSTATUS downgrade_share_lease(struct smbd_server_connection *sconn,
 	l = &d->leases[i];
 
 	if (!l->breaking) {
-		DEBUG(1, ("Attempt to break from %d to %d - but we're not in breaking state\n",
-			   (int)l->current_state, (int)new_lease_state));
+		DBG_WARNING("Attempt to break from %"PRIu32" to %"PRIu32" - "
+			    "but we're not in breaking state\n",
+			    l->current_state, new_lease_state);
 		return NT_STATUS_UNSUCCESSFUL;
 	}
 
@@ -1008,9 +1009,10 @@ NTSTATUS downgrade_share_lease(struct smbd_server_connection *sconn,
 	 * must be a strict bitwise superset of new_lease_state
 	 */
 	if ((new_lease_state & l->breaking_to_requested) != new_lease_state) {
-		DEBUG(1, ("Attempt to upgrade from %d to %d - expected %d\n",
-			   (int)l->current_state, (int)new_lease_state,
-			   (int)l->breaking_to_requested));
+		DBG_WARNING("Attempt to upgrade from %"PRIu32" to %"PRIu32" "
+			    "- expected %"PRIu32"\n",
+			    l->current_state, new_lease_state,
+			    l->breaking_to_requested);
 		return NT_STATUS_REQUEST_NOT_ACCEPTED;
 	}
 
@@ -1020,10 +1022,11 @@ NTSTATUS downgrade_share_lease(struct smbd_server_connection *sconn,
 	}
 
 	if ((new_lease_state & ~l->breaking_to_required) != 0) {
-		DEBUG(5, ("lease state %d not fully broken from %d to %d\n",
-			   (int)new_lease_state,
-			   (int)l->current_state,
-			   (int)l->breaking_to_required));
+		DBG_INFO("lease state %"PRIu32" not fully broken from "
+			 "%"PRIu32" to %"PRIu32"\n",
+			 new_lease_state,
+			 l->current_state,
+			 l->breaking_to_required);
 		l->breaking_to_requested = l->breaking_to_required;
 		if (l->current_state & (~SMB2_LEASE_READ)) {
 			/*
@@ -1037,9 +1040,11 @@ NTSTATUS downgrade_share_lease(struct smbd_server_connection *sconn,
 		return NT_STATUS_OPLOCK_BREAK_IN_PROGRESS;
 	}
 
-	DEBUG(10, ("breaking from %d to %d - expected %d\n",
-		   (int)l->current_state, (int)new_lease_state,
-		   (int)l->breaking_to_requested));
+	DBG_DEBUG("breaking from %"PRIu32" to %"PRIu32" - "
+		  "expected %"PRIu32"\n",
+		  l->current_state,
+		  new_lease_state,
+		  l->breaking_to_requested);
 
 	l->breaking_to_requested = 0;
 	l->breaking_to_required = 0;
-- 
2.11.0


From 639d93d2155da5fcbd532854e71334558d9a5b74 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 10 Sep 2018 12:55:48 +0200
Subject: [PATCH 05/12] smbd: Avoid casts in DEBUG statements

Some indentation changed, best viewed with "git show -w"

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/share_mode_lock.c | 115 +++++++++++++++++++-------------------
 1 file changed, 56 insertions(+), 59 deletions(-)

diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 5596dfbcc45..6fe75490d3b 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -151,10 +151,10 @@ static void share_mode_memcache_delete(struct share_mode_data *d)
 {
 	const DATA_BLOB key = memcache_key(&d->id);
 
-	DEBUG(10,("deleting entry for file %s seq 0x%llu key %s\n",
-		d->base_name,
-		(unsigned long long) d->sequence_number,
-		file_id_string(talloc_tos(), &d->id)));
+	DBG_DEBUG("deleting entry for file %s seq %"PRIx64" key %s\n",
+		  d->base_name,
+		  d->sequence_number,
+		  file_id_string(talloc_tos(), &d->id));
 
 	memcache_delete(NULL,
 			SHARE_MODE_LOCK_CACHE,
@@ -165,10 +165,10 @@ static void share_mode_memcache_store(struct share_mode_data *d)
 {
 	const DATA_BLOB key = memcache_key(&d->id);
 
-	DEBUG(10,("stored entry for file %s seq 0x%llu key %s\n",
-		d->base_name,
-		(unsigned long long) d->sequence_number,
-		file_id_string(talloc_tos(), &d->id)));
+	DBG_DEBUG("stored entry for file %s seq %"PRIx64" key %s\n",
+		  d->base_name,
+		  d->sequence_number,
+		  file_id_string(talloc_tos(), &d->id));
 
 	/* Ensure everything stored in the cache is pristine. */
 	d->modified = false;
@@ -250,11 +250,11 @@ static struct share_mode_data *share_mode_memcache_fetch(TALLOC_CTX *mem_ctx,
 
 	d = (struct share_mode_data *)ptr;
 	if (d->sequence_number != sequence_number) {
-		DEBUG(10,("seq changed (cached 0x%llu) (new 0x%llu) "
-			"for key %s\n",
-			(unsigned long long)d->sequence_number,
-			(unsigned long long)sequence_number,
-			file_id_string(mem_ctx, &id)));
+		DBG_DEBUG("seq changed (cached %"PRIx64") (new %"PRIx64") "
+			  "for key %s\n",
+			  d->sequence_number,
+			  sequence_number,
+			  file_id_string(mem_ctx, &id));
 		/* Cache out of date. Remove entry. */
 		memcache_delete(NULL,
 			SHARE_MODE_LOCK_CACHE,
@@ -279,10 +279,10 @@ static struct share_mode_data *share_mode_memcache_fetch(TALLOC_CTX *mem_ctx,
 	/* And reset the destructor to none. */
 	talloc_set_destructor(d, NULL);
 
-	DEBUG(10,("fetched entry for file %s seq 0x%llu key %s\n",
-		d->base_name,
-		(unsigned long long)d->sequence_number,
-		file_id_string(mem_ctx, &id)));
+	DBG_DEBUG("fetched entry for file %s seq %"PRIx64" key %s\n",
+		  d->base_name,
+		  d->sequence_number,
+		  file_id_string(mem_ctx, &id));
 
 	return d;
 }
@@ -948,21 +948,20 @@ bool share_mode_cleanup_disconnected(struct file_id fid,
 			goto done;
 		}
 		if (open_persistent_id != entry->share_file_id) {
-			DEBUG(5, ("share_mode_cleanup_disconnected: "
-				  "entry for file "
-				  "(file-id='%s', servicepath='%s', "
-				  "base_name='%s%s%s') "
-				  "has share_file_id %llu but expected %llu"
-				  "==> do not cleanup\n",
-				  file_id_string(frame, &fid),
-				  data->servicepath,
-				  data->base_name,
-				  (data->stream_name == NULL)
-				  ? "" : "', stream_name='",
-				  (data->stream_name == NULL)
-				  ? "" : data->stream_name,
-				  (unsigned long long)entry->share_file_id,
-				  (unsigned long long)open_persistent_id));
+			DBG_INFO("entry for file "
+				 "(file-id='%s', servicepath='%s', "
+				 "base_name='%s%s%s') "
+				 "has share_file_id %"PRIu64" but expected "
+				 "%"PRIu64"==> do not cleanup\n",
+				 file_id_string(frame, &fid),
+				 data->servicepath,
+				 data->base_name,
+				 (data->stream_name == NULL)
+				 ? "" : "', stream_name='",
+				 (data->stream_name == NULL)
+				 ? "" : data->stream_name,
+				 entry->share_file_id,
+				 open_persistent_id);
 			goto done;
 		}
 	}
@@ -979,36 +978,34 @@ bool share_mode_cleanup_disconnected(struct file_id fid,
 
 	ok = brl_cleanup_disconnected(fid, open_persistent_id);
 	if (!ok) {
-		DEBUG(10, ("share_mode_cleanup_disconnected: "
-			   "failed to clean up byte range locks associated "
-			   "with file (file-id='%s', servicepath='%s', "
-			   "base_name='%s%s%s') and open_persistent_id %llu "
-			   "==> do not cleanup\n",
-			   file_id_string(frame, &fid),
-			   data->servicepath,
-			   data->base_name,
-			   (data->stream_name == NULL)
-			   ? "" : "', stream_name='",
-			   (data->stream_name == NULL)
-			   ? "" : data->stream_name,
-			   (unsigned long long)open_persistent_id));
+		DBG_DEBUG("failed to clean up byte range locks associated "
+			  "with file (file-id='%s', servicepath='%s', "
+			  "base_name='%s%s%s') and open_persistent_id %"PRIu64" "
+			  "==> do not cleanup\n",
+			  file_id_string(frame, &fid),
+			  data->servicepath,
+			  data->base_name,
+			  (data->stream_name == NULL)
+			  ? "" : "', stream_name='",
+			  (data->stream_name == NULL)
+			  ? "" : data->stream_name,
+			  open_persistent_id);
 		goto done;
 	}
 
-	DEBUG(10, ("share_mode_cleanup_disconnected: "
-		   "cleaning up %u entries for file "
-		   "(file-id='%s', servicepath='%s', "
-		   "base_name='%s%s%s') "
-		   "from open_persistent_id %llu\n",
-		   data->num_share_modes,
-		   file_id_string(frame, &fid),
-		   data->servicepath,
-		   data->base_name,
-		   (data->stream_name == NULL)
-		   ? "" : "', stream_name='",
-		   (data->stream_name == NULL)
-		   ? "" : data->stream_name,
-		   (unsigned long long)open_persistent_id));
+	DBG_DEBUG("cleaning up %u entries for file "
+		  "(file-id='%s', servicepath='%s', "
+		  "base_name='%s%s%s') "
+		  "from open_persistent_id %"PRIu64"\n",
+		  data->num_share_modes,
+		  file_id_string(frame, &fid),
+		  data->servicepath,
+		  data->base_name,
+		  (data->stream_name == NULL)
+		  ? "" : "', stream_name='",
+		  (data->stream_name == NULL)
+		  ? "" : data->stream_name,
+		  open_persistent_id);
 
 	data->num_share_modes = 0;
 	data->num_leases = 0;
-- 
2.11.0


From f8766b9be8dada32737030c15ed6947ce38b8b36 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 13 Sep 2018 06:21:37 +0200
Subject: [PATCH 06/12] smbd: Remove "file_sync_all" function

Replace with a call to files_forall. Why? I just came across this
function that only has one pretty obscure user. This does not justify
a full library function, IMHO at least.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/files.c | 16 ----------------
 source3/smbd/proto.h |  1 -
 source3/smbd/reply.c | 19 ++++++++++++++++++-
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index 303ab7bb926..397baea84cb 100644
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -455,22 +455,6 @@ bool file_find_subpath(files_struct *dir_fsp)
 }
 
 /****************************************************************************
- Sync open files on a connection.
-****************************************************************************/
-
-void file_sync_all(connection_struct *conn)
-{
-	files_struct *fsp, *next;
-
-	for (fsp=conn->sconn->files; fsp; fsp=next) {
-		next=fsp->next;
-		if ((conn == fsp->conn) && (fsp->fh->fd != -1)) {
-			sync_file(conn, fsp, True /* write through */);
-		}
-	}
-}
-
-/****************************************************************************
  Free up a fsp.
 ****************************************************************************/
 
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 5399c5ab483..d49cd5c2251 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -402,7 +402,6 @@ struct files_struct *file_find_one_fsp_from_lease_key(
 	struct smbd_server_connection *sconn,
 	const struct smb2_lease_key *lease_key);
 bool file_find_subpath(files_struct *dir_fsp);
-void file_sync_all(connection_struct *conn);
 void fsp_free(files_struct *fsp);
 void file_free(struct smb_request *req, files_struct *fsp);
 files_struct *file_fsp(struct smb_request *req, uint16_t fid);
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 4c6456c88f6..49ca1d70411 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -5267,6 +5267,23 @@ void reply_lseek(struct smb_request *req)
 	return;
 }
 
+static struct files_struct *file_sync_one_fn(struct files_struct *fsp,
+					     void *private_data)
+{
+	connection_struct *conn = talloc_get_type_abort(
+		private_data, connection_struct);
+
+	if (conn != fsp->conn) {
+		return NULL;
+	}
+	if (fsp->fh->fd == -1) {
+		return NULL;
+	}
+	sync_file(conn, fsp, True /* write through */);
+
+	return NULL;
+}
+
 /****************************************************************************
  Reply to a flush.
 ****************************************************************************/
@@ -5292,7 +5309,7 @@ void reply_flush(struct smb_request *req)
 	}
 
 	if (!fsp) {
-		file_sync_all(conn);
+		files_forall(req->sconn, file_sync_one_fn, conn);
 	} else {
 		NTSTATUS status = sync_file(conn, fsp, True);
 		if (!NT_STATUS_IS_OK(status)) {
-- 
2.11.0


From 4b8e120f50a1dd18bc8c5aec2348b1ee83b0437c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 22 Oct 2016 13:59:12 +0200
Subject: [PATCH 07/12] tevent: Fix a docu typo

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/tevent/doc/tevent_request.dox | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/tevent/doc/tevent_request.dox b/lib/tevent/doc/tevent_request.dox
index 7fcfb55e8fb..e1e45b170a5 100644
--- a/lib/tevent/doc/tevent_request.dox
+++ b/lib/tevent/doc/tevent_request.dox
@@ -4,7 +4,7 @@
 
 A specific feature of the library is the tevent request API that provides for
 asynchronous computation and allows much more interconnected working and
-cooperation among func- tions and events. When working with tevent request it
+cooperation among functions and events. When working with tevent request it
 is possible to nest one event under another and handle them bit by bit. This
 enables the creation of sequences of steps, and provides an opportunity to
 prepare for all problems which may unexpectedly happen within the different
-- 
2.11.0


From 76c0529efaaaca1b5c85de3222ee2252fd552590 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 10 Sep 2018 12:20:10 +0200
Subject: [PATCH 08/12] smbd: Fix a false comment

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/locking.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index a31a4c2b170..8ee92370aed 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -879,8 +879,7 @@ struct share_mode_entry *find_share_mode_entry(
 }
 
 /*******************************************************************
- Del the share mode of a file for this process. Return the number of
- entries left.
+ Del the share mode of a file for this process.
 ********************************************************************/
 
 bool del_share_mode(struct share_mode_lock *lck, files_struct *fsp)
-- 
2.11.0


From 557785bf23e475ce84962b6a2f7fbf0c3a97f952 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 10 Sep 2018 14:50:40 +0200
Subject: [PATCH 09/12] smbd: Simplify share_mode_lock.c

"the_lock_id" is not required here. The share mode data carry the file
id, so use that.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/share_mode_lock.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 6fe75490d3b..2850e502bda 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -560,12 +560,10 @@ static struct share_mode_lock *get_share_mode_lock_internal(
  * talloc_reference.
  */
 static struct share_mode_lock *the_lock;
-static struct file_id the_lock_id;
 
 static int the_lock_destructor(struct share_mode_lock *l)
 {
 	the_lock = NULL;
-	ZERO_STRUCT(the_lock_id);
 	return 0;
 }
 
@@ -595,9 +593,8 @@ struct share_mode_lock *get_share_mode_lock(
 			goto fail;
 		}
 		talloc_set_destructor(the_lock, the_lock_destructor);
-		the_lock_id = id;
 	} else {
-		if (!file_id_equal(&the_lock_id, &id)) {
+		if (!file_id_equal(&the_lock->data->id, &id)) {
 			DEBUG(1, ("Can not lock two share modes "
 				  "simultaneously\n"));
 			goto fail;
-- 
2.11.0


From af6468864750e140cc0e5e6d9bcaeb2c28c0bca2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 10 Sep 2018 15:14:00 +0200
Subject: [PATCH 10/12] smbd: Simplify parse_share_modes

Since 823bc4c07ad pidl initializes the [skip] entries

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/locking/share_mode_lock.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index 2850e502bda..919e74c0851 100644
--- a/source3/locking/share_mode_lock.c
+++ b/source3/locking/share_mode_lock.c
@@ -297,7 +297,6 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx,
 {
 	struct share_mode_data *d;
 	enum ndr_err_code ndr_err;
-	uint32_t i;
 	DATA_BLOB blob;
 
 	blob.data = dbuf.dptr;
@@ -323,17 +322,6 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx,
 		goto fail;
 	}
 
-	/*
-	 * Initialize the values that are [skip] or [ignore]
-	 * in the idl. The NDR code does not initialize them.
-	 */
-
-	for (i=0; i<d->num_share_modes; i++) {
-		d->share_modes[i].stale = false;
-	}
-	d->modified = false;
-	d->fresh = false;
-
 	if (DEBUGLEVEL >= 10) {
 		DEBUG(10, ("parse_share_modes:\n"));
 		NDR_PRINT_DEBUG(share_mode_data, d);
@@ -786,7 +774,6 @@ static int share_mode_traverse_fn(struct db_record *rec, void *_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;
@@ -819,10 +806,6 @@ static int share_mode_traverse_fn(struct db_record *rec, void *_state)
 		return 0;
 	}
 
-	for (i=0; i<d->num_share_modes; i++) {
-		d->share_modes[i].stale = false;
-	}
-
 	if (DEBUGLEVEL > 10) {
 		DEBUG(11, ("parse_share_modes:\n"));
 		NDR_PRINT_DEBUG(share_mode_data, d);
-- 
2.11.0


From 33ddd47e7639e83a00bce5ed55c6c4ed72e93e5f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 8 Aug 2018 14:20:58 +0200
Subject: [PATCH 11/12] streams_xattr: Make error handling more obvious

Do the NULL check right after the alloc call

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/modules/vfs_streams_xattr.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/source3/modules/vfs_streams_xattr.c b/source3/modules/vfs_streams_xattr.c
index 8714007cb8d..0743959e957 100644
--- a/source3/modules/vfs_streams_xattr.c
+++ b/source3/modules/vfs_streams_xattr.c
@@ -503,6 +503,11 @@ static int streams_xattr_open(vfs_handle_struct *handle,
 
         sio->xattr_name = talloc_strdup(VFS_MEMCTX_FSP_EXTENSION(handle, fsp),
 					xattr_name);
+	if (sio->xattr_name == NULL) {
+		errno = ENOMEM;
+		goto fail;
+	}
+
 	/*
 	 * so->base needs to be a copy of fsp->fsp_name->base_name,
 	 * making it identical to streams_xattr_recheck(). If the
@@ -512,15 +517,15 @@ static int streams_xattr_open(vfs_handle_struct *handle,
 	 */
         sio->base = talloc_strdup(VFS_MEMCTX_FSP_EXTENSION(handle, fsp),
 				  fsp->fsp_name->base_name);
-	sio->fsp_name_ptr = fsp->fsp_name;
-	sio->handle = handle;
-	sio->fsp = fsp;
-
-	if ((sio->xattr_name == NULL) || (sio->base == NULL)) {
+	if (sio->base == NULL) {
 		errno = ENOMEM;
 		goto fail;
 	}
 
+	sio->fsp_name_ptr = fsp->fsp_name;
+	sio->handle = handle;
+	sio->fsp = fsp;
+
 	return fakefd;
 
  fail:
-- 
2.11.0


From d9fb1370bb844ab8893ff470b6da8b80c9b4d899 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 8 Aug 2018 14:20:58 +0200
Subject: [PATCH 12/12] streams_xattr: Make error handling more obvious

Do the NULL check right after the alloc call

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/modules/vfs_streams_xattr.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/source3/modules/vfs_streams_xattr.c b/source3/modules/vfs_streams_xattr.c
index 0743959e957..30459fec4bf 100644
--- a/source3/modules/vfs_streams_xattr.c
+++ b/source3/modules/vfs_streams_xattr.c
@@ -190,16 +190,21 @@ static bool streams_xattr_recheck(struct stream_io *sio)
 	TALLOC_FREE(sio->base);
 	sio->xattr_name = talloc_strdup(VFS_MEMCTX_FSP_EXTENSION(sio->handle, sio->fsp),
 					xattr_name);
-        sio->base = talloc_strdup(VFS_MEMCTX_FSP_EXTENSION(sio->handle, sio->fsp),
-                                  sio->fsp->fsp_name->base_name);
-	sio->fsp_name_ptr = sio->fsp->fsp_name;
-
+	if (sio->xattr_name == NULL) {
+		DBG_DEBUG("sio->xattr_name==NULL\n");
+		return false;
+	}
 	TALLOC_FREE(xattr_name);
 
-	if ((sio->xattr_name == NULL) || (sio->base == NULL)) {
+	sio->base = talloc_strdup(VFS_MEMCTX_FSP_EXTENSION(sio->handle, sio->fsp),
+				  sio->fsp->fsp_name->base_name);
+	if (sio->base == NULL) {
+		DBG_DEBUG("sio->base==NULL\n");
 		return false;
 	}
 
+	sio->fsp_name_ptr = sio->fsp->fsp_name;
+
 	return true;
 }
 
-- 
2.11.0



More information about the samba-technical mailing list