[PATCHSET]: Simplify set_share_mode a bit
Volker Lendecke
Volker.Lendecke at SerNet.DE
Mon Oct 7 14:03:51 MDT 2013
Hi!
$SUBJECT says it all. Please review & push!
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
*****************************************************************
visit us on it-sa:IT security exhibitions in Nürnberg, Germany
October 8th - 10th 2013, hall 12, booth 333
free tickets available via code 270691 on: www.it-sa.de/gutschein
******************************************************************
-------------- next part --------------
From f03bed92c07d9b2dd0ad8a3f31205ade2c124188 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Sep 2013 18:39:27 -0700
Subject: [PATCH 1/5] smbd: Change parameter from unsigned to uint32_t
share_mode_stale_pid internally only has to deal with uint32_t. Make
the parameter match this.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/locking.c | 2 +-
source3/locking/proto.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index d0b6eaf..d701964 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -636,7 +636,7 @@ bool is_valid_share_mode_entry(const struct share_mode_entry *e)
* being used, we need to make sure the corresponding process still
* exists.
*/
-bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx)
+bool share_mode_stale_pid(struct share_mode_data *d, uint32_t idx)
{
struct share_mode_entry *e;
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index 1573f6b..dde0be4 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -170,7 +170,7 @@ void get_file_infos(struct file_id id,
bool *delete_on_close,
struct timespec *write_time);
bool is_valid_share_mode_entry(const struct share_mode_entry *e);
-bool share_mode_stale_pid(struct share_mode_data *d, unsigned idx);
+bool share_mode_stale_pid(struct share_mode_data *d, uint32_t idx);
void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
uid_t uid, uint64_t mid, uint16 op_type);
bool del_share_mode(struct share_mode_lock *lck, files_struct *fsp);
--
1.7.9.5
From 8be3fa6c38c0ca68cb6ee1bba7f2d468badb696b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 14 Sep 2013 13:48:03 +0200
Subject: [PATCH 2/5] smbd: Make add_share_mode return bool
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/locking.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index d701964..f584e0c 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -717,12 +717,21 @@ static void fill_share_mode_entry(struct share_mode_entry *e,
e->name_hash = fsp->name_hash;
}
-static void add_share_mode_entry(struct share_mode_data *d,
+static bool add_share_mode_entry(struct share_mode_data *d,
const struct share_mode_entry *entry)
{
- ADD_TO_ARRAY(d, struct share_mode_entry, *entry,
- &d->share_modes, &d->num_share_modes);
- d->modified = True;
+ struct share_mode_entry *tmp;
+
+ tmp = talloc_realloc(d, d->share_modes, struct share_mode_entry,
+ d->num_share_modes+1);
+ if (tmp == NULL) {
+ return false;
+ }
+ d->share_modes = tmp;
+ d->share_modes[d->num_share_modes] = *entry;
+ d->num_share_modes += 1;
+ d->modified = true;
+ return true;
}
void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
--
1.7.9.5
From d15dfe5502e9c43b8ef2ef3e6c4f2634dd9db845 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 14 Sep 2013 13:49:14 +0200
Subject: [PATCH 3/5] smbd: Convert set_share_mode to return bool for success
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/locking.c | 4 ++--
source3/locking/proto.h | 2 +-
source3/smbd/open.c | 19 ++++++++++++++-----
3 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index f584e0c..055d0f8 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -734,12 +734,12 @@ static bool add_share_mode_entry(struct share_mode_data *d,
return true;
}
-void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
+bool set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
uid_t uid, uint64_t mid, uint16 op_type)
{
struct share_mode_entry entry;
fill_share_mode_entry(&entry, fsp, uid, mid, op_type);
- add_share_mode_entry(lck->data, &entry);
+ return add_share_mode_entry(lck->data, &entry);
}
/*******************************************************************
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index dde0be4..02e2bf5 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -171,7 +171,7 @@ void get_file_infos(struct file_id id,
struct timespec *write_time);
bool is_valid_share_mode_entry(const struct share_mode_entry *e);
bool share_mode_stale_pid(struct share_mode_data *d, uint32_t idx);
-void set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
+bool set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
uid_t uid, uint64_t mid, uint16 op_type);
bool del_share_mode(struct share_mode_lock *lck, files_struct *fsp);
bool mark_share_mode_disconnected(struct share_mode_lock *lck,
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 858d2be..55f2fb2 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2681,9 +2681,13 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
fsp->oplock_type = NO_OPLOCK;
}
- set_share_mode(lck, fsp, get_current_uid(conn),
- req ? req->mid : 0,
- fsp->oplock_type);
+ if (!set_share_mode(lck, fsp, get_current_uid(conn),
+ req ? req->mid : 0,
+ fsp->oplock_type)) {
+ TALLOC_FREE(lck);
+ fd_close(fsp);
+ return NT_STATUS_NO_MEMORY;
+ }
/* Handle strange delete on close create semantics. */
if (create_options & FILE_DELETE_ON_CLOSE) {
@@ -3173,8 +3177,13 @@ static NTSTATUS open_directory(connection_struct *conn,
return status;
}
- set_share_mode(lck, fsp, get_current_uid(conn),
- req ? req->mid : 0, NO_OPLOCK);
+ if (!set_share_mode(lck, fsp, get_current_uid(conn),
+ req ? req->mid : 0, NO_OPLOCK)) {
+ TALLOC_FREE(lck);
+ fd_close(fsp);
+ file_free(req, fsp);
+ return NT_STATUS_NO_MEMORY;
+ }
/* For directories the delete on close bit at open time seems
always to be honored on close... See test 19 in Samba4 BASE-DELETE. */
--
1.7.9.5
From 80e1a229377bc75bc83f02137f736361cd632cd4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 16 Sep 2013 13:58:54 -0700
Subject: [PATCH 4/5] smbd: Simplify find_share_mode_entry callers
All callers used fill_share_mode_entry before calling
find_share_mode_entry. Remove that requirement.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/locking.c | 71 ++++++++++++++++-----------------------------
1 file changed, 25 insertions(+), 46 deletions(-)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index 055d0f8..d7ba653 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -742,40 +742,31 @@ bool set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
return add_share_mode_entry(lck->data, &entry);
}
-/*******************************************************************
- Check if two share mode entries are identical, ignoring oplock
- and mid info and desired_access. (Removed paranoia test - it's
- not automatically a logic error if they are identical. JRA.)
-********************************************************************/
-
-static bool share_modes_identical(const struct share_mode_entry *e1,
- const struct share_mode_entry *e2)
-{
- /* We used to check for e1->share_access == e2->share_access here
- as well as the other fields but 2 different DOS or FCB opens
- sharing the same share mode entry may validly differ in
- fsp->share_access field. */
-
- return (serverid_equal(&e1->pid, &e2->pid) &&
- file_id_equal(&e1->id, &e2->id) &&
- e1->share_file_id == e2->share_file_id );
-}
-
static struct share_mode_entry *find_share_mode_entry(
- struct share_mode_data *d, const struct share_mode_entry *entry)
+ struct share_mode_lock *lck, files_struct *fsp)
{
+ struct share_mode_data *d = lck->data;
+ struct server_id pid;
int i;
- if (!is_valid_share_mode_entry(entry)) {
- return NULL;
- }
+ pid = messaging_server_id(fsp->conn->sconn->msg_ctx);
for (i=0; i<d->num_share_modes; i++) {
struct share_mode_entry *e = &d->share_modes[i];
- if (is_valid_share_mode_entry(e) &&
- share_modes_identical(e, entry)) {
- return e;
+
+ if (!is_valid_share_mode_entry(e)) {
+ continue;
+ }
+ if (!serverid_equal(&pid, &e->pid)) {
+ continue;
}
+ if (!file_id_equal(&fsp->file_id, &e->id)) {
+ continue;
+ }
+ if (fsp->fh->gen_id != e->share_file_id) {
+ continue;
+ }
+ return e;
}
return NULL;
}
@@ -787,12 +778,9 @@ static struct share_mode_entry *find_share_mode_entry(
bool del_share_mode(struct share_mode_lock *lck, files_struct *fsp)
{
- struct share_mode_entry entry, *e;
-
- /* Don't care about the pid owner being correct here - just a search. */
- fill_share_mode_entry(&entry, fsp, (uid_t)-1, 0, NO_OPLOCK);
+ struct share_mode_entry *e;
- e = find_share_mode_entry(lck->data, &entry);
+ e = find_share_mode_entry(lck, fsp);
if (e == NULL) {
return False;
}
@@ -805,7 +793,7 @@ bool del_share_mode(struct share_mode_lock *lck, files_struct *fsp)
bool mark_share_mode_disconnected(struct share_mode_lock *lck,
struct files_struct *fsp)
{
- struct share_mode_entry entry, *e;
+ struct share_mode_entry *e;
if (lck->data->num_share_modes != 1) {
return false;
@@ -818,10 +806,7 @@ bool mark_share_mode_disconnected(struct share_mode_lock *lck,
return false;
}
- /* Don't care about the pid owner being correct here - just a search. */
- fill_share_mode_entry(&entry, fsp, (uid_t)-1, 0, NO_OPLOCK);
-
- e = find_share_mode_entry(lck->data, &entry);
+ e = find_share_mode_entry(lck, fsp);
if (e == NULL) {
return false;
}
@@ -846,12 +831,9 @@ bool mark_share_mode_disconnected(struct share_mode_lock *lck,
bool remove_share_oplock(struct share_mode_lock *lck, files_struct *fsp)
{
- struct share_mode_entry entry, *e;
-
- /* Don't care about the pid owner being correct here - just a search. */
- fill_share_mode_entry(&entry, fsp, (uid_t)-1, 0, NO_OPLOCK);
+ struct share_mode_entry *e;
- e = find_share_mode_entry(lck->data, &entry);
+ e = find_share_mode_entry(lck, fsp);
if (e == NULL) {
return False;
}
@@ -879,12 +861,9 @@ bool remove_share_oplock(struct share_mode_lock *lck, files_struct *fsp)
bool downgrade_share_oplock(struct share_mode_lock *lck, files_struct *fsp)
{
- struct share_mode_entry entry, *e;
-
- /* Don't care about the pid owner being correct here - just a search. */
- fill_share_mode_entry(&entry, fsp, (uid_t)-1, 0, NO_OPLOCK);
+ struct share_mode_entry *e;
- e = find_share_mode_entry(lck->data, &entry);
+ e = find_share_mode_entry(lck, fsp);
if (e == NULL) {
return False;
}
--
1.7.9.5
From 6e9c8e973f6aba4b938b1e0493c7954a62cab75e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 16 Sep 2013 14:02:48 -0700
Subject: [PATCH 5/5] smbd: Simplify set_share_mode
With the find_share_mode simplification we don't need fill_share_mode anymore.
So this coalesces add_share_mode as well.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/locking/locking.c | 45 +++++++++++++++------------------------------
1 file changed, 15 insertions(+), 30 deletions(-)
diff --git a/source3/locking/locking.c b/source3/locking/locking.c
index d7ba653..b9db27c 100644
--- a/source3/locking/locking.c
+++ b/source3/locking/locking.c
@@ -693,14 +693,22 @@ bool share_mode_stale_pid(struct share_mode_data *d, uint32_t idx)
return true;
}
-/*******************************************************************
- Fill a share mode entry.
-********************************************************************/
-
-static void fill_share_mode_entry(struct share_mode_entry *e,
- files_struct *fsp,
- uid_t uid, uint64_t mid, uint16 op_type)
+bool set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
+ uid_t uid, uint64_t mid, uint16 op_type)
{
+ struct share_mode_data *d = lck->data;
+ struct share_mode_entry *tmp, *e;
+
+ tmp = talloc_realloc(d, d->share_modes, struct share_mode_entry,
+ d->num_share_modes+1);
+ if (tmp == NULL) {
+ return false;
+ }
+ d->share_modes = tmp;
+ e = &d->share_modes[d->num_share_modes];
+ d->num_share_modes += 1;
+ d->modified = true;
+
ZERO_STRUCTP(e);
e->pid = messaging_server_id(fsp->conn->sconn->msg_ctx);
e->share_access = fsp->share_access;
@@ -715,33 +723,10 @@ static void fill_share_mode_entry(struct share_mode_entry *e,
e->uid = (uint32)uid;
e->flags = fsp->posix_open ? SHARE_MODE_FLAG_POSIX_OPEN : 0;
e->name_hash = fsp->name_hash;
-}
-
-static bool add_share_mode_entry(struct share_mode_data *d,
- const struct share_mode_entry *entry)
-{
- struct share_mode_entry *tmp;
- tmp = talloc_realloc(d, d->share_modes, struct share_mode_entry,
- d->num_share_modes+1);
- if (tmp == NULL) {
- return false;
- }
- d->share_modes = tmp;
- d->share_modes[d->num_share_modes] = *entry;
- d->num_share_modes += 1;
- d->modified = true;
return true;
}
-bool set_share_mode(struct share_mode_lock *lck, files_struct *fsp,
- uid_t uid, uint64_t mid, uint16 op_type)
-{
- struct share_mode_entry entry;
- fill_share_mode_entry(&entry, fsp, uid, mid, op_type);
- return add_share_mode_entry(lck->data, &entry);
-}
-
static struct share_mode_entry *find_share_mode_entry(
struct share_mode_lock *lck, files_struct *fsp)
{
--
1.7.9.5
More information about the samba-technical
mailing list