The branch, master has been updated via 6b9564c1084 s3:ctdbd_conn: simplify get_public_ips() / find_in_public_ips() API via 0253ba159b9 s3:smbd: rename has_ctdb_public_ip to has_cluster_movable_ip via 55dad704116 smb2_ioctl_network_fs: fix minor leak in error path via b78ff571765 interface: fix if_index is not parsed correctly from 74fbe0b987a vfs_shadow_copy2: Avoid closing snapsdir twice
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 6b9564c1084d8dc7319857fac984808571ef0eb9 Author: David Disseldorp <dd...@samba.org> Date: Mon Sep 7 00:17:11 2020 +0200 s3:ctdbd_conn: simplify get_public_ips() / find_in_public_ips() API These calls are used to check whether an IP address is static to the host, or whether it could be migrated by ctdb. Combine the calls into a simple ctdbd_public_ip_foreach(cb) function, which avoids the need to expose struct ctdb_public_ip_list_old. Signed-off-by: David Disseldorp <dd...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> Autobuild-User(master): Stefan Metzmacher <me...@samba.org> Autobuild-Date(master): Wed Oct 14 12:29:56 UTC 2020 on sn-devel-184 commit 0253ba159b9a5017a11c63e362d70c9fea637ca2 Author: David Disseldorp <dd...@samba.org> Date: Sun Sep 6 22:59:20 2020 +0200 s3:smbd: rename has_ctdb_public_ip to has_cluster_movable_ip This provides a little more detail to what's actually being tracked with this boolean. Signed-off-by: David Disseldorp <dd...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit 55dad704116b31249bd8004241ea4e1d0b481512 Author: David Disseldorp <dd...@samba.org> Date: Sun Sep 6 23:59:04 2020 +0200 smb2_ioctl_network_fs: fix minor leak in error path The struct fsctl_net_iface_info array needs to be cleaned up. Signed-off-by: David Disseldorp <dd...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> commit b78ff5717654064c8a4facc54a8e9833e5843c21 Author: Jones Syue <joness...@qnap.com> Date: Mon Sep 28 09:10:03 2020 +0800 interface: fix if_index is not parsed correctly Replace probed_ifaces[i] with ifs. In SDC 2020 SMB3 Virtual IO Lab, run Windows Protocol Test Suite to test FileServer multichannel test cases. Samba server has 2 virtual interfaces for VPN connection: > name=tun2001, ip/mask=192.168.144.9/22 > name=tun2002, ip/mask=192.168.144.10/22 test suite client can ping these 2 ip addresses and browse shares. Then client try to use IOCTL FSCTL_QUERY_NETWORK_INTERFACE_INFO to get the virtual ip addresses of samba server, but samba server responded it without the virtual ip addresses. My VPN setup is point-to-point and the virtual interfaces 'tun2001' & 'tun2002' are without flag IFF_BROADCAST. So edit smb.conf and add "interfaces = ${virtual_ip}/${mask_length};if_index=${id}", like this: > interfaces = eth4 eth8 eth11 eth10 qvs0 "192.168.144.9/22;if_index=50" "192.168.144.10/22;if_index=51" then samba server IOCTL response could return the virtual ip addresses, but found a issue: the interface index of virtual ip addresses is always 4294967295 (0xFFFFFFFF, -1). Quote Metze: https://gitlab.com/samba-team/devel/samba/-/commit/6cadb55d975a6348a417caed8b3258f5be2acba4#note_419181789 This looks good, I think that also explains the possible memory corruption/crash I mentioned in the bug report. As 'i' is most likely the same as 'total_probed' and probed_ifaces[i] is not valid, so we overwrite unrelated memory. Later I see 'realloc(): invalid pointer' and this backtrace: BACKTRACE: #0 log_stack_trace + 0x29 [ip=0x7f2f1b6fffa9] [sp=0x7ffcd0ab53e0] #1 smb_panic + 0x11 [ip=0x7f2f1b700301] [sp=0x7ffcd0ab5d10] #2 sig_fault + 0x54 [ip=0x7f2f1b7004f4] [sp=0x7ffcd0ab5e20] #3 funlockfile + 0x50 [ip=0x7f2f17ce6dd0] [sp=0x7ffcd0ab5ec0] #4 gsignal + 0x10f [ip=0x7f2f1794970f] [sp=0x7ffcd0ab6b90] #5 abort + 0x127 [ip=0x7f2f17933b25] [sp=0x7ffcd0ab6cb0] #6 __libc_message + 0x297 [ip=0x7f2f1798c897] [sp=0x7ffcd0ab6de0] #7 malloc_printerr + 0x1c [ip=0x7f2f17992fdc] [sp=0x7ffcd0ab6ef0] #8 realloc + 0x23a [ip=0x7f2f17997f6a] [sp=0x7ffcd0ab6f00] #9 _talloc_realloc + 0xee [ip=0x7f2f1a365d2e] [sp=0x7ffcd0ab6f50] #10 messaging_filtered_read_send + 0x18c [ip=0x7f2f1a10f54c] [sp=0x7ffcd0ab6fb0] #11 messaging_read_send + 0x55 [ip=0x7f2f1a10f705] [sp=0x7ffcd0ab7000] #12 smb2srv_session_table_init + 0x83 [ip=0x7f2f1b3a6cd3] [sp=0x7ffcd0ab7040] #13 smbXsrv_connection_init_tables + 0x2d [ip=0x7f2f1b373f4d] [sp=0x7ffcd0ab7060] #14 smbd_smb2_request_process_negprot + 0x827 [ip=0x7f2f1b38cb47] [sp=0x7ffcd0ab7080] #15 smbd_smb2_request_dispatch + 0x19db [ip=0x7f2f1b38921b] [sp=0x7ffcd0ab71d0] #16 smbd_smb2_process_negprot + 0x298 [ip=0x7f2f1b38bb38] [sp=0x7ffcd0ab7260] #17 process_smb + 0x2ca [ip=0x7f2f1b37537a] [sp=0x7ffcd0ab72b0] #18 smbd_server_connection_read_handler + 0xd0 [ip=0x7f2f1b376420] [sp=0x7ffcd0ab7350] BUG: https://bugzilla.samba.org/show_bug.cgi?id=14514 Signed-off-by: Jones Syue <joness...@qnap.com> Reviewed-by: Ralph Boehme <s...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> ----------------------------------------------------------------------- Summary of changes: source3/include/ctdbd_conn.h | 21 ++++--- source3/lib/ctdb_dummy.c | 17 ++---- source3/lib/ctdbd_conn.c | 43 ++++++++++---- source3/lib/interface.c | 2 +- source3/smbd/globals.h | 2 +- source3/smbd/process.c | 48 ++++++++++----- source3/smbd/smb2_ioctl_network_fs.c | 112 ++++++++++++++++++++++++++++------- source3/smbd/smb2_server.c | 4 +- 8 files changed, 179 insertions(+), 70 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/include/ctdbd_conn.h b/source3/include/ctdbd_conn.h index b77dd06fd09..74db96e89e7 100644 --- a/source3/include/ctdbd_conn.h +++ b/source3/include/ctdbd_conn.h @@ -85,13 +85,20 @@ int ctdbd_register_ips(struct ctdbd_connection *conn, void *private_data), void *private_data); -struct ctdb_public_ip_list_old; -int ctdbd_control_get_public_ips(struct ctdbd_connection *conn, - uint32_t flags, - TALLOC_CTX *mem_ctx, - struct ctdb_public_ip_list_old **_ips); -bool ctdbd_find_in_public_ips(const struct ctdb_public_ip_list_old *ips, - const struct sockaddr_storage *ip); +/* + * call @cb for each public IP. If @cb returns non-zero, then break the loop + * and propagate the return value upwards. + * @returns: 0 on success, where all @cb invocations also returned zero + * ENOMEM on memory allocation failure + * EIO on ctdbd connection failure + * @cb() return value if non-zero + */ +int ctdbd_public_ip_foreach(struct ctdbd_connection *conn, + int (*cb)(uint32_t total_ip_count, + const struct sockaddr_storage *ip, + bool is_movable_ip, + void *private_data), + void *private_data); int ctdbd_control_local(struct ctdbd_connection *conn, uint32_t opcode, uint64_t srvid, uint32_t flags, TDB_DATA data, diff --git a/source3/lib/ctdb_dummy.c b/source3/lib/ctdb_dummy.c index 062fa999b06..294d178966b 100644 --- a/source3/lib/ctdb_dummy.c +++ b/source3/lib/ctdb_dummy.c @@ -62,21 +62,16 @@ int ctdbd_register_ips(struct ctdbd_connection *conn, return ENOSYS; } -int ctdbd_control_get_public_ips(struct ctdbd_connection *conn, - uint32_t flags, - TALLOC_CTX *mem_ctx, - struct ctdb_public_ip_list_old **_ips) +int ctdbd_public_ip_foreach(struct ctdbd_connection *conn, + int (*cb)(uint32_t total_ip_count, + const struct sockaddr_storage *ip, + bool is_movable_ip, + void *private_data), + void *private_data) { - *_ips = NULL; return ENOSYS; } -bool ctdbd_find_in_public_ips(const struct ctdb_public_ip_list_old *ips, - const struct sockaddr_storage *ip) -{ - return false; -} - bool ctdbd_process_exists(struct ctdbd_connection *conn, uint32_t vnn, pid_t pid, uint64_t unique_id) { diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c index a4a9f4e0cae..8fe94226590 100644 --- a/source3/lib/ctdbd_conn.c +++ b/source3/lib/ctdbd_conn.c @@ -1179,10 +1179,10 @@ int ctdbd_register_ips(struct ctdbd_connection *conn, return 0; } -int ctdbd_control_get_public_ips(struct ctdbd_connection *conn, - uint32_t flags, - TALLOC_CTX *mem_ctx, - struct ctdb_public_ip_list_old **_ips) +static int ctdbd_control_get_public_ips(struct ctdbd_connection *conn, + uint32_t flags, + TALLOC_CTX *mem_ctx, + struct ctdb_public_ip_list_old **_ips) { struct ctdb_public_ip_list_old *ips = NULL; TDB_DATA outdata; @@ -1225,27 +1225,44 @@ int ctdbd_control_get_public_ips(struct ctdbd_connection *conn, return 0; } -bool ctdbd_find_in_public_ips(const struct ctdb_public_ip_list_old *ips, - const struct sockaddr_storage *ip) +int ctdbd_public_ip_foreach(struct ctdbd_connection *conn, + int (*cb)(uint32_t total_ip_count, + const struct sockaddr_storage *ip, + bool is_movable_ip, + void *private_data), + void *private_data) { uint32_t i; + struct ctdb_public_ip_list_old *ips = NULL; + int ret = ENOMEM; + TALLOC_CTX *frame = talloc_stackframe(); + + ret = ctdbd_control_get_public_ips(conn, 0, frame, &ips); + if (ret < 0) { + ret = EIO; + goto out_free; + } for (i=0; i < ips->num; i++) { struct samba_sockaddr tmp = { .u = { - .ss = *ip, + .sa = ips->ips[i].addr.sa, }, }; - bool match; - match = sockaddr_equal(&ips->ips[i].addr.sa, - &tmp.u.sa); - if (match) { - return true; + ret = cb(ips->num, + &tmp.u.ss, + true, /* all ctdb public ips are movable */ + private_data); + if (ret != 0) { + goto out_free; } } - return false; + ret = 0; +out_free: + TALLOC_FREE(frame); + return ret; } /* diff --git a/source3/lib/interface.c b/source3/lib/interface.c index 5a86524e696..440d74cab99 100644 --- a/source3/lib/interface.c +++ b/source3/lib/interface.c @@ -618,7 +618,7 @@ static void interpret_interface(char *token) ifs.netmask = ss_mask; ifs.bcast = ss_bcast; if (if_index_set) { - probed_ifaces[i].if_index = if_index; + ifs.if_index = if_index; } if (speed_set) { ifs.linkspeed = speed; diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index fcf33a699c6..4685b6971d3 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -361,7 +361,7 @@ struct smbXsrv_connection { const struct tsocket_address *local_address; const struct tsocket_address *remote_address; const char *remote_hostname; - bool has_ctdb_public_ip; + bool has_cluster_movable_ip; enum protocol_types protocol; diff --git a/source3/smbd/process.c b/source3/smbd/process.c index 2b2a2e09d05..4cf53720067 100644 --- a/source3/smbd/process.c +++ b/source3/smbd/process.c @@ -2776,6 +2776,30 @@ static int release_ip(struct tevent_context *ev, return 0; } +static int match_cluster_movable_ip(uint32_t total_ip_count, + const struct sockaddr_storage *ip, + bool is_movable_ip, + void *private_data) +{ + const struct sockaddr_storage *srv = private_data; + struct samba_sockaddr pub_ip = { + .u = { + .ss = *ip, + }, + }; + struct samba_sockaddr srv_ip = { + .u = { + .ss = *srv, + }, + }; + + if (is_movable_ip && sockaddr_equal(&pub_ip.u.sa, &srv_ip.u.sa)) { + return EREMOTEIO; + } + + return 0; +} + static NTSTATUS smbd_register_ips(struct smbXsrv_connection *xconn, struct sockaddr_storage *srv, struct sockaddr_storage *clnt) @@ -2803,21 +2827,17 @@ static NTSTATUS smbd_register_ips(struct smbXsrv_connection *xconn, } if (xconn->client->server_multi_channel_enabled) { - struct ctdb_public_ip_list_old *ips = NULL; - - ret = ctdbd_control_get_public_ips(cconn, - 0, /* flags */ - state, - &ips); - if (ret != 0) { - return NT_STATUS_INTERNAL_ERROR; - } - - xconn->has_ctdb_public_ip = ctdbd_find_in_public_ips(ips, srv); - TALLOC_FREE(ips); - if (xconn->has_ctdb_public_ip) { - DBG_DEBUG("CTDB public ip on %s\n", + ret = ctdbd_public_ip_foreach(cconn, + match_cluster_movable_ip, + srv); + if (ret == EREMOTEIO) { + xconn->has_cluster_movable_ip = true; + DBG_DEBUG("cluster movable IP on %s\n", smbXsrv_connection_dbg(xconn)); + } else if (ret != 0) { + DBG_ERR("failed to iterate cluster IPs: %s\n", + strerror(ret)); + return NT_STATUS_INTERNAL_ERROR; } } diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c index e1881ae4485..ceb57cefb06 100644 --- a/source3/smbd/smb2_ioctl_network_fs.c +++ b/source3/smbd/smb2_ioctl_network_fs.c @@ -291,6 +291,70 @@ static NTSTATUS fsctl_srv_copychunk_recv(struct tevent_req *req, return status; } +struct cluster_movable_ips { + uint32_t array_len; + uint32_t array_index; + struct sockaddr_storage *ips; +}; + +static int stash_cluster_movable_ips(uint32_t total_ip_count, + const struct sockaddr_storage *ip, + bool is_movable_ip, + void *private_data) +{ + struct cluster_movable_ips *cluster_movable_ips + = talloc_get_type_abort(private_data, + struct cluster_movable_ips); + + if (!is_movable_ip) { + return 0; + } + + if (cluster_movable_ips->array_len == 0) { + SMB_ASSERT(total_ip_count < INT_MAX); + cluster_movable_ips->ips + = talloc_zero_array(cluster_movable_ips, + struct sockaddr_storage, + total_ip_count); + if (cluster_movable_ips->ips == NULL) { + return ENOMEM; + } + cluster_movable_ips->array_len = total_ip_count; + } + + SMB_ASSERT(cluster_movable_ips->array_index + < cluster_movable_ips->array_len); + + cluster_movable_ips->ips[cluster_movable_ips->array_index] = *ip; + cluster_movable_ips->array_index++; + + return 0; +} + +static bool find_in_cluster_movable_ips( + struct cluster_movable_ips *cluster_movable_ips, + const struct sockaddr_storage *ifss) +{ + struct samba_sockaddr srv_ip = { + .u = { + .ss = *ifss, + }, + }; + int i; + + for (i = 0; i < cluster_movable_ips->array_index; i++) { + struct samba_sockaddr pub_ip = { + .u = { + .ss = cluster_movable_ips->ips[i], + }, + }; + if (sockaddr_equal(&pub_ip.u.sa, &srv_ip.u.sa)) { + return true; + } + } + return false; +} + static NTSTATUS fsctl_network_iface_info(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct smbXsrv_connection *xconn, @@ -304,7 +368,8 @@ static NTSTATUS fsctl_network_iface_info(TALLOC_CTX *mem_ctx, size_t i; size_t num_ifaces = iface_count(); enum ndr_err_code ndr_err; - struct ctdb_public_ip_list_old *ips = NULL; + struct cluster_movable_ips *cluster_movable_ips = NULL; + int ret; if (in_input->length != 0) { return NT_STATUS_INVALID_PARAMETER; @@ -312,18 +377,6 @@ static NTSTATUS fsctl_network_iface_info(TALLOC_CTX *mem_ctx, *out_output = data_blob_null; - if (lp_clustering()) { - int ret; - - ret = ctdbd_control_get_public_ips(messaging_ctdb_connection(), - 0, /* flags */ - mem_ctx, - &ips); - if (ret != 0) { - return NT_STATUS_INTERNAL_ERROR; - } - } - array = talloc_zero_array(mem_ctx, struct fsctl_net_iface_info, num_ifaces); @@ -331,6 +384,22 @@ static NTSTATUS fsctl_network_iface_info(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } + if (lp_clustering()) { + cluster_movable_ips = talloc_zero(array, + struct cluster_movable_ips); + if (cluster_movable_ips == NULL) { + TALLOC_FREE(array); + return NT_STATUS_NO_MEMORY; + } + ret = ctdbd_public_ip_foreach(messaging_ctdb_connection(), + stash_cluster_movable_ips, + cluster_movable_ips); + if (ret != 0) { + TALLOC_FREE(array); + return NT_STATUS_INTERNAL_ERROR; + } + } + for (i=0; i < num_ifaces; i++) { struct fsctl_net_iface_info *cur = &array[i]; const struct interface *iface = get_interface(i); @@ -340,13 +409,14 @@ static NTSTATUS fsctl_network_iface_info(TALLOC_CTX *mem_ctx, struct tsocket_address *a = NULL; char *addr; bool ok; - int ret; ret = tsocket_address_bsd_from_sockaddr(array, ifsa, sizeof(struct sockaddr_storage), &a); if (ret != 0) { - return map_nt_error_from_unix_common(errno); + NTSTATUS status = map_nt_error_from_unix_common(errno); + TALLOC_FREE(array); + return status; } ok = tsocket_address_is_inet(a, "ip"); @@ -360,13 +430,13 @@ static NTSTATUS fsctl_network_iface_info(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - if (ips != NULL) { - bool is_public_ip; - - is_public_ip = ctdbd_find_in_public_ips(ips, ifss); - if (is_public_ip) { + if (cluster_movable_ips != NULL) { + bool is_movable_ip = find_in_cluster_movable_ips( + cluster_movable_ips, + ifss); + if (is_movable_ip) { DBG_DEBUG("Interface [%s] - " - "has public ip - " + "has movable public ip - " "skipping address [%s].\n", iface->name, addr); continue; diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index cf9de185c1f..2750c93b1e8 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -1635,9 +1635,9 @@ void smbd_server_connection_terminate_ex(struct smbXsrv_connection *xconn, smbXsrv_connection_dbg(xconn), num_ok, reason, location); - if (xconn->has_ctdb_public_ip) { + if (xconn->has_cluster_movable_ip) { /* - * If the connection has a ctdb public address + * If the connection has a movable cluster public address * we disconnect all client connections, * as the public address might be moved to * a different node. -- Samba Shared Repository