Added checks of changed handlers or blocked signal after plugin function calls

CHANGELOG.md

@@ -68,6 +68,7 @@ Developers may need to change their code
 
 ### Minor features
 
+* Added checks of changed handlers or blocked signal after plugin function calls
 * Added set/get pointer API to clixon_data:
    * clicon_ptr_get(), clicon_ptr_set(), 
 * Restconf YANG PATCH according to RFC 8072

apps/backend/backend_commit.c

@@ -937,10 +937,16 @@ from_client_restart_one(clicon_handle h,
 	goto done;
     /* Application may define extra xml in its reset function*/
     if ((resetfn = clixon_plugin_api_get(cp)->ca_reset) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if ((retval = resetfn(h, db)) < 0) {
 	    clicon_debug(1, "plugin_start() failed");
 	    goto done;
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     /* 1. Start transaction */
     if ((td = transaction_new()) == NULL)

apps/backend/backend_plugin.c

@@ -82,12 +82,18 @@ clixon_plugin_reset_one(clixon_plugin_t *cp,
     plgreset_t  *fn;       /* callback */
 
     if ((fn = clixon_plugin_api_get(cp)->ca_reset) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, db) < 0) {
 	    if (clicon_errno < 0) 
 		clicon_log(LOG_WARNING, "%s: Internal error: Reset callback in plugin: %s returned -1 but did not make a clicon_err call",
 			   __FUNCTION__, clixon_plugin_name_get(cp));
 	    goto done;
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -133,6 +139,10 @@ clixon_plugin_pre_daemon_one(clixon_plugin_t *cp,
     plgdaemon_t *fn;          /* Daemonize plugin callback function */
 
     if ((fn = clixon_plugin_api_get(cp)->ca_pre_daemon) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
         if (fn(h) < 0) {
             if (clicon_errno < 0)
                 clicon_log(LOG_WARNING, "%s: Internal error: Pre-daemon callback in plugin:\
@@ -140,6 +150,8 @@ clixon_plugin_pre_daemon_one(clixon_plugin_t *cp,
                            __FUNCTION__, clixon_plugin_name_get(cp));
             goto done;
         }
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -186,12 +198,18 @@ clixon_plugin_daemon_one(clixon_plugin_t *cp,
     plgdaemon_t *fn;          /* Daemonize plugin callback function */    
 
     if ((fn = clixon_plugin_api_get(cp)->ca_daemon) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h) < 0) {
 	    if (clicon_errno < 0) 
 		clicon_log(LOG_WARNING, "%s: Internal error: Daemon callback in plugin: %s returned -1 but did not make a clicon_err call",
 			   __FUNCTION__, clixon_plugin_name_get(cp));
 	    goto done;
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -265,12 +283,18 @@ clixon_plugin_statedata_one(clixon_plugin_t *cp,
     if ((fn = clixon_plugin_api_get(cp)->ca_statedata) != NULL){
 	if ((x = xml_new(DATASTORE_TOP_SYMBOL, NULL, CX_ELMNT)) == NULL)
 	    goto done;
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, nsc, xpath, x) < 0){
 	    if (clicon_errno < 0) 
 		clicon_log(LOG_WARNING, "%s: Internal error: State callback in plugin: %s returned -1 but did not make a clicon_err call",
 			   __FUNCTION__, clixon_plugin_name_get(cp));
 	    goto fail;  /* Dont quit here on user callbacks */
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     if (xp && x)
 	*xp = x;
@@ -410,8 +434,14 @@ clixon_plugin_lockdb_one(clixon_plugin_t *cp,
     plglockdb_t *fn;          /* Plugin statedata fn */
     
     if ((fn = clixon_plugin_api_get(cp)->ca_lockdb) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, db, lock, id) < 0)
 	    goto done;  
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -579,12 +609,18 @@ plugin_transaction_begin_one(clixon_plugin_t    *cp,
     trans_cb_t *fn;
     
     if ((fn = clixon_plugin_api_get(cp)->ca_trans_begin) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, (transaction_data)td) < 0){
 	    if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */
 		clicon_log(LOG_NOTICE, "%s: Plugin '%s' callback does not make clicon_err call on error", 
 		       __FUNCTION__, clixon_plugin_name_get(cp));
 	    goto done;
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -632,12 +668,18 @@ plugin_transaction_validate_one(clixon_plugin_t      *cp,
     trans_cb_t *fn;
     
     if ((fn = clixon_plugin_api_get(cp)->ca_trans_validate) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, (transaction_data)td) < 0){
 	    if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */
 		clicon_log(LOG_NOTICE, "%s: Plugin '%s' callback does not make clicon_err call on error", 
 		       __FUNCTION__, clixon_plugin_name_get(cp));
 	    goto done;
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -683,12 +725,18 @@ plugin_transaction_complete_one(clixon_plugin_t      *cp,
     trans_cb_t *fn;
     
     if ((fn = clixon_plugin_api_get(cp)->ca_trans_complete) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, (transaction_data)td) < 0){
 	    if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */
 		clicon_log(LOG_NOTICE, "%s: Plugin '%s' callback does not make clicon_err call on error", 
 		       __FUNCTION__, clixon_plugin_name_get(cp));
 	    goto done;
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -766,12 +814,18 @@ plugin_transaction_commit_one(clixon_plugin_t      *cp,
     trans_cb_t *fn;
     
     if ((fn = clixon_plugin_api_get(cp)->ca_trans_commit) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, (transaction_data)td) < 0){
 	    if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */
 		clicon_log(LOG_NOTICE, "%s: Plugin '%s' callback does not make clicon_err call on error", 
 		       __FUNCTION__, clixon_plugin_name_get(cp));
 	    goto done;
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -825,12 +879,18 @@ plugin_transaction_commit_done_one(clixon_plugin_t      *cp,
     trans_cb_t *fn;
     
     if ((fn = clixon_plugin_api_get(cp)->ca_trans_commit_done) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, (transaction_data)td) < 0){
 	    if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */
 		clicon_log(LOG_NOTICE, "%s: Plugin '%s' callback does not make clicon_err call on error", 
 		       __FUNCTION__, clixon_plugin_name_get(cp));
 	    goto done;
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -876,12 +936,18 @@ plugin_transaction_end_one(clixon_plugin_t      *cp,
     trans_cb_t *fn;
     
     if ((fn = clixon_plugin_api_get(cp)->ca_trans_end) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, (transaction_data)td) < 0){
 	    if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */
 		clicon_log(LOG_NOTICE, "%s: Plugin '%s' callback does not make clicon_err call on error", 
 		       __FUNCTION__, clixon_plugin_name_get(cp));
 	    goto done;
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -920,12 +986,18 @@ plugin_transaction_abort_one(clixon_plugin_t      *cp,
     trans_cb_t *fn;
     
     if ((fn = clixon_plugin_api_get(cp)->ca_trans_abort) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, (transaction_data)td) < 0){
 	    if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */
 		clicon_log(LOG_NOTICE, "%s: Plugin '%s' callback does not make clicon_err call on error", 
 		       __FUNCTION__, clixon_plugin_name_get(cp));
 	    goto done;
 	}
+	if (plugin_context_check(&pc, clixon_plugin_name_get(cp), __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:

apps/cli/cli_plugin.c

@@ -522,6 +522,47 @@ cli_handler_err(FILE *f)
     return 0;
 }
 
+/*! Variant of eval for context checking 
+ * @see cligen_eval
+ */
+int
+cligen_clixon_eval(cligen_handle h, 
+		   cg_obj       *co, 
+		   cvec         *cvv)
+{
+    struct cg_callback *cc;
+    int                 retval = 0;
+    cvec               *argv;
+
+    if (h)
+	cligen_co_match_set(h, co);
+    for (cc = co->co_callbacks; cc; cc=cc->cc_next){
+	/* Vector cvec argument to callback */
+    	if (cc->cc_fn_vec){
+	    plugin_context_t  pc = {0,};
+	    argv = cc->cc_cvec ? cvec_dup(cc->cc_cvec) : NULL;
+	    cligen_fn_str_set(h, cc->cc_fn_str);
+	    if (plugin_context_get(&pc) < 0)
+		break;
+	    if ((retval = (*cc->cc_fn_vec)(
+					cligen_userhandle(h)?cligen_userhandle(h):h, 
+					cvv, 
+					argv)) < 0){
+		if (argv != NULL)
+		    cvec_free(argv);
+		cligen_fn_str_set(h, NULL);
+		break;
+	    }
+	    if (plugin_context_check(&pc, "CLIgen", cc->cc_fn_str) < 0)
+		break;
+	    if (argv != NULL)
+		cvec_free(argv);
+	    cligen_fn_str_set(h, NULL);
+	}
+    }
+    return retval;
+}
+
 /*! Evaluate a matched command
  * @param[in]     h         Clicon handle
  * @param[in]     cmd       The command string
@@ -541,7 +582,8 @@ clicon_eval(clicon_handle h,
     cli_output_reset();
     if (!cligen_exiting(cli_cligen(h))) {	
 	clicon_err_reset();
-	if ((retval = cligen_eval(cli_cligen(h), match_obj, cvv)) < 0) {
+
+	if ((retval = cligen_clixon_eval(cli_cligen(h), match_obj, cvv)) < 0) {
 #if 0 /* This is removed since we get two error messages on failure.
 	 But maybe only sometime?
 	 Both a real log when clicon_err is called, and the  here again.

lib/clixon/clixon_plugin.h

@@ -337,6 +337,17 @@ struct clixon_plugin_api{
 #define ca_trans_abort    u.cau_backend.cb_trans_abort
 #define ca_datastore_upgrade  u.cau_backend.cb_datastore_upgrade
 
+/*! Structure for checking status before and after a plugin call
+ * Currently signal settings: blocked and handlers, could be extended to more
+ * @see plugin_context_check
+ */
+struct plugin_context {
+    sigset_t         pc_sigset;            /* See sigprocmask(2) */
+    struct sigaction pc_sigaction_vec[32]; /* See sigaction(2) */
+    int              pc_status;            /* 0: OK, -1: fail */
+};
+typedef struct plugin_context plugin_context_t;
+
 /*
  * Macros
  */
@@ -375,6 +386,9 @@ int clixon_plugins_load(clicon_handle h, const char *function, const char *dir,
 
 int clixon_pseudo_plugin(clicon_handle h, const char *name, clixon_plugin_t **cpp);
 
+int plugin_context_get(plugin_context_t *pc);
+int plugin_context_check(plugin_context_t *pc, const char *name, const char *fn);
+
 int clixon_plugin_start_one(clixon_plugin_t *cp, clicon_handle h);
 int clixon_plugin_start_all(clicon_handle h);
 

lib/src/clixon_plugin.c

@@ -43,6 +43,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <errno.h>
+#include <signal.h>
 #include <dlfcn.h>
 #include <dirent.h>
 #include <syslog.h>
@@ -330,6 +331,7 @@ plugin_load_one(clicon_handle   h,
     clixon_plugin_t     *cp = NULL;
     char              *name;
     char              *p;
+    plugin_context_t   pc = {0,};
 
     clicon_debug(1, "%s file:%s function:%s", __FUNCTION__, file, function);
     dlerror();    /* Clear any existing error */
@@ -348,6 +350,11 @@ plugin_load_one(clicon_handle   h,
 	goto done;
     }
     clicon_err_reset();
+
+
+    if (plugin_context_get(&pc) < 0)
+	goto done;
+
     if ((api = initfn(h)) == NULL) {
 	if (!clicon_errno){ 	/* if clicon_err() is not called then log and continue */
 	    clicon_log(LOG_DEBUG, "Warning: failed to initiate %s", strrchr(file,'/')?strchr(file, '/'):file);
@@ -359,6 +366,9 @@ plugin_load_one(clicon_handle   h,
 	    goto done;
 	}
     }
+    if (plugin_context_check(&pc, file, __FUNCTION__) < 0)
+	goto done;
+
     /* Note: sizeof clixon_plugin_api which is largest of clixon_plugin_api:s */
     if ((cp = (clixon_plugin_t *)malloc(sizeof(struct clixon_plugin))) == NULL){
 	clicon_err(OE_UNIX, errno, "malloc");
@@ -483,6 +493,93 @@ done:
     return retval;
 }
 
+/*! Get system context, eg signal procmask (for blocking) and sigactions
+ * Call this before a plugin
+ * @see plugin_context_check
+ */
+int
+plugin_context_get(plugin_context_t *pc)
+{
+    int retval = -1;
+    int i;
+
+    if (sigprocmask(0, NULL, &pc->pc_sigset) < 0){
+	clicon_err(OE_UNIX, errno, "sigprocmask");
+	goto done;
+    }
+    for (i=1; i<32; i++){
+	if (sigaction(i, NULL, &pc->pc_sigaction_vec[i]) < 0){
+	    clicon_err(OE_UNIX, errno, "sigaction");
+	    goto done;
+	}
+	/* Mask SA_RESTORER: Not intended for application use.
+	 * Note that it may not be included in user space so may be hardcoded below
+	 */
+#ifdef SA_RESTORER
+	pc->pc_sigaction_vec[i].sa_flags &= ~SA_RESTORER;
+#else
+	pc->pc_sigaction_vec[i].sa_flags &= ~0x04000000;
+#endif
+    }
+    retval = 0;
+ done:
+    return retval;
+}
+
+/*! Given an existing, old plugin context, check if anytjing has changed
+ * @param[in,out] oldpc  Old plugin context, status will be returned inside oldpc
+ * @param[in]     name   Name of plugin for logging
+ * @param[in]     fn     Name of callback
+ * @retval       -1      Error
+ * @retval        0      OK
+ * @see plugin_context_get
+ */
+int
+plugin_context_check(plugin_context_t *oldpc,
+		     const char       *name,
+    		     const char       *fn)
+{
+    int              retval = -1;
+    int              failed;
+    int              i;
+    plugin_context_t newpc = {0, };
+
+    if (plugin_context_get(&newpc) < 0)
+	goto done;
+    for (i=1; i<32; i++){
+	failed = 0;
+	if (sigismember(&oldpc->pc_sigset, i) != sigismember(&newpc.pc_sigset, i)){
+	    clicon_log(LOG_WARNING, "%s Plugin %s %s: Changed blocking of signal %s(%d) from %d to %d", __FUNCTION__,
+		       name, fn, strsignal(i), i,
+		       sigismember(&oldpc->pc_sigset, i),
+		       sigismember(&newpc.pc_sigset, i)
+		       );
+	    failed++;
+	}
+	if (oldpc->pc_sigaction_vec[i].sa_flags != newpc.pc_sigaction_vec[i].sa_flags){
+	    clicon_log(LOG_WARNING, "%s Plugin %s %s: Changed flags of signal %s(%d) from 0x%x to 0x%x", __FUNCTION__,
+		       name, fn, strsignal(i), i,
+		       oldpc->pc_sigaction_vec[i].sa_flags,
+		       newpc.pc_sigaction_vec[i].sa_flags);;
+	    failed++;
+	}
+	if (oldpc->pc_sigaction_vec[i].sa_sigaction != newpc.pc_sigaction_vec[i].sa_sigaction){
+	    clicon_log(LOG_WARNING, "%s Plugin %s %s: Changed action of signal %s(%d) from %p to %p", __FUNCTION__,
+		       name, fn, strsignal(i), i,
+		       oldpc->pc_sigaction_vec[i].sa_sigaction,
+		       newpc.pc_sigaction_vec[i].sa_sigaction);
+	    failed++;
+	}
+	if (failed){
+	    oldpc->pc_status = -1;
+	}
+	/* assert(oldpc->pc_status == 0); */
+    }
+    retval = 0;
+ done:
+    return retval;
+}
+
 /*! Call single plugin start callback
  * @param[in]  cp      Plugin handle
  * @param[in]  h       Clixon handle
@@ -497,12 +594,18 @@ clixon_plugin_start_one(clixon_plugin_t *cp,
     plgstart_t  *fn;          /* Plugin start */
 
     if ((fn = cp->cp_api.ca_start) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h) < 0) {
 	    if (clicon_errno < 0) 
 		clicon_log(LOG_WARNING, "%s: Internal error: Start callback in plugin: %s returned -1 but did not make a clicon_err call",
 			   __FUNCTION__, cp->cp_name);
 	    goto done;
 	}
+	if (plugin_context_check(&pc, cp->cp_name, __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -545,12 +648,18 @@ clixon_plugin_exit_one(clixon_plugin_t *cp,
     plgexit_t   *fn;
 
     if ((fn = cp->cp_api.ca_exit) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h) < 0) {
 	    if (clicon_errno < 0) 
 		clicon_log(LOG_WARNING, "%s: Internal error: Exit callback in plugin: %s returned -1 but did not make a clicon_err call",
 			   __FUNCTION__, cp->cp_name);
 	    goto done;
 	}
+	if (plugin_context_check(&pc, cp->cp_name, __FUNCTION__) < 0)
+	    goto done;
 	if (dlclose(cp->cp_handle) != 0) {
 	    error = (char*)dlerror();
 	    clicon_err(OE_PLUGIN, errno, "dlclose: %s", error ? error : "Unknown error");
@@ -611,12 +720,18 @@ clixon_plugin_auth_one(clixon_plugin_t     *cp,
 
     clicon_debug(1, "%s", __FUNCTION__);
     if ((fn = cp->cp_api.ca_auth) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if ((retval = fn(h, req, auth_type, authp)) < 0) {
 	    if (clicon_errno < 0) 
 		clicon_log(LOG_WARNING, "%s: Internal error: Auth callback in plugin: %s returned -1 but did not make a clicon_err call",
 			   __FUNCTION__, cp->cp_name);
 	    goto done;
 	}
+	if (plugin_context_check(&pc, cp->cp_name, __FUNCTION__) < 0)
+	    goto done;
     }
     else
 	retval = 0; /* Ignored / no callback */
@@ -688,12 +803,18 @@ clixon_plugin_extension_one(clixon_plugin_t *cp,
     plgextension_t *fn;          /* Plugin extension fn */
     
     if ((fn = cp->cp_api.ca_extension) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, yext, ys) < 0) {
 	    if (clicon_errno < 0) 
 		clicon_log(LOG_WARNING, "%s: Internal error: Extension callback in plugin: %s returned -1 but did not make a clicon_err call",
 			   __FUNCTION__, cp->cp_name);
 	    goto done;
 	}
+	if (plugin_context_check(&pc, cp->cp_name, __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done:
@@ -752,12 +873,18 @@ clixon_plugin_datastore_upgrade_one(clixon_plugin_t   *cp,
     datastore_upgrade_t *fn;
     
     if ((fn = cp->cp_api.ca_datastore_upgrade) != NULL){
+	plugin_context_t  pc = {0,};
+
+	if (plugin_context_get(&pc) < 0)
+	    goto done;
 	if (fn(h, db, xt, msd) < 0) {
 	    if (clicon_errno < 0) 
 		clicon_log(LOG_WARNING, "%s: Internal error: Datastore upgrade callback in plugin: %s returned -1 but did not make a clicon_err call",
 			   __FUNCTION__, cp->cp_name);
 	    goto done;
 	}
+	if (plugin_context_check(&pc, cp->cp_name, __FUNCTION__) < 0)
+	    goto done;
     }
     retval = 0;
  done: