CVE-2022-32746 ldb: Release LDB 2.6.1

* CVE-2022-32746 Use-after-free occurring in database audit logging module (bug 15009)

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Andrew Bartlett <abartlet@samba.org>

CVE-2022-32746 ldb: Make use of functions for appending to an ldb_message

This aims to minimise usage of the error-prone pattern of searching for
a just-added message element in order to make modifications to it (and
potentially finding the wrong element).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 ldb: Add functions for appending to an ldb_message

Currently, there are many places where we use ldb_msg_add_empty() to add
an empty element to a message, and then call ldb_msg_add_value() or
similar to add values to that element. However, this performs an
unnecessary search of the message's elements to locate the new element.
Moreover, if an element with the same attribute name already exists
earlier in the message, the values will be added to that element,
instead of to the intended newly added element.

A similar pattern exists where we add values to a message, and then call
ldb_msg_find_element() to locate that message element and sets its flags
to (e.g.) LDB_FLAG_MOD_REPLACE. This also performs an unnecessary
search, and may locate the wrong message element for setting the flags.

To avoid these problems, add functions for appending a value to a
message, so that a particular value can be added to the end of a message
in a single operation.

For ADD requests, it is important that no two message elements share the
same attribute name, otherwise things will break. (Normally,
ldb_msg_normalize() is called before processing the request to help
ensure this.) Thus, we must be careful not to append an attribute to an
ADD message, unless we are sure (e.g. through ldb_msg_find_element())
that an existing element for that attribute is not present.

These functions will be used in the next commit.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 ldb: Ensure shallow copy modifications do not affect original message

Using the newly added ldb flag, we can now detect when a message has
been shallow-copied so that its elements share their values with the
original message elements. Then when adding values to the copied
message, we now make a copy of the shared values array first.

This should prevent a use-after-free that occurred in LDB modules when
new values were added to a shallow copy of a message by calling
talloc_realloc() on the original values array, invalidating the 'values'
pointer in the original message element. The original values pointer can
later be used in the database audit logging module which logs database
requests, and potentially cause a crash.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 ldb: Add flag to mark message element values as shared

When making a shallow copy of an ldb message, mark the message elements
of the copy as sharing their values with the message elements in the
original message.

This flag value will be heeded in the next commit.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 s4/registry: Use LDB_FLAG_MOD_TYPE() for flags equality check

Now unrelated flags will no longer affect the result.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 s4/dsdb/tombstone_reanimate: Use LDB_FLAG_MOD_TYPE() for flags equality check

Now unrelated flags will no longer affect the result.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 s4/dsdb/repl_meta_data: Use LDB_FLAG_MOD_TYPE() for flags equality check

Now unrelated flags will no longer affect the result.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 ldb:rdn_name: Use LDB_FLAG_MOD_TYPE() for flags equality check

Now unrelated flags will no longer affect the result.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 s4/dsdb/acl: Fix LDB flags comparison

LDB_FLAG_MOD_* values are not actually flags, and the previous
comparison was equivalent to

(el->flags & LDB_FLAG_MOD_MASK) == 0

which is only true if none of the LDB_FLAG_MOD_* values are set, so we
would not successfully return if the element was a DELETE. Correct the
expression to what it was intended to be.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 s4:torture: Fix LDB flags comparison

LDB_FLAG_MOD_* values are not actually flags, and the previous
comparison was equivalent to

(el->flags & LDB_FLAG_MOD_MASK) == 0

which is only true if none of the LDB_FLAG_MOD_* values are set. Correct
the expression to what it was probably intended to be.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 s4/dsdb/partition: Fix LDB flags comparison

LDB_FLAG_MOD_* values are not actually flags, and the previous
comparison was equivalent to

(req_msg->elements[el_idx].flags & LDB_FLAG_MOD_MASK) != 0

which is true whenever any of the LDB_FLAG_MOD_* values are set. Correct
the expression to what it was probably intended to be.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 s4:dsdb:tests: Add test for deleting a disallowed SPN

If an account has an SPN that requires Write Property to set, we should
still be able to delete it with just Validated Write.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

CVE-2022-32746 s4/dsdb/objectclass_attrs: Fix typo

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

s3:dbwrap_watch: call dbwrap_watched_trigger_wakeup() outside of the low level record lock

This gives a nice speed up, as it's unlikely for the waiters to hit
contention.

