Skip to content

Commit e386501

Browse files
Roytakmichalvasko
authored andcommitted
session server BUGFIX call home deadlock
Whenever a CH client is listening before a CH endpoint is created by the server, a deadlock happens where the server's CH thread tries to grab the config READ lock for a second time in ignored mod check (grabs it for the 1st time in ch_client_thread). The config applying thread then gets hung up in apply_diff on WRITE lock acquisition. This commit removes the 2nd READ lock acquisition of the CH thread, removing the deadlock scenario. Fixes CESNET/netopeer2#1787
1 parent 53393d3 commit e386501

3 files changed

Lines changed: 70 additions & 15 deletions

File tree

src/session.c

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,8 +1116,16 @@ add_cpblt(const char *capab, char ***cpblts, int *size, int *count)
11161116
++(*count);
11171117
}
11181118

1119-
API char **
1120-
nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version)
1119+
/**
1120+
* @brief Get the server capabilities.
1121+
*
1122+
* @param[in] ctx libyang context.
1123+
* @param[in] version YANG version of the schemas to be included in result.
1124+
* @param[in] config_locked Whether the configuration lock is already held or should be acquired.
1125+
* @return Array of capabilities, NULL on error.
1126+
*/
1127+
static char **
1128+
_nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version, int config_locked)
11211129
{
11221130
char **cpblts;
11231131
const struct lys_module *mod;
@@ -1232,7 +1240,7 @@ nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version)
12321240
/* models */
12331241
u = 0;
12341242
while ((mod = ly_ctx_get_module_iter(ctx, &u))) {
1235-
if (nc_server_is_mod_ignored(mod->name)) {
1243+
if (nc_server_is_mod_ignored(mod->name, config_locked)) {
12361244
/* ignored, not part of the cababilities */
12371245
continue;
12381246
}
@@ -1337,10 +1345,16 @@ nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version)
13371345
return NULL;
13381346
}
13391347

1348+
API char **
1349+
nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version)
1350+
{
1351+
return _nc_server_get_cpblts_version(ctx, version, 0);
1352+
}
1353+
13401354
API char **
13411355
nc_server_get_cpblts(const struct ly_ctx *ctx)
13421356
{
1343-
return nc_server_get_cpblts_version(ctx, LYS_VERSION_UNDEF);
1357+
return _nc_server_get_cpblts_version(ctx, LYS_VERSION_UNDEF, 0);
13441358
}
13451359

13461360
static int
@@ -1400,8 +1414,15 @@ parse_cpblts(struct lyd_node *capabilities, char ***list)
14001414
return ver;
14011415
}
14021416

