From: Libor Peltan <libor.peltan@nic.cz>
Date: Fri, 6 Nov 2020 16:51:10 +0000 (+0100)
Subject: zone: use a mutex for access to zone->flags
X-Git-Tag: v3.1.0~314^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=772eb52f3b08f44d2a6a2fd79bed8deea2c81c38;p=thirdparty%2Fknot-dns.git

zone: use a mutex for access to zone->flags
---

diff --git a/src/knot/ctl/commands.c b/src/knot/ctl/commands.c
index 9c7a8027dc..0f801694a9 100644
--- a/src/knot/ctl/commands.c
+++ b/src/knot/ctl/commands.c
@@ -349,7 +349,7 @@ static int zone_retransfer(zone_t *zone, ctl_args_t *args)
 		return KNOT_ENOTSUP;
 	}
 
-	zone->flags |= ZONE_FORCE_AXFR;
+	zone_set_flag(zone, ZONE_FORCE_AXFR);
 	schedule_trigger(zone, args, ZONE_EVENT_REFRESH, true);
 
 	return KNOT_EOK;
@@ -379,7 +379,7 @@ static int zone_flush(zone_t *zone, ctl_args_t *args)
 	}
 
 	if (ctl_has_flag(args->data[KNOT_CTL_IDX_FLAGS], CTL_FLAG_FORCE)) {
-		zone->flags |= ZONE_FORCE_FLUSH;
+		zone_set_flag(zone, ZONE_FORCE_FLUSH);
 	}
 
 	schedule_trigger(zone, args, ZONE_EVENT_FLUSH, true);
@@ -460,7 +460,7 @@ static int zone_sign(zone_t *zone, ctl_args_t *args)
 		return KNOT_ENOTSUP;
 	}
 
-	zone->flags |= ZONE_FORCE_RESIGN;
+	zone_set_flag(zone, ZONE_FORCE_RESIGN);
 	schedule_trigger(zone, args, ZONE_EVENT_DNSSEC, true);
 
 	return KNOT_EOK;
@@ -475,9 +475,9 @@ static int zone_key_roll(zone_t *zone, ctl_args_t *args)
 
 	const char *key_type = args->data[KNOT_CTL_IDX_TYPE];
 	if (strncasecmp(key_type, "ksk", 3) == 0) {
-		zone->flags |= ZONE_FORCE_KSK_ROLL;
+		zone_set_flag(zone, ZONE_FORCE_KSK_ROLL);
 	} else if (strncasecmp(key_type, "zsk", 3) == 0) {
-		zone->flags |= ZONE_FORCE_ZSK_ROLL;
+		zone_set_flag(zone, ZONE_FORCE_ZSK_ROLL);
 	} else {
 		return KNOT_EINVAL;
 	}
diff --git a/src/knot/events/handlers/dnssec.c b/src/knot/events/handlers/dnssec.c
index dfbe6993bb..ace19118db 100644
--- a/src/knot/events/handlers/dnssec.c
+++ b/src/knot/events/handlers/dnssec.c
@@ -75,22 +75,19 @@ int event_dnssec(conf_t *conf, zone_t *zone)
 	int sign_flags = 0;
 	bool zone_changed = false;
 