The following test with 256 commections all looping with open/close
on the same inode (share root) is improved drastically:

  smbtorture //127.0.0.1/m -Uroot%test smb2.create.bench-path-contention-shared \
     --option='torture:bench_path=' \
     --option="torture:timelimit=60" \
     --option="torture:nprocs=256"

From some like this:

   open[num/s=8800,avslat=0.021445,minlat=0.000095,maxlat=0.179786]
   close[num/s=8800,avslat=0.021658,minlat=0.000044,maxlat=0.179819]

to:

   open[num/s=10223,avslat=0.017922,minlat=0.000083,maxlat=0.106759]
   close[num/s=10223,avslat=0.017694,minlat=0.000040,maxlat=0.107345]

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Tue Jul 26 14:32:35 UTC 2022 on sn-devel-184

s3:dbwrap_watch: only notify the first waiter

In case of a highly contended record we will have a lot of watchers,
which will all race to get g_lock_lock() to finish.

If g_lock_unlock() wakes them all, e.g. 250 of them, we get a thundering
herd, were 249 will only find that one of them as able to get the lock
and re-add their watcher entry (not unlikely in a different order).

With this commit we only wake the first watcher and let it remove
itself once it no longer wants to monitor the record content
(at that time it will wake the new first watcher).

It means the woken watcher doesn't have to race with all others
and also means order of watchers is kept, which means that we
most likely get a fair latency distribution for all watchers.

The following test with 256 commections all looping with open/close
on the same inode (share root) is improved drastically:

  smbtorture //127.0.0.1/m -Uroot%test smb2.create.bench-path-contention-shared \
     --option='torture:bench_path=' \
     --option="torture:timelimit=60" \
     --option="torture:nprocs=256"

From some like this:

   open[num/s=80,avslat=2.793862,minlat=0.004097,maxlat=46.597053]
   close[num/s=80,avslat=2.387326,minlat=0.023875,maxlat=50.878165]

to:

   open[num/s=8800,avslat=0.021445,minlat=0.000095,maxlat=0.179786]
   close[num/s=8800,avslat=0.021658,minlat=0.000044,maxlat=0.179819]

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:smbXsrv_session: only change the dbwrap_watch instance when the record has changed

This will become important in the following commits when the
dbwrap_watch layer will only wake up one watcher at a time
and each woken watcher will wakeup the next one.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:smbXsrv_session: introduce smb2srv_session_close_previous_cleanup()

This makes sure we cleanup the locked record in all cases.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:smbXsrv_client: only change the dbwrap_watch instance when the record has changed

This will become important in the following commits when the
dbwrap_watch layer will only wake up one watcher at a time
and each woken watcher will wakeup the next one.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:g_lock: try to keep the watch instance during g_lock_watch_data()

Unless the unique_lock_epoch changes via g_lock_lock()/g_lock_unlock()
we try to keep our existing watch instance alive while waiting
for unique_data_epoch to change.

This will become important in the following commits when the
dbwrap_watch layer will only wake up one watcher at a time
and each woken watcher will wakeup the next one. Without this
commit we would trigger an endless loop as none of the watchers
will ever change unique_data_epoch.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:g_lock: remember an unique_lock_epoch similar to unique_data_epoch

It changes with every lock and unlock.

This will be needed in future in order to differentiate between
lock and data changed.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:g_lock: avoid a lot of unused overhead using the new dbwrap_watch features

The key points are:

1. We keep our position in the watcher queue until we got what
   we were waiting for. It means the order is now fair and stable.

2. We only wake up other during g_lock_unlock() and only if
   we detect that an pending exclusive lock is able to make progress.
   (Note: read lock holders are never waiters on their own)

This reduced the contention on locking.tdb records drastically,
as waiters are no longer woken 3 times (where the first 2 times were completely useless).

The following test with 256 commections all looping with open/close
on the same inode (share root) is improved drastically:

  smbtorture //127.0.0.1/m -Uroot%test smb2.create.bench-path-contention-shared \
     --option='torture:bench_path=' \
     --option="torture:timelimit=60" \
     --option="torture:nprocs=256"

From some like this:

   open[num/s=50,avslat=6.455775,minlat=0.000157,maxlat=55.683846]
   close[num/s=50,avslat=4.563605,minlat=0.000128,maxlat=53.585839]

to:

   open[num/s=80,avslat=2.793862,minlat=0.004097,maxlat=46.597053]
   close[num/s=80,avslat=2.387326,minlat=0.023875,maxlat=50.878165]

