Andrew Bartlett [Wed, 15 May 2019 04:07:16 +0000 (16:07 +1200)]
torture: Address flapping samba4.rpc.altercontext test
NT_STATUS_CONNECTION_DISCONNECTED and NT_STATUS_CONNECTION_RESET are
equivilent for the purposes of this test, both come from the server
shutting down the connection, the difference comes from two different
unix error numbers that can come from this.
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Mon May 20 05:17:42 UTC 2019 on sn-devel-184
Andrew Bartlett [Thu, 16 May 2019 04:15:39 +0000 (16:15 +1200)]
dsdb/partition: Remove teardown of data->metadata on partition_metadata_set_sequence_number() failure
This changes variables that are not the responsiblity of this function, the unlock
implied by partition_del_trans() needs to be done carefully in the right spot.
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
Andrew Bartlett [Thu, 16 May 2019 02:35:00 +0000 (14:35 +1200)]
dsdb/partition: Ensure metadata.tdb is opened early in partition_reload_if_required()
This allows metadata.tdb to be locked in the correct place in
in the lock order, as partition_reload_if_required() implicitly
calls partition_lock_read().
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
Andrew Bartlett [Mon, 13 May 2019 03:32:23 +0000 (15:32 +1200)]
dsdb: Add random values to names in tests for large LDAP responses
This test is run agianst multiple DCs in the same domain, so there can
be a race with replication. Therefore avoid using the same name twice
by adding a random suffix.
This is an improvement to a demonstrator for this bug in TDB:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13952
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
Rikard Falkeborn [Thu, 16 May 2019 19:43:46 +0000 (21:43 +0200)]
s3: torture: Fix return values
Torture tests should return true on success and false on failure.
Returning -1 is the same as returning true and returning 0 is the same
as returning false. Change the return values to true and false to fix
the return values.
Detected by the help of cppcheck.
Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com> Reviewed-by: Ralph Böhme <slow@samba.org> Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Sun May 19 18:48:01 UTC 2019 on sn-devel-184
/home/rclinux/rc-buildroot-test/scripts/instance-1/output/host/mips64el-buildroot-linux-uclibc/sysroot/usr/include/stdint.h:122:27:
error: conflicting types for 'uintptr_t'
typedef unsigned long int uintptr_t;
^
In file included from ../lib/ldb/tests/ldb_msg.c:17:0:
../third_party/cmocka/cmocka.h:126:28: note: previous declaration of 'uintptr_t' was here
typedef unsigned int uintptr_t;
The define __WORDSIZE is missing when cmocka.h decides how to
define uintptr_t, this patch includes stdint.h when needed.
Patch sent upstream:
https://lists.samba.org/archive/samba-technical/2018-January/125306.html
[updated for samba-4.8.4, v2 sent upstream
https://lists.samba.org/archive/samba-technical/2018-August/129732.html,
updated for samba-4.9.1]
Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
[Retrieved from:
https://git.buildroot.net/buildroot/tree/package/samba4/0003-Fix-uClibc-build-on-64bit-platforms-by-including-std.patch] Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> Reviewed-by: Ralph Böhme <slow@samba.org> Reviewed-by: Jeremy Allison <jra@samba.org>
Volker Lendecke [Fri, 17 May 2019 08:44:23 +0000 (10:44 +0200)]
smbd: Send "share_file_id" with the rename msg
file_id plus share_file_id remotely specify the fsp. This avoids the
explicit loop in the receiver.
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): Sat May 18 20:18:55 UTC 2019 on sn-devel-184
Signed-off-by: Andreas Schneider <asn@samba.org> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Fri May 17 20:44:36 UTC 2019 on sn-devel-184
Andrew Bartlett [Fri, 17 May 2019 02:16:43 +0000 (14:16 +1200)]
tdb: Release tdb 1.4.1
* Do not propogate any errors from tdb_repack() to to tdb_transaction_commit()
The repack may fail due to lock ordering or memory limits, but the original data has
been written so the call must succeed. (bug 13952)
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Fri May 17 08:21:52 UTC 2019 on sn-devel-184
Andrew Bartlett [Mon, 13 May 2019 03:32:23 +0000 (15:32 +1200)]
dsdb: Add tests for large LDAP responses
This behaviour is Samba-specific, we have not traditionally cut of responses at 1000
or so as Windows does, and we need to change that behaviour carefully.
This triggers this bug in TDB:
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13952
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Jeremy Allison <jra@samba.org>
Andrew Bartlett [Thu, 16 May 2019 04:14:13 +0000 (16:14 +1200)]
tdb: Do not return errors from tdb_repack() in the tail of tdb_transaction_commit()
The call to tdb_repack() inside tdb_transaction_commit()
is an optimization, not part of the transaction itself,
so failing due to lock or other errors isn't a fatal error
that should cause the caller to think the transaction was
a failure by returning -1.
The tdb transaction itself has finished and been committed
onto stable storage via fsync and all locks released at the
point tdb_repack() is called.
tdb_repack() is only called here as it's a convenient point
to attempt to reduce tdb fragmentation without having to add
a timer call to repack in all users of tdb.
This causes lock ordering issues in Samba, showing up as:
Volker Lendecke [Thu, 16 May 2019 13:38:26 +0000 (15:38 +0200)]
smbd: Do oplock break messages in ndr
The previous scheme was overloaded, a idl definition is easier to
print, and it clarifies what data is actually needed
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): Thu May 16 23:48:18 UTC 2019 on sn-devel-184
lib:util: Fix tfork return value if sigprocmask fails
Returning a non-zero value from a function with bool as return value is
the same as returning true. Change the return value to false if
sigprocmask or pthread_sigmask fails to indicate failure.
Detected with the help of cppcheck.
Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com> Reviewed-by: Ralph Boehme <slow@samba.org> Reviewed-by: Andreas Schneider <asn@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Thu May 16 19:08:29 UTC 2019 on sn-devel-184
s3:loadparm: Ensure to truncate FS Volume Label at multibyte boundary
For FS_VOLUME_INFO/FS_INFO operation, a maximum of 32 characters are
sent back. However, since Samba chops off any share name with >32
bytes at 32, it is possible that a multi-byte share name can get chopped
off between a full character. This causes the string decoding for unicode
failure which sends back NT_STATUS_ILLEGAL_CHARACTER (EILSEQ) to the client
applications.
On Windows, Notepad doesn't like it, and refuses to open a file in this
case and fails with the following error:
Invalid character. For multibyte character sets, only the leading byte is
included without the trailing byte. For Unicode character sets, include
the characters 0xFFFF and 0xFFFE.
Proposed fix:
- Find the last starting point of a multibyte codepoint if the character
at 32nd byte is a subsequent byte of a MB codepoint.
Noel Power [Thu, 2 May 2019 18:45:14 +0000 (19:45 +0100)]
s4: squash 'cast between incompatible function types' warning
To avoid warning above produced by using
-Wcast-function-type we;
+ ensure PyCFunctions of type METH_NOARGS defined dummy arg
+ ensure PyCFunctions of type METH_KEYWORDS use PY_DISCARD_FUNC_SIG
macro
+ ensure PyCFunctions of type METH_KEYWORDS really actually use the
problematic kargs param, if not remove it
Signed-off-by: Noel Power <noel.power@suse.com> Reviewed-by: Andreas Schneider <asn@samba.org>
Noel Power [Thu, 2 May 2019 18:40:23 +0000 (19:40 +0100)]
s3: squash 'cast between incompatible function types' warning
Some functions (e.g. py_smb_savefile) have an extra unecessary
*kwargs param in their signatures, these definitions are
causing 'cast between incompatible function types' warnings when
compiled with -Wcast-function-type. Some other functions have
the *kwargs which causes "cast between incompatible function types'
warnings which need to be squashed with use of the
PY_DISCARD_FUNC_SIG macro.
Signed-off-by: Noel Power <noel.power@suse.com> Reviewed-by: Andreas Schneider <asn@samba.org>
Noel Power [Thu, 2 May 2019 18:35:56 +0000 (19:35 +0100)]
s4/librpc: squash 'cast between incompatible function types' warning
Where possible make PyCFunction definition signature match. Sometimes
this is not possible (e.g. when the c-function is associated with a
python method definition with 'METH_VARARGS|METH_KEYWORDS' in this
case we use the PY_DISCARD_FUNC_SIG macro.
Signed-off-by: Noel Power <noel.power@suse.com> Reviewed-by: Andreas Schneider <asn@samba.org>
Noel Power [Thu, 2 May 2019 11:32:36 +0000 (12:32 +0100)]
python: Create macro to hide ugly function signature cast
-Wcast-function-type triggers various warnings related to bad api
decisions in python source. To avoid these warnings we hide them
by casting to void first. Macro here is to hide the uglyness and
provide an easy place to either/or
+ fix better in a single place in the future
+ identify the places involved should python api change in a way
this can be handled better
Signed-off-by: Noel Power <noel.power@suse.com> Reviewed-by: Andreas Schneider <asn@samba.org>
Volker Lendecke [Wed, 15 May 2019 16:23:27 +0000 (18:23 +0200)]
smbd: Slightly simplify delay_for_oplock()
We don't have to look at the leases.tdb record if it's our own lease.
Signed-off-by: Volker Lendecke <vl@samba.org> Reviewed-by: Andreas Schneider <asn@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Thu May 16 07:59:52 UTC 2019 on sn-devel-184
Signed-off-by: Jeremy Allison <jra@samba.org> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Wed May 15 23:08:58 UTC 2019 on sn-devel-184
Gary Lockyer [Tue, 14 May 2019 03:53:22 +0000 (15:53 +1200)]
s4 dsdb/repl_meta_data: fix use after free in dsdb_audit_add_ldb_value
Fix use after free detected by AddressSanitizer
AddressSanitizer: heap-use-after-free on address 0x61400026a4a0
at pc 0x7fd555c52f12 bp 0x7ffed7231180 sp 0x7ffed7231170
READ of size 1 at 0x61400026a4a0 thread T0
#0 0x7fd555c52f11 in ldb_should_b64_encode
../../lib/ldb/common/ldb_ldif.c:197
#1 0x7fd539dc9417 in dsdb_audit_add_ldb_value
../../source4/dsdb/samdb/ldb_modules/audit_util.c:491
#2 0x7fd539dc9417 in dsdb_audit_attributes_json
../../source4/dsdb/samdb/ldb_modules/audit_util.c:651
#3 0x7fd539dc6a7e in operation_json
../../source4/dsdb/samdb/ldb_modules/audit_log.c:305
The problem is that at the successful end of these functions
el->values is overwritten with new_values. However get_parsed_dns()
points p->v at the supplied el and it effectively gets used
as a working area by replmd_build_la_val(). So we must duplicate it
because our caller only called ldb_msg_copy_shallow().
The reason this matters is that the audit_log module is
above repl_meta_data in the stack, and tries to log the
ldb_message it saw after the reply (to include the error code).
If that ldb_message is changed it is not only misleading,
it can point to memory that has since gone away.
In this case the memory for the full extended DN in the
member attribute ended up on 'ac', a context lost by
the time repl_meta_data has finished processing.
Signed-off-by: Gary Lockyer <gary@catalyst.net.nz> Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Wed May 15 05:35:47 UTC 2019 on sn-devel-184
Andrew Bartlett [Wed, 15 May 2019 02:47:53 +0000 (14:47 +1200)]
s4 dsdb/repl_meta_data: allocate new extended DNs during ADD on a better context
Lower down in this function new_values is assigned over el->values and is
filled in with the values of all the parsed DNs. Therefore it is the natural
talloc parent.
This will allow el->values to be allocated on tmp_ctx in the next commit for
a working area during the function call.
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>