-	if (zone->flags & ZONE_FORCE_RESIGN) {
+	if (zone_get_flag(zone, ZONE_FORCE_RESIGN, true)) {
 		log_zone_info(zone->name, "DNSSEC, dropping previous "
 		              "signatures, re-signing zone");
-		zone->flags &= ~ZONE_FORCE_RESIGN;
 		sign_flags = ZONE_SIGN_DROP_SIGNATURES;
 	} else {
 		log_zone_info(zone->name, "DNSSEC, signing zone");
 		sign_flags = 0;
 	}
 
-	if (zone->flags & ZONE_FORCE_KSK_ROLL) {
-		zone->flags &= ~ZONE_FORCE_KSK_ROLL;
+	if (zone_get_flag(zone, ZONE_FORCE_KSK_ROLL, true)) {
 		r_flags |= KEY_ROLL_FORCE_KSK_ROLL;
 	}
-	if (zone->flags & ZONE_FORCE_ZSK_ROLL) {
-		zone->flags &= ~ZONE_FORCE_ZSK_ROLL;
+	if (zone_get_flag(zone, ZONE_FORCE_ZSK_ROLL, true)) {
 		r_flags |= KEY_ROLL_FORCE_ZSK_ROLL;
 	}
 
diff --git a/src/knot/events/handlers/refresh.c b/src/knot/events/handlers/refresh.c
index 78cfbbb043..646caf55b0 100644
--- a/src/knot/events/handlers/refresh.c
+++ b/src/knot/events/handlers/refresh.c
@@ -1263,8 +1263,7 @@ int event_refresh(conf_t *conf, zone_t *zone)
 	try_refresh_ctx_t trctx = { 0 };
 
 	// TODO: Flag on zone is ugly. Event specific parameters would be nice.
-	if (zone->flags & ZONE_FORCE_AXFR) {
-		zone->flags &= ~ZONE_FORCE_AXFR;
+	if (zone_get_flag(zone, ZONE_FORCE_AXFR, true)) {
 		trctx.force_axfr = true;
 		zone->zonefile.retransfer = true;
 	}
diff --git a/src/knot/nameserver/process_query.c b/src/knot/nameserver/process_query.c
index 0891845209..4c4a62999d 100644
--- a/src/knot/nameserver/process_query.c
+++ b/src/knot/nameserver/process_query.c
@@ -424,7 +424,7 @@ static int prepare_answer(knot_pkt_t *query, knot_pkt_t *resp, knot_layer_t *ctx
 	}
 
 	/* Allow normal queries to catalog only over TCP and if allowed by ACL. */
-	if (qdata->extra->zone != NULL && (qdata->extra->zone->flags & ZONE_IS_CATALOG) &&
+	if (qdata->extra->zone != NULL && qdata->extra->zone->is_catalog_flag &&
 	    query_type(query) == KNOTD_QUERY_TYPE_NORMAL) {
 		if ((qdata->params->flags & KNOTD_QUERY_FLAG_LIMIT_SIZE) ||
 		    !process_query_acl_check(conf(), ACL_ACTION_TRANSFER, qdata)) {
diff --git a/src/knot/updates/zone-update.c b/src/knot/updates/zone-update.c
index 1973bd7e52..6927bbb9a3 100644
--- a/src/knot/updates/zone-update.c
+++ b/src/knot/updates/zone-update.c
@@ -713,7 +713,7 @@ static int update_catalog(conf_t *conf, zone_update_t *update)
 		return (val.code == KNOT_ENOENT || val.code == KNOT_YP_EINVAL_ID) ? KNOT_EOK : val.code;
 	}
 
-	update->zone->flags |= ZONE_IS_CATALOG;
+	zone_set_flag(update->zone, ZONE_IS_CATALOG);
 
 	int ret = KNOT_EOK;
 	if ((update->flags & UPDATE_INCREMENTAL)) {
diff --git a/src/knot/zone/zone.c b/src/knot/zone/zone.c
index 7998d10cb1..5e608f28af 100644
--- a/src/knot/zone/zone.c
+++ b/src/knot/zone/zone.c
@@ -65,8 +65,7 @@ static int flush_journal(conf_t *conf, zone_t *zone, bool allow_empty_zone, bool
 	int ret = KNOT_EOK;
 	zone_journal_t j = zone_journal(zone);
 
-	bool force = zone->flags & ZONE_FORCE_FLUSH;
-	zone->flags &= ~ZONE_FORCE_FLUSH;
+	bool force = zone_get_flag(zone, ZONE_FORCE_FLUSH, true);
 
 	conf_val_t val = conf_zone_get(conf, C_ZONEFILE_SYNC, zone->name);
 	int64_t sync_timeout = conf_int(&val);
@@ -357,6 +356,38 @@ void zone_clear_preferred_master(zone_t *zone)
 	pthread_mutex_unlock(&zone->preferred_lock);
 }
 
+void zone_set_flag(zone_t *zone, zone_flag_t flag)
+{
+	if (zone == NULL) {
+		return;
+	}
+
+	pthread_mutex_lock(&zone->preferred_lock); // this mutex seems OK to be reused for this
+	zone->flags |= flag;
+	pthread_mutex_unlock(&zone->preferred_lock);
+
+	if (flag & ZONE_IS_CATALOG) {
+		zone->is_catalog_flag = true;
+	}
+}
+
+zone_flag_t zone_get_flag(zone_t *zone, zone_flag_t flag, bool clear)
+{
+	if (zone == NULL) {
+		return 0;
+	}
+
+	pthread_mutex_lock(&zone->preferred_lock);
+	zone_flag_t res = (zone->flags & flag);
+	if (clear && res) {
+		zone->flags &= ~flag;
+	}
+	assert(((bool)(zone->flags & ZONE_IS_CATALOG)) == zone->is_catalog_flag);
+	pthread_mutex_unlock(&zone->preferred_lock);
+
+	return res;
+}
+
 const knot_rdataset_t *zone_soa(const zone_t *zone)
 {
 	if (!zone || zone_contents_is_empty(zone->contents)) {
diff --git a/src/knot/zone/zone.h b/src/knot/zone/zone.h
index 1154e80dc7..db2e889047 100644
--- a/src/knot/zone/zone.h
+++ b/src/knot/zone/zone.h
@@ -34,7 +34,7 @@ struct zone_backup_ctx;
 /*!
  * \brief Zone flags.
  */
-typedef enum zone_flag_t {
+typedef enum {
 	ZONE_FORCE_AXFR     = 1 << 0, /*!< Force AXFR as next transfer. */
 	ZONE_FORCE_RESIGN   = 1 << 1, /*!< Force zone re-sign. */
 	ZONE_FORCE_FLUSH    = 1 << 2, /*!< Force zone flush. */
@@ -52,6 +52,7 @@ typedef struct zone
 	knot_dname_t *name;
 	zone_contents_t *contents;
 	zone_flag_t flags;
+	bool is_catalog_flag; //!< Lock-less indication of ZONE_IS_CATALOG flag.
 
 	/*! \brief Dynamic configuration zone change type. */
 	conf_io_type_t change_type;
@@ -93,7 +94,7 @@ typedef struct zone
 	catalog_t *catalog;
 	catalog_update_t *catalog_upd;
 
-	/*! \brief Preferred master lock. */
+	/*! \brief Preferred master lock. Also used for flags access. */
 	pthread_mutex_t preferred_lock;
 	/*! \brief Preferred master for remote operation. */
 	struct sockaddr_storage *preferred_master;
@@ -165,6 +166,12 @@ void zone_set_preferred_master(zone_t *zone, const struct sockaddr_storage *addr
 /*! \brief Clears the current preferred master address. */
 void zone_clear_preferred_master(zone_t *zone);
 
+/*! \brief Sets a zone flag. */
+void zone_set_flag(zone_t *zone, zone_flag_t flag);
+
+/*! \brief Returns if a flag is set (and optionally clears it). */
+zone_flag_t zone_get_flag(zone_t *zone, zone_flag_t flag, bool clear);
+
 /*! \brief Get zone SOA RR. */
 const knot_rdataset_t *zone_soa(const zone_t *zone);
 
diff --git a/src/knot/zone/zonedb-load.c b/src/knot/zone/zonedb-load.c
index aba2b76ae7..e7b0bf09f7 100644
--- a/src/knot/zone/zonedb-load.c
+++ b/src/knot/zone/zonedb-load.c
@@ -127,7 +127,7 @@ static zone_t *create_zone_reload(conf_t *conf, const knot_dname_t *name,
 	}
 
 	zone->contents = old_zone->contents;
-	zone->flags = (old_zone->flags & (ZONE_IS_CATALOG | ZONE_IS_CAT_MEMBER));
+	zone_set_flag(zone, zone_get_flag(old_zone, ZONE_IS_CATALOG | ZONE_IS_CAT_MEMBER, false));
 
 	zone->timers = old_zone->timers;
 	timers_sanitize(conf, zone);
@@ -263,7 +263,7 @@ static bool check_open_catalog(catalog_t *cat)
 static zone_t *reuse_member_zone(zone_t *zone, server_t *server, conf_t *conf,
                                  list_t *expired_contents)
 {
-	if (!(zone->flags & ZONE_IS_CAT_MEMBER)) {
+	if (!zone_get_flag(zone, ZONE_IS_CAT_MEMBER, false)) {
 		return NULL;
 	}
 
@@ -283,7 +283,7 @@ static zone_t *reuse_member_zone(zone_t *zone, server_t *server, conf_t *conf,
 	if (newzone == NULL) {
 		log_zone_error(zone->name, "zone cannot be created");
 	} else {
-		assert(newzone->flags & ZONE_IS_CAT_MEMBER);
+		assert(zone_get_flag(newzone, ZONE_IS_CAT_MEMBER, false));
 		conf_activate_modules(conf, server, newzone->name, &newzone->query_modules,
 		                      &newzone->query_plan);
 	}
@@ -302,7 +302,7 @@ static zone_t *reuse_cold_zone(const knot_dname_t *zname, server_t *server, conf
 	if (zone == NULL) {
 		log_zone_error(zname, "zone cannot be created");
 	} else {
-		zone->flags |= ZONE_IS_CAT_MEMBER;
+		zone_set_flag(zone, ZONE_IS_CAT_MEMBER);
 		conf_activate_modules(conf, server, zone->name, &zone->query_modules,
 		                      &zone->query_plan);
 	}
@@ -326,7 +326,7 @@ static zone_t *add_member_zone(catalog_upd_val_t *val, knot_zonedb_t *check,
 		log_zone_error(val->member, "zone cannot be created");
 		catalog_del2(conf->catalog, val);
 	} else {
-		zone->flags |= ZONE_IS_CAT_MEMBER;
+		zone_set_flag(zone, ZONE_IS_CAT_MEMBER);
 		conf_activate_modules(conf, server, zone->name, &zone->query_modules,
 		                      &zone->query_plan);
 		log_zone_info(val->member, "zone added from catalog");