Note the real effect of this commit will releaved together
with a following commit that only wakes one waiter at a time.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:g_lock: always call g_lock_cleanup_shared() before getting stuck on lck.num_shared != 0

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:g_lock: avoid calling g_lock_store() from g_lock_cleanup_dead()

This matches the behavior of g_lock_cleanup_shared(), which also
only operates on the in memory struct g_lock.

We do a g_lock_store() later during g_lock_trylock() anyway
when we make any progress.

In the case we where a pending exclusive lock holder
we now force a g_lock_store() if g_lock_cleanup_dead()
removed the dead blocker.

This will be useful for the following changes...

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: allow callers of dbwrap_watched_watch_send/recv() to manage the watcher instances

The destructor triggered by dbwrap_watched_watch_recv() will
remove the watcher instance via a dedicated dbwrap_do_locked(),
just calling dbwrap_watched_watch_remove_instance() inside.

But the typical caller triggers a dbwrap_do_locked() again after
dbwrap_watched_watch_recv() returned. Which means we call
dbwrap_do_locked() twice.

We now allow dbwrap_watched_watch_recv() to return the existing
instance id (if it still exists) and removes the destructor.
That way the caller can pass the given instance id to
dbwrap_watched_watch_remove_instance() from within its own dbwrap_do_locked(),
when it decides to leave the queue, because it's happy with the new
state of the record. In order to get the best performance
dbwrap_watched_watch_remove_instance() should be called before any
dbwrap_record_storev() or dbwrap_record_delete(),
because that will only trigger a single low level storev/delete.

If the caller found out that the state of the record doesn't meet the
expectations and the callers wants to continue watching the
record (from its current position, most likely the first one),
dbwrap_watched_watch_remove_instance() can be skipped and the
instance id can be passed to dbwrap_watched_watch_send() again,
in order to resume waiting on the existing instance.
Currently the watcher instance were always removed (most likely from
the first position) and re-added (to the last position), which may
cause unfair latencies.

In order to improve the overhead of adding a new watcher instance
the caller can call dbwrap_watched_watch_add_instance() before
any dbwrap_record_storev() or dbwrap_record_delete(), which
will only result in a single low level storev/delete.
The returned instance id is then passed to dbwrap_watched_watch_send(),
within the same dbwrap_do_locked() run.

It also adds a way to avoid alerting any callers during
the current dbwrap_do_locked() run.

Layers above may only want to wake up watchers
during specific situations and while it's useless to wake
others in other situations.

This will soon be used to add more fairness to the g_lock code.

Note that this commit only prepares the api for the above to be useful,
the instance returned by dbwrap_watched_watch_recv() is most likely 0,
which means the watcher entry was already removed, but that will change
in the following commits.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: remove a watcher via db_watched_record_fini()

The new dbwrap_watched_watch_remove_instance() will just remove ourself
from the in memory array and let db_watched_record_fini() call
dbwrap_watched_record_storev() in order to write the modified version
into the low level backend record.

For now there's no change in behavior, but it allows us to change it
soon....

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: use dbwrap_watched_record_storev() to add a new watcher

It means we only have one code path storing the low level record
and that's dbwrap_watched_record_storev on the main record.

It avoids the nested dbwrap_do_locked() and only uses
dbwrap_parse_record() and talloc_memdup() when needed.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: let dbwrap_watched_delete() call dbwrap_watched_record_storev(num_dbufs=0)

dbwrap_watched_record_storev() will handle the high level storev and
delete, it will find out if we can remove the record as there's no value
and also no watchers to be stored.

This is no real change for now as dbwrap_watched_record_wakeup() will
always exits with wrec->watchers.count = 0, but that will change in the next
commits.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: filter out records with empty payload during traverse

We will soon have records with just a number of watchers, but without
payload. These records should not be visible during traverse.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: prepare dbwrap_watched_record_storev() to store watchers if requested

It will also delete the low level record in case there are no watchers
should be stored and no data buffers are given.

This is no real change for now as dbwrap_watched_record_wakeup() will
always exit with wrec->watchers.count = 0, but that will change in the next
commits.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: define/use DBWRAP_MAX_WATCHERS

dbwrap backends are unlikely to be able to store
UINT32_MAX*DBWRAP_WATCHER_BUF_LENGTH in a single record
and most likely also not with the whole database!