1417+
/**
1418+
* @brief Send NETCONF hello message on a session.
1419+
*
1420+
* @param[in] session Session to send the message on.
1421+
* @param[in] config_locked Whether the configuration READ lock is already held (only relevant for server side).
1422+
* @return Sent message type.
1423+
*/
14031424
static NC_MSG_TYPE
1404-
nc_send_hello_io(struct nc_session *session)
1425+
nc_send_hello_io(struct nc_session *session, int config_locked)
14051426
{
14061427
NC_MSG_TYPE ret;
14071428
int i, timeout_io;
@@ -1419,7 +1440,7 @@ nc_send_hello_io(struct nc_session *session)
14191440
timeout_io = NC_CLIENT_HELLO_TIMEOUT * 1000;
14201441
sid = NULL;
14211442
} else {
1422-
cpblts = nc_server_get_cpblts_version(session->ctx, LYS_VERSION_1_0);
1443+
cpblts = _nc_server_get_cpblts_version(session->ctx, LYS_VERSION_1_0, config_locked);
14231444
if (!cpblts) {
14241445
return NC_MSG_ERROR;
14251446
}
@@ -1609,7 +1630,7 @@ nc_handshake_io(struct nc_session *session)
16091630
{
16101631
NC_MSG_TYPE type;
16111632

1612-
type = nc_send_hello_io(session);
1633+
type = nc_send_hello_io(session, 0);
16131634
if (type != NC_MSG_HELLO) {
16141635
return type;
16151636
}
@@ -1623,6 +1644,26 @@ nc_handshake_io(struct nc_session *session)
16231644
return type;
16241645
}
16251646

1647+
NC_MSG_TYPE
1648+
nc_ch_handshake_io(struct nc_session *session)
1649+
{
1650+
NC_MSG_TYPE type;
1651+
1652+
if ((session->side != NC_SERVER) || !(session->flags & NC_SESSION_CALLHOME)) {
1653+
ERR(session, "Call Home handshake can only be performed on a server session with Call Home flag set.");
1654+
return NC_MSG_ERROR;
1655+
}
1656+
1657+
type = nc_send_hello_io(session, 1);
1658+
if (type != NC_MSG_HELLO) {
1659+
return type;
1660+
}
1661+
1662+
type = nc_server_recv_hello_io(session);
1663+
1664+
return type;
1665+
}
1666+
16261667
#ifdef NC_ENABLED_SSH_TLS
16271668

16281669
/**

src/session_p.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1227,6 +1227,15 @@ int nc_ctx_check_and_fill(struct nc_session *session);
12271227
*/
12281228
NC_MSG_TYPE nc_handshake_io(struct nc_session *session);
12291229

1230+
/**
1231+
* @brief Perform NETCONF handshake on a server-side Call Home session.
1232+
*
1233+
* @param[in] session NETCONF session to use.
1234+
* @return NC_MSG_HELLO on success, NC_MSG_BAD_HELLO on client \<hello\> message parsing fail,
1235+
* NC_MSG_WOULDBLOCK on timeout, NC_MSG_ERROR on other error.
1236+
*/
1237+
NC_MSG_TYPE nc_ch_handshake_io(struct nc_session *session);
1238+
12301239
/**
12311240
* @brief Bind a socket to an address and a port.
12321241
*
@@ -1434,9 +1443,10 @@ int nc_session_tls_crl_from_cert_ext_fetch(void *leaf_cert, void *cert_store, vo
14341443
* @brief Check whether a module is not ignored by the server.
14351444
*
14361445
* @param[in] mod_name Module name to check.
1446+
* @param[in] config_locked Whether the configuration lock is already held or should be acquired in this function.
14371447
* @return Whether the module is ignored.
14381448
*/
1439-
int nc_server_is_mod_ignored(const char *mod_name);
1449+
int nc_server_is_mod_ignored(const char *mod_name, int config_locked);
14401450

14411451
/**
14421452
* Functions

src/session_server.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3016,7 +3016,7 @@ nc_connect_ch_endpt(struct nc_ch_endpt *endpt, nc_server_ch_session_acquire_ctx_
30163016
(*session)->id = ATOMIC_INC_RELAXED(server_opts.new_session_id);
30173017

30183018
/* NETCONF handshake */
3019-
msgtype = nc_handshake_io(*session);
3019+
msgtype = nc_ch_handshake_io(*session);
30203020
if (msgtype != NC_MSG_HELLO) {
30213021
goto fail;
30223022
}
@@ -4514,14 +4514,16 @@ nc_server_notif_cert_expiration_thread_stop(int wait)
45144514
#endif /* NC_ENABLED_SSH_TLS */
45154515

45164516
int
4517-
nc_server_is_mod_ignored(const char *mod_name)
4517+
nc_server_is_mod_ignored(const char *mod_name, int config_locked)
45184518
{
45194519
int ignored = 0;
45204520
LY_ARRAY_COUNT_TYPE i;
45214521

4522-
/* LOCK */
4523-
if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) {
4524-
return 0;
4522+
if (!config_locked) {
4523+
/* LOCK */
4524+
if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) {
4525+
return 0;
4526+
}
45254527
}
45264528

45274529
LY_ARRAY_FOR(server_opts.config.ignored_modules, i) {
@@ -4531,8 +4533,10 @@ nc_server_is_mod_ignored(const char *mod_name)
45314533
}
45324534
}
45334535

4534-
/* UNLOCK */
4535-
nc_rwlock_unlock(&server_opts.config_lock, __func__);
4536+
if (!config_locked) {
4537+
/* UNLOCK */
4538+
nc_rwlock_unlock(&server_opts.config_lock, __func__);
4539+
}
45364540

45374541
return ignored;
45384542
}

0 commit comments

Comments
 (0)