The branch, master has been updated
via 1909fa3ca1a smbd: Simplify inotify_handler()
via 5b5d3e8343f smbd: Simplify inotify_dispatch()
via e62f0b723b4 smbd: Handle what mkdir_internals() puts on inotify
via 245d80c6a98 smbd: Handle split MOVED_FROM/MOVED_TO inotify events
via 2b317dd8000 smbd: Simplify inotify_handler()
via 4ed098b6730 smbd: Modernize a DEBUG
via a8980e68d3a smbd: Initialize all members of struct
inotify_watch_context
via 2d35eab05fb smbd: Fix a typo
via 89038eafd90 smbd: Fix some whitespace
from 6f0ae60428a CVE-2025-0620: smbd: smbd doesn't pick up group
membership changes when re-authenticating an expired SMB session
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 1909fa3ca1ab3cdd476c7af1d07876cbca53d4e4
Author: Volker Lendecke <[email protected]>
Date: Wed May 28 12:15:01 2025 +0200
smbd: Simplify inotify_handler()
Now that we don't have to calculate "e2" anymore, we can simplify the
loop walking the buffer.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15864
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Ralph Boehme <[email protected]>
Autobuild-User(master): Ralph Böhme <[email protected]>
Autobuild-Date(master): Mon Jun 2 18:10:20 UTC 2025 on atb-devel-224
commit 5b5d3e8343fe04eb5d0fbee833ad341cfb5d553d
Author: Volker Lendecke <[email protected]>
Date: Tue May 27 17:12:05 2025 +0200
smbd: Simplify inotify_dispatch()
inotify_dispatch() does not need the additional arguments anymore
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15864
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Ralph Boehme <[email protected]>
commit e62f0b723b46c59a351457fad108334b75136eaa
Author: Volker Lendecke <[email protected]>
Date: Tue May 27 14:29:55 2025 +0200
smbd: Handle what mkdir_internals() puts on inotify
Properly handle the TMPNAME events
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15864
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Ralph Boehme <[email protected]>
commit 245d80c6a98349a656f5a79ffd3e15e6255df243
Author: Volker Lendecke <[email protected]>
Date: Tue May 27 13:46:30 2025 +0200
smbd: Handle split MOVED_FROM/MOVED_TO inotify events
inotify(7) explicitly states that MOVED_FROM and MOVED_TO are not
atomically added to the inotify queue. Save MOVED_FROM in the watch
descriptor and try to match a later MOVED_TO with that. If no
corresponding MOVED_TO comes, send out a NOTIFY_ACTION_REMOVED after
100 milliseconds.
This is all racy, as we only catch directly corresponding FROM and TO
events. According to inotify(7) anything can come in between FROM and
TO, in particular other FROM/TO pairs. The best we can do I think is
turn dislocated FROM/TO pairs as REMOVED/ADDED instead of the original
Windows rename OLD_NAME/NEW_NAME.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15864
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Ralph Boehme <[email protected]>
commit 2b317dd80002b7f87710317118bca35a20e85f4c
Author: Volker Lendecke <[email protected]>
Date: Fri May 23 15:22:39 2025 +0200
smbd: Simplify inotify_handler()
[sizeof(struct inotify_event) + NAME_MAX + 1] is the recommended
buffer size in "man 7 inotify". Pulling everything out of the inotify
buffer makes sense if we would reliably get the two rename events in
the same read-call. Unfortunately this is not the case, even with a
buffer size of 64k I've seen MOVED_FROM and MOVED_to in separate reads
from the socket. We'll have to take care of this situation next. Until
then, we don't have to FIONREAD and then read everything that's
there. Rather go through another event loop, replacing the ioctl with
a epoll_wait syscall.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15864
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Ralph Boehme <[email protected]>
commit 4ed098b6730bb6bd09eaada0c228b29d4b6abb6a
Author: Volker Lendecke <[email protected]>
Date: Mon May 26 12:54:45 2025 +0200
smbd: Modernize a DEBUG
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15864
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Ralph Boehme <[email protected]>
commit a8980e68d3a20c40b386e2a262834715efcf7823
Author: Volker Lendecke <[email protected]>
Date: Mon May 26 11:53:31 2025 +0200
smbd: Initialize all members of struct inotify_watch_context
We don't have talloc_zero()
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15864
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Ralph Boehme <[email protected]>
commit 2d35eab05fbaf7ada825946f7dcca777826918dd
Author: Volker Lendecke <[email protected]>
Date: Wed May 28 12:32:29 2025 +0200
smbd: Fix a typo
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15864
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Ralph Boehme <[email protected]>
commit 89038eafd902d233cb641e76508e7f7d1068e467
Author: Volker Lendecke <[email protected]>
Date: Fri May 23 14:50:15 2025 +0200
smbd: Fix some whitespace
Bug: https://bugzilla.samba.org/show_bug.cgi?id=15864
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Ralph Boehme <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
source3/smbd/notify_inotify.c | 309 ++++++++++++++++++++++++++++++------------
source3/smbd/open.c | 2 +-
2 files changed, 224 insertions(+), 87 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/smbd/notify_inotify.c b/source3/smbd/notify_inotify.c
index 81fafc218dd..4275b221440 100644
--- a/source3/smbd/notify_inotify.c
+++ b/source3/smbd/notify_inotify.c
@@ -1,4 +1,4 @@
-/*
+/*
Unix SMB/CIFS implementation.
Copyright (C) Andrew Tridgell 2006
@@ -24,7 +24,8 @@
#include "includes.h"
#include "../librpc/gen_ndr/notify.h"
#include "smbd/smbd.h"
-#include "lib/util/sys_rw_data.h"
+#include "lib/util/sys_rw.h"
+#include "smbd/globals.h"
#include <sys/inotify.h>
@@ -46,7 +47,7 @@ struct inotify_watch_context {
struct inotify_watch_context *next, *prev;
struct inotify_private *in;
int wd;
- void (*callback)(struct sys_notify_context *ctx,
+ void (*callback)(struct sys_notify_context *ctx,
void *private_data,
struct notify_event *ev,
uint32_t filter);
@@ -54,6 +55,9 @@ struct inotify_watch_context {
uint32_t mask; /* the inotify mask */
uint32_t filter; /* the windows completion filter */
const char *path;
+
+ char *last_mkdir;
+ struct inotify_event *moved_from_event;
};
@@ -162,25 +166,165 @@ static bool filter_match(struct inotify_watch_context *w,
return ok;
}
+static void trigger_orphaned_moved_from(struct inotify_watch_context *w)
+{
+ struct notify_event ne = {};
+ struct inotify_event *e = w->moved_from_event;
+
+ ne = (struct notify_event){
+ .action = NOTIFY_ACTION_REMOVED,
+ .path = e->name,
+ .dir = w->path,
+ };
+
+ w->callback(w->in->ctx,
+ w->private_data,
+ &ne,
+ inotify_map_mask_to_filter(e->mask));
+}
+
+static void moved_from_timeout(struct tevent_context *ev,
+ struct tevent_timer *te,
+ struct timeval now,
+ void *private_data)
+{
+ struct inotify_watch_context *w = talloc_get_type_abort(
+ private_data, struct inotify_watch_context);
+
+ trigger_orphaned_moved_from(w);
+ TALLOC_FREE(w->moved_from_event);
+}
+static void save_moved_from(struct tevent_context *ev,
+ struct inotify_watch_context *w,
+ struct inotify_event *e)
+{
+ if (w->moved_from_event != NULL) {
+ trigger_orphaned_moved_from(w);
+ TALLOC_FREE(w->moved_from_event);
+ }
+
+ w->moved_from_event = talloc_memdup(
+ w, e, sizeof(struct inotify_event) + e->len);
+ if (w->moved_from_event == NULL) {
+ /*
+ * Not much we can do here
+ */
+ return;
+ }
+
+ tevent_add_timer(ev,
+ w->moved_from_event,
+ tevent_timeval_current_ofs(0, 100000),
+ moved_from_timeout,
+ w);
+}
+
+static bool handle_local_rename(struct inotify_watch_context *w,
+ struct inotify_event *to)
+{
+ struct inotify_private *in = w->in;
+ struct inotify_event *from = w->moved_from_event;
+ struct notify_event ne = {};
+ uint32_t filter;
+
+ if ((w->last_mkdir != NULL) && (w->moved_from_event != NULL) &&
+ IS_SMBD_TMPNAME(w->moved_from_event->name, NULL))
+ {
+ if (strcmp(to->name, w->last_mkdir) == 0) {
+ /*
+ * Assume this is a rename() from smbd after a
+ * mkdir of the real target directory. See the
+ * comment about RENAME_NOREPLACE in
+ * mkdir_internals(). We have already sent out
+ * the mkdir notify event, this MOVED_FROM/TO
+ * pair is just internal fluff that the client
+ * should not get wind of via notify.
+ */
+ TALLOC_FREE(w->last_mkdir);
+ TALLOC_FREE(w->moved_from_event);
+ return true;
+ }
+
+ if (strcmp(w->moved_from_event->name, w->last_mkdir) == 0) {
+ /*
+ * Assume this is a renameat2() from smbd's
+ * mkdir_internal().
+ */
+ TALLOC_FREE(w->last_mkdir);
+ TALLOC_FREE(w->moved_from_event);
+
+ /*
+ * Pretend this is not a rename but a new
+ * directory.
+ */
+ to->mask = IN_ISDIR | IN_CREATE;
+
+ return false;
+ }
+ }
+
+ ne = (struct notify_event){
+ .action = NOTIFY_ACTION_OLD_NAME,
+ .path = from->name,
+ .dir = w->path,
+ };
+
+ if (filter_match(w, from)) {
+ filter = inotify_map_mask_to_filter(to->mask);
+ w->callback(in->ctx, w->private_data, &ne, filter);
+ }
+
+ ne = (struct notify_event){
+ .action = NOTIFY_ACTION_NEW_NAME,
+ .path = to->name,
+ .dir = w->path,
+ };
+
+ filter = inotify_map_mask_to_filter(to->mask);
+
+ if (filter_match(w, to)) {
+ w->callback(in->ctx, w->private_data, &ne, filter);
+ }
+
+ if (to->mask & IN_ISDIR) {
+ return true;
+ }
+ if ((w->filter & FILE_NOTIFY_CHANGE_CREATION) == 0) {
+ return true;
+ }
+
+ /*
+ * SMB expects a file rename to generate three events, two for
+ * the rename and the other for a modify of the
+ * destination. Strange!
+ */
+
+ ne.action = NOTIFY_ACTION_MODIFIED;
+ to->mask = IN_ATTRIB;
+
+ if (filter_match(w, to)) {
+ w->callback(in->ctx, w->private_data, &ne, filter);
+ }
+
+ return true;
+}
/*
dispatch one inotify event
the cookies are used to correctly handle renames
*/
-static void inotify_dispatch(struct inotify_private *in,
- struct inotify_event *e,
- int prev_wd,
- uint32_t prev_cookie,
- struct inotify_event *e2)
+static void inotify_dispatch(struct tevent_context *ev,
+ struct inotify_private *in,
+ struct inotify_event *e)
{
struct inotify_watch_context *w, *next;
struct notify_event ne;
uint32_t filter;
- DEBUG(10, ("inotify_dispatch called with mask=%x, name=[%s]\n",
- e->mask, e->len ? e->name : ""));
+ DBG_DEBUG("called with mask=%x, name=[%s]\n",
+ e->mask, e->len ? e->name : "");
/* ignore extraneous events, such as unmount and IN_IGNORED events */
if ((e->mask & (IN_ATTRIB|IN_MODIFY|IN_CREATE|IN_DELETE|
@@ -188,25 +332,50 @@ static void inotify_dispatch(struct inotify_private *in,
return;
}
- /* map the inotify mask to a action. This gets complicated for
- renames */
+ if (e->mask & IN_MOVED_FROM) {
+ for (w = in->watches; w != NULL; w = w->next) {
+ if (e->wd != w->wd) {
+ continue;
+ }
+ save_moved_from(ev, w, e);
+ }
+ return;
+ }
+
+ if (e->mask & IN_MOVED_TO) {
+ for (w = in->watches; w != NULL; w = w->next) {
+ if ((w->wd == e->wd) &&
+ (w->moved_from_event != NULL) &&
+ (w->moved_from_event->cookie == e->cookie))
+ {
+ bool handled = handle_local_rename(w, e);
+ if (handled) {
+ return;
+ }
+ }
+ }
+ }
+
+ if ((e->mask & IN_CREATE) && (e->mask & IN_ISDIR)) {
+ for (w = in->watches; w != NULL; w = w->next) {
+ if (w->wd != e->wd) {
+ continue;
+ }
+ TALLOC_FREE(w->last_mkdir);
+ w->last_mkdir = talloc_strdup(w, e->name);
+ }
+
+ if (IS_SMBD_TMPNAME(e->name, NULL)) {
+ return;
+ }
+ }
+
if (e->mask & IN_CREATE) {
ne.action = NOTIFY_ACTION_ADDED;
} else if (e->mask & IN_DELETE) {
ne.action = NOTIFY_ACTION_REMOVED;
- } else if (e->mask & IN_MOVED_FROM) {
- if (e2 != NULL && e2->cookie == e->cookie &&
- e2->wd == e->wd) {
- ne.action = NOTIFY_ACTION_OLD_NAME;
- } else {
- ne.action = NOTIFY_ACTION_REMOVED;
- }
} else if (e->mask & IN_MOVED_TO) {
- if ((e->cookie == prev_cookie) && (e->wd == prev_wd)) {
- ne.action = NOTIFY_ACTION_NEW_NAME;
- } else {
- ne.action = NOTIFY_ACTION_ADDED;
- }
+ ne.action = NOTIFY_ACTION_ADDED;
} else {
ne.action = NOTIFY_ACTION_MODIFIED;
}
@@ -225,29 +394,6 @@ static void inotify_dispatch(struct inotify_private *in,
w->callback(in->ctx, w->private_data, &ne, filter);
}
}
-
- if ((ne.action == NOTIFY_ACTION_NEW_NAME) &&
- ((e->mask & IN_ISDIR) == 0)) {
-
- /*
- * SMB expects a file rename to generate three events, two for
- * the rename and the other for a modify of the
- * destination. Strange!
- */
-
- ne.action = NOTIFY_ACTION_MODIFIED;
- e->mask = IN_ATTRIB;
-
- for (w=in->watches;w;w=next) {
- next = w->next;
- if (w->wd == e->wd && filter_match(w, e) &&
- !(w->filter & FILE_NOTIFY_CHANGE_CREATION)) {
- ne.dir = w->path;
- w->callback(in->ctx, w->private_data, &ne,
- filter);
- }
- }
- }
}
/*
@@ -258,53 +404,41 @@ static void inotify_handler(struct tevent_context *ev,
struct tevent_fd *fde,
{
struct inotify_private *in = talloc_get_type(private_data,
struct inotify_private);
+ char buf[sizeof(struct inotify_event) + NAME_MAX + 1];
int bufsize = 0;
- struct inotify_event *e0, *e;
- uint32_t prev_cookie=0;
- int prev_wd = -1;
+ struct inotify_event *e = NULL;
ssize_t ret;
- /*
- we must use FIONREAD as we cannot predict the length of the
- filenames, and thus can't know how much to allocate
- otherwise
- */
- if (ioctl(in->fd, FIONREAD, &bufsize) != 0 ||
- bufsize == 0) {
- DEBUG(0,("No data on inotify fd?!\n"));
- TALLOC_FREE(fde);
- return;
- }
-
- e0 = e = (struct inotify_event *)TALLOC_SIZE(in, bufsize + 1);
- if (e == NULL) return;
- ((uint8_t *)e)[bufsize] = '\0';
-
- ret = read_data(in->fd, e0, bufsize);
- if (ret != bufsize) {
+ ret = sys_read(in->fd, buf, sizeof(buf));
+ if (ret == -1) {
DEBUG(0, ("Failed to read all inotify data - %s\n",
strerror(errno)));
- talloc_free(e0);
/* the inotify fd will now be out of sync,
* can't keep reading data off it */
TALLOC_FREE(fde);
return;
}
+ bufsize = ret;
+
+ e = (struct inotify_event *)buf;
/* we can get more than one event in the buffer */
- while (e && (bufsize >= sizeof(*e))) {
- struct inotify_event *e2 = NULL;
- bufsize -= e->len + sizeof(*e);
- if (bufsize >= sizeof(*e)) {
- e2 = (struct inotify_event *)(e->len + sizeof(*e) +
(char *)e);
+ while (bufsize >= sizeof(struct inotify_event)) {
+ size_t e_len = sizeof(struct inotify_event) + e->len;
+
+ if ((e_len < sizeof(struct inotify_event)) ||
+ (e_len > bufsize))
+ {
+ DBG_ERR("Invalid data from inotify\n");
+ TALLOC_FREE(fde);
+ return;
}
- inotify_dispatch(in, e, prev_wd, prev_cookie, e2);
- prev_wd = e->wd;
- prev_cookie = e->cookie;
- e = e2;
- }
- talloc_free(e0);
+ inotify_dispatch(ev, in, e);
+
+ e = (struct inotify_event *)((char *)e + e_len);
+ bufsize -= e_len;
+ }
}
/*
@@ -421,12 +555,15 @@ int inotify_watch(TALLOC_CTX *mem_ctx,
return ENOMEM;
}
- w->in = in;
- w->callback = callback;
- w->private_data = private_data;
- w->mask = mask;
- w->filter = orig_filter;
- w->path = talloc_strdup(w, path);
+ *w = (struct inotify_watch_context) {
+ .in = in,
+ .callback = callback,
+ .private_data = private_data,
+ .mask = mask,
+ .filter = orig_filter,
+ .path = talloc_strdup(w, path),
+ };
+
if (w->path == NULL) {
*filter = orig_filter;
TALLOC_FREE(w);
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 988317b804c..3ffcfa36bb1 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -4540,7 +4540,7 @@ static NTSTATUS mkdir_internal(connection_struct *conn,
#ifdef OPENBSD
/*
* OpenBSD requires to have write permissions
- * on both source and destimation of renameat(),
+ * on both source and destination of renameat(),
* see https://bugzilla.samba.org/show_bug.cgi?id=15801
*
* For now just disable the new code by default.
--
Samba Shared Repository