DBWRAP_MAX_WATCHERS = INT32_MAX/DBWRAP_WATCHER_BUF_LENGTH should be
enough and makes further changes easier as we don't need to care
about size_t overflows.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: remove unused dbwrap_watched_do_locked_state.status

This is never set...

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: let dbwrap_watched_watch_recv() use tevent_req_received()

At the end of the dbwrap_watched_watch_recv() all temporary state should
be destroyed. It also means dbwrap_watched_watch_state_destructor() was
triggered.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: don't use talloc_tos() for messaging_filtered_read_recv()

Async function always have their 'state' context for temporary memory.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: move db_record and db_watched_record to dbwrap_watched_do_locked()

This will help in the next commits.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: split out a dbwrap_watched_watch_add_instance() helper

This will be used in other places soon.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: remove dbwrap_watched_record_wakeup_fn() indirection

This reduces quite some complexity and will make further changes
(which will follow soon) easier.

Review with git show --patience

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: also the fetch_locked case only needs to wake waiters just once

This is no change in behavior, because:

- The first dbwrap_do_locked(dbwrap_watched_record_wakeup_fn), is
  called at the start of dbwrap_watched_record_{storev,delete}().
  That means the nested dbwrap_do_locked() will pass the
  exact value same (unchanged) value to dbwrap_watched_record_wakeup_fn.

- After the first change we have either removed the whole backend
  record in dbwrap_watched_record_delete or dbwrap_watched_record_storev()
  removed all watchers and store num_watchers = 0.

- With that any further updates will have no watchers in the backend
  record, so dbwrap_do_locked(dbwrap_watched_record_wakeup_fn) will
  never do anything useful. It only burns cpu time any may cause memory
  fragmentation.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: split out db_watched_record_fini() from db_watched_record_destructor()

That makes it easier to understand that db_watched_record_init() and
db_watched_record_fini() wrap any caller activity on the record,
either during do_locked or between fetch_locked and the related
destructor.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: split out a db_watched_record_init() helper function

The code to construct a struct db_watched_record is mostly common
between dbwrap_watched_fetch_locked() and dbwrap_watched_do_locked_fn().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: remove unused dbwrap_watched_do_locked_{storev,delete}()

dbwrap_watched_do_locked_{storev,delete}() was now exactly the
same as dbwrap_watched_{storev,delete}().

We only need to know if dbwrap_watched_record_wakeup() is called from
within dbwrap_watched_do_locked_fn().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: move the do_locked optimization to dbwrap_watched_record_wakeup()

Both dbwrap_watched_record_storev() and dbwrap_watched_record_delete()
call dbwrap_watched_record_wakeup() as their first action.

So the behavior stays the same, but dbwrap_watched_do_locked_storev()
and dbwrap_watched_do_locked_delete() are not trivial and we
have the wakeup logic isolated in dbwrap_watched_record_wakeup() only.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: add db_record_get_watched_record() helper

This allows safe casting off rec->private_data to get
struct db_watched_record. And that works fetch_locked and do_locked

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: use backend.{rec,initial_value} instead of subrec[_value]

This makes it much clearer to me what it actually is.

Keeping the initial_value with struct db_watched_record will also
simplify further changes.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: only pass struct db_watched_record to dbwrap_watched_record_*() functions

We get to the main 'struct db_record' via wrec->rec where needed.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: use dbwrap_record_get_key() to access the key

We should avoid doing shortcuts if not needed.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: move 'wrec' from dbwrap_watched_do_locked_state to dbwrap_watched_do_locked_fn

We can use a local variable in dbwrap_watched_do_locked_fn.
As 'wrec' should have the same lifetime as 'rec'.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: use struct db_watched_record as rec->private_data for do_locked too

There's no real reason to pass struct dbwrap_watched_do_locked_state
anymore. The only difference is that we can't use
talloc_get_type_abort().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: use dbwrap_record_get_db(rec) instead of state->db

We should try to avoid using dbwrap_watched_do_locked_state in low
level code.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: move wakeup_value to struct db_watched_record

For the do_locked case they have the same scope, but having
it on db_watched_record will simplify further changes.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: rename struct dbwrap_watched_record variables to 'wrec'

This makes it much easier to understand...

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: s/dbwrap_watched_subrec/dbwrap_watched_record

These functions operate on struct db_watched_record.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: s/db_watched_subrec/db_watched_record

struct db_watched_record is the private data of
the struct db_record produced by the struct db_context that
uses struct db_watched_ctx.

db_watched_subrec had nothing really todo with the
sub record we got back from db_watched_ctx->backend.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: use value_valid = false during dbwrap_watched_do_locked_fn()

This matches db_tdb_do_locked() and the fetch_locked based fallback in
dbwrap_do_locked().

Calling dbwrap_record_get_value() is not allowed from within
dbwrap_do_locked()!

Now that rec.value is only internal, use it to remember the initial
payload value. This will simplify further code changes as it
makes the fetch_locked case.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

s3:dbwrap_watch: let dbwrap_watched_watch_state_destructor() use DBG_WARNING()

When we (need) to ignore an error from dbwrap_do_locked() within
dbwrap_watched_watch_state_destructor(), we better print this
with log level 1 instead of 10.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: split out smbd_check_access_rights_fname and call it before SMB_VFS_FGET_NT_ACL

commit 8e3798dd22276bc1ac8e96004d0e5e974240a7b9 actually came with a
change in behavior..., as SMB_VFS_GET_NT_ACL_AT() (at the time) and
now SMB_VFS_FGET_NT_ACL() is always called even if it's not needed.
E.g. access by root.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Tue Jul 26 00:31:29 UTC 2022 on sn-devel-184

s3:profile: make use of tevent_cached_getpid() in performance critical code

This avoids wasting getpid() during profiling.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Mon Jul 25 18:32:18 UTC 2022 on sn-devel-184

lib/util: make use of tevent_cached_getpid() in performance critical code

This avoids wasting getpid() calls in a lot of places...

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

s3:lib/messages*: s/getpid/tevent_cached_getpid

Our messaging code is very performance critical and
we should note waste time in getpid() syscalls...

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

lib/messaging: s/getpid/tevent_cached_getpid

Our messaging code is very performance critical and
we should note waste time in getpid() syscalls...

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tevent: version 0.13.0

- add tevent_cached_getpid()

Note the changes to ABI/tevent-0.12.1.sigs only
revert the temporary changes made there...

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tevent: tevent_cached_getpid() tests

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tevent: add tevent_cached_getpid() helper

This avoids a getpid() syscall per tevent_loop_once() iteration.

We provide tevent_cached_getpid() also as helper for external consumers
in order to have the logic only once.

Note the change to ABI/tevent-0.12.1.sigs will be reverted
with the bump to 0.13.0.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

smbd: Userspace symlink eval in filename_convert_dirfsp()

This converts filename_convert_dirfsp to do symlink evaluation in user
space. It uses openat_pathref_dirfsp_nosymlink() to open the dirpath
and looks at the proper NT_STATUS_STOPPED_ON_SYMLINK response. Using
this avoids filename_convert() and thus unix_convert() completely for
the SMB2_CREATE case.

The tests

samba3.blackbox.smbclient_s3.NT1.plain.Recursive ls across MS-DFS links

now correctly stop the symlink lookup recursion with
NT_STATUS_OBJECT_PATH_NOT_FOUND. Previously we did not correcly pass up the
ELOOP coming back from the stat-call.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Mon Jul 25 12:56:08 UTC 2022 on sn-devel-184

smbd: Take care of @GMT in SMB1's reply_ntcreate_and_X()

Next we want to avoid filename_convert() to take care of this. The
SMB2 code has a proper TWRP token anyway, so let's push the
@GMT-handling to the SMB1 code that will be converted to
filename_convert_dirfsp().

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

vfs_error_inject: Ignore openat() from openat_pathref_dirfsp_nosymlink()

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Add openat_pathref_dirfsp_nosymlink()

This does a step-by-step path resolution for a directory by splitting
up the path into individual components and does a loop like that

for component in components:
    fd = openat(dirfd, component, O_NOFOLLOW);
    close(dirfd);
    dirfd = fd

and it will report any symlink it finds in a way that will be
indirectly consumable for the smb2 symlink error response.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Make get_real_filename_at public

We'll use this in files.c, which creates a bit of a cyclic
dependency. But files.c has all the lowlevel fsp handling, and we'll
add another routine there next which needs get_real_filename_at()

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Some more assert in fd_openat()

Before this patch we asserted that if we have a base_fsp then
smb_fname must have a stream name attached. Now we also assert that if
we don't have a base_fsp smb_fname is not a stream.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Simplify open_stream_pathref_fsp()

The main point of this function was to avoid fd_openat() and thus the
expensive non_widelink_open(). Now that fd_openat() has the direct
SMB_VFS_OPENAT() fast-path for streams, we can avoid duplicating the
logic in open_stream_pathref_fsp() again.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: open_stream_pathref_fsp() does not need a dirfsp

It opens relative to fsp->base_fsp

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Simplify openat_pathref_fullname()

Don't set O_RDONLY|O_NONBLOCK in two steps into a variable

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Open openat_pathref_fullname() for streams

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Hand full_fname from openat_pathref_nostream()

Rename it to openat_pathref_fullname(), it will be used for stream
open next

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: A stream open does not need O_NOFOLLOW

Would not have hurt either, but this makes the next patch easier to
verify properly

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Simplify openat_pathref_fsp()

Remove the implicit recursion

openat_pathref_fsp->openat_pathref_base_fsp->openat_pathref_fsp

by introducing openat_pathref_nostream() and use
open_stream_pathref_fsp() where possible. openat_pathref_nostream()
will change its name in further refactoring patches, but for
understanding this patch I think this name is good :-)

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Simplify non_widelink_open()

Now that non_widelink_open() does not see streams opens, we don't need
to take care of fsp->base_fsp anymore.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

VFS: NULL dirfsp for openat on stream opens

The main optimization is to avoid non_widelink_open() for streams
opens based on the fact that all streams opens are relative to
fsp->base_fsp, which is a pathref fsp already.

Neither streams_xattr nor streams_depot referenced dirfsp for the
streams case. Make this more obvious in the callers by passing NULL
and asserting this: non-streams opens and streams opens are just
different things, streams-opens can and do reference a base fsp and
don't need the non_widelink_open logic.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

vfs_shadow_copy2: Don't reference dirfsp for streams

A stream open is always relative to fsp->base_fsp. This already holds
the full path name in fsp->base_fsp->fsp_name, so we don't really need
the full_path_from_dirfsp_atname(). full_path_from_dirfsp_atname() is
not really bad, but the next patches will avoid having a dirfsp for
stream opens overall.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Make fsp_attach_smb_fname() talloc_move() the name to the fsp

For the current callers this does not make a difference, they have
already allocated *_smb_fname as a talloc child of fsp, but the next
patches will add one where it does.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Update fsp->fsp_flags.is_directory in vfs_stat_fsp()

The type of a fsp should never change, but if this call to
vfs_stat_fsp() is the very first one on this fsp, we must update this
flag.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

libcli: Modernize a few DEBUG statements

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

test3: Fix a debug message

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Factor out extract_snapshot_token() from canonicalize_snapshot_path()

We'll use this elsewhere soon.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Simplify canonicalize_snapshot_path()

All we need to do is to convert the @GMT-Token and move the
rest. Before this patch we did a lot of talloc to move the @GMT token
to the beginning of the path only to cut it off immediately
again. Merge that logic into a simple memmove()

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Don't create a fsp->base_fsp for a "::$DATA" stream

"::$DATA" is the main file, we don't need the overhead of base_fsp here.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

torture3: Fix an error check in torture_delete_fn()

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

registry3: Align an integer type

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

registry3: Align function types to what is returned

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

lib: Simplify canonicalize_absolute_path()

We don't need the separate "wrote_slash" boolean variable, we can just
look at what we wrote into p[-1]

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

lib: Remove a few #include "includes.h"

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Avoid a "? True : False"

Just came across this, looked weird...

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

smbd: Open up openat_internal_dir_from_pathref() for general dirs

We open "." fixed here, and fd_openat (or rather SMB_VFS_OPENAT) will
tell us if "dirfsp" does not point at a proper directory

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

lib: On FreeBSD util_paths.c does not find struct stat

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Sun Jul 24 00:25:49 UTC 2022 on sn-devel-184

lib: Fix the 32-bit build

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

torture: Fix the 32-bit build

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

lib: Fix the FreeBSD build

"time_t" only comes in via a proper include of <time.h>

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

winbind: Fix a "format string is not a string literal" warning

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

winbind: Fix the 32-bit build

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

gpo: samba-gpupdate use s3 param for registry conf

Cause samba-gpupdate to use an s3 param so that
it can load settings from registry configuration.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Fri Jul 22 20:40:51 UTC 2022 on sn-devel-184