Merge pull request #2530 from pacovn/Coverity_1399295_Out-of-bounds_read

Coverity 1399295 out of bounds read

diff --git a/doc/user/basic.rst b/doc/user/basic.rst
index a75017c4429ec3ef582ec39eee483d6b58cf513d..cb46080055bde7fc1d69621f320a7f0d4a3388b8 100644 (file)
--- a/doc/user/basic.rst
+++ b/doc/user/basic.rst
@@ -405,6 +405,18 @@ These options apply to all |PACKAGE_NAME| daemons.
 
    Print program version.
 
+.. option:: --log <stdout|syslog|file:/path/to/log/file>
+
+   When initializing the daemon, setup the log to go to either stdout,
+   syslog or to a file.  These values will be displayed as part of
+   a show run.  Additionally they can be overridden at runtime if
+   desired via the normal log commands.
+
+.. option:: --log-level <emergencies|alerts|critical|errors|warnings|notifications|informational|debugging>
+
+   When initializing the daemon, allow the specification of a default
+   log level at startup from one of the specified levels.
+
 .. _loadable-module-support:
 
 Loadable Module Support

diff --git a/ldpd/ldpd.c b/ldpd/ldpd.c
index b265c98dae3d43d990d246807035a576f8016d03..b51ff82cea5a9b035fa182dddcc4b64009d44ad0 100644 (file)
--- a/ldpd/ldpd.c
+++ b/ldpd/ldpd.c
@@ -187,6 +187,22 @@ FRR_DAEMON_INFO(ldpd, LDP,
        .privs = &ldpd_privs,
 )
 
+static int ldp_config_fork_apply(struct thread *t)
+{
+       /*
+        * So the frr_config_fork() function schedules
+        * the read of the vty config( if there is a
+        * non-integrated config ) to be after the
+        * end of startup and we are starting the
+        * main process loop.  We need to schedule
+        * the application of this if necessary
+        * after the read in of the config.
+        */
+       ldp_config_apply(NULL, vty_conf);
+
+       return 0;
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -195,6 +211,7 @@ main(int argc, char *argv[])
        int                      pipe_parent2ldpe[2], pipe_parent2ldpe_sync[2];
        int                      pipe_parent2lde[2], pipe_parent2lde_sync[2];
        char                    *ctl_sock_name;
+       struct thread           *thread = NULL;
 
        ldpd_process = PROC_MAIN;
        log_procname = log_procnames[ldpd_process];
@@ -331,7 +348,7 @@ main(int argc, char *argv[])
        frr_config_fork();
 
        /* apply configuration */
-       ldp_config_apply(NULL, vty_conf);
+       thread_add_event(master, ldp_config_fork_apply, NULL, 0, &thread);
 
        /* setup pipes to children */
        if ((iev_ldpe = calloc(1, sizeof(struct imsgev))) == NULL ||

diff --git a/lib/command.c b/lib/command.c
index a8e61c6bb4f1a8784ea2a441be104b1760c6deb9..d666e8c593df9372e71214ed9d5111882faa6484 100644 (file)
--- a/lib/command.c
+++ b/lib/command.c
@@ -2429,7 +2429,8 @@ static int set_log_file(struct vty *vty, const char *fname, int loglevel)
                XFREE(MTYPE_TMP, p);
 
        if (!ret) {
-               vty_out(vty, "can't open logfile %s\n", fname);
+               if (vty)
+                       vty_out(vty, "can't open logfile %s\n", fname);
                return CMD_WARNING_CONFIG_FAILED;
        }
 
@@ -2445,6 +2446,39 @@ static int set_log_file(struct vty *vty, const char *fname, int loglevel)
        return CMD_SUCCESS;
 }
 
+void command_setup_early_logging(const char *dest, const char *level)
+{
+       char *token;
+
+       if (level) {
+               int nlevel = level_match(level);
+
+               if (nlevel != ZLOG_DISABLED)
+                       zlog_default->default_lvl = nlevel;
+       }
+
+       if (!dest)
+               return;
+
+       if (strcmp(dest, "stdout") == 0) {
+               zlog_set_level(ZLOG_DEST_STDOUT, zlog_default->default_lvl);
+               return;
+       }
+
+       if (strcmp(dest, "syslog") == 0) {
+               zlog_set_level(ZLOG_DEST_SYSLOG, zlog_default->default_lvl);
+               return;
+       }
+
+       token = strstr(dest, ":");
+       if (token == NULL)
+               return;
+
+       token++;
+
+       set_log_file(NULL, token, zlog_default->default_lvl);
+}
+
 DEFUN (config_log_file,
        config_log_file_cmd,
        "log file FILENAME [<emergencies|alerts|critical|errors|warnings|notifications|informational|debugging>]",

diff --git a/lib/command.h b/lib/command.h
index 9bf482f41b6054c902adce326dd8839140a374fe..395c971c55535e07b0db746af7b87cddc0bd3884 100644 (file)
--- a/lib/command.h
+++ b/lib/command.h
@@ -479,4 +479,5 @@ extern void
 cmd_variable_handler_register(const struct cmd_variable_handler *cvh);
 extern char *cmd_variable_comp2str(vector comps, unsigned short cols);
 
+extern void command_setup_early_logging(const char *dest, const char *level);
 #endif /* _ZEBRA_COMMAND_H */

diff --git a/lib/libfrr.c b/lib/libfrr.c
index 88203fbeb6bb57e424d5a1a2a5bfbd7db0d78fd2..505bea9b18fef91858c2b60de579a0449c4de6f2 100644 (file)
--- a/lib/libfrr.c
+++ b/lib/libfrr.c
@@ -78,6 +78,8 @@ static void opt_extend(const struct optspec *os)
 
 #define OPTION_VTYSOCK   1000
 #define OPTION_MODULEDIR 1002
+#define OPTION_LOG       1003
+#define OPTION_LOGLEVEL  1004
 
 static const struct option lo_always[] = {
        {"help", no_argument, NULL, 'h'},
@@ -86,6 +88,8 @@ static const struct option lo_always[] = {
        {"module", no_argument, NULL, 'M'},
        {"vty_socket", required_argument, NULL, OPTION_VTYSOCK},
        {"moduledir", required_argument, NULL, OPTION_MODULEDIR},
+       {"log", required_argument, NULL, OPTION_LOG},
+       {"log-level", required_argument, NULL, OPTION_LOGLEVEL},
        {NULL}};
 static const struct optspec os_always = {
        "hvdM:",
@@ -94,7 +98,9 @@ static const struct optspec os_always = {
        "  -d, --daemon       Runs in daemon mode\n"
        "  -M, --module       Load specified module\n"
        "      --vty_socket   Override vty socket path\n"
-       "      --moduledir    Override modules directory\n",
+       "      --moduledir    Override modules directory\n"
+       "      --log          Set Logging to stdout, syslog, or file:<name>\n"
+       "      --log-level    Set Logging Level to use, debug, info, warn, etc\n",
        lo_always};
 
 
@@ -444,6 +450,12 @@ static int frr_opt(int opt)
                        return 1;
                di->privs->group = optarg;
                break;
+       case OPTION_LOG:
+               di->early_logging = optarg;
+               break;
+       case OPTION_LOGLEVEL:
+               di->early_loglevel = optarg;
+               break;
        default:
                return 1;
        }
@@ -543,9 +555,8 @@ struct thread_master *frr_init(void)
 
        openzlog(di->progname, di->logname, di->instance,
                 LOG_CONS | LOG_NDELAY | LOG_PID, LOG_DAEMON);
-#if defined(HAVE_CUMULUS)
-       zlog_set_level(ZLOG_DEST_SYSLOG, zlog_default->default_lvl);
-#endif
+
+       command_setup_early_logging(di->early_logging, di->early_loglevel);
 
        if (!frr_zclient_addr(&zclient_addr, &zclient_addr_len,
                              frr_zclientpath)) {
@@ -721,15 +732,37 @@ static void frr_daemonize(void)
        frr_daemon_wait(fds[0]);
 }
 
+/*
+ * Why is this a thread?
+ *
+ * The read in of config for integrated config happens *after*
+ * thread execution starts( because it is passed in via a vtysh -b -n )
+ * While if you are not using integrated config we want the ability
+ * to read the config in after thread execution starts, so that
+ * we can match this behavior.
+ */
+static int frr_config_read_in(struct thread *t)
+{
+       if (!vty_read_config(di->config_file, config_default) &&
+           di->backup_config_file) {
+               zlog_info("Attempting to read backup config file: %s specified",
+                         di->backup_config_file);
+               vty_read_config(di->backup_config_file, config_default);
+       }
+       return 0;
+}
+
 void frr_config_fork(void)
 {
        hook_call(frr_late_init, master);
 
-       vty_read_config(di->config_file, config_default);
-
        /* Don't start execution if we are in dry-run mode */
-       if (di->dryrun)
+       if (di->dryrun) {
+               frr_config_read_in(NULL);
                exit(0);
+       }
+
+       thread_add_event(master, frr_config_read_in, NULL, 0, &di->read_in);
 
        if (di->daemon_mode || di->terminal)
                frr_daemonize();

diff --git a/lib/libfrr.h b/lib/libfrr.h
index 7ffa780bfb9a75d8d6316a0b73d564d9399650d4..d2552799069ce9233d048c7f2a46f8383e9d3f64 100644 (file)
--- a/lib/libfrr.h
+++ b/lib/libfrr.h
@@ -50,11 +50,16 @@ struct frr_daemon_info {
        bool dryrun;
        bool daemon_mode;
        bool terminal;
+
+       struct thread *read_in;
        const char *config_file;
+       const char *backup_config_file;
        const char *pid_file;
        const char *vty_path;
        const char *module_path;
        const char *pathspace;
+       const char *early_logging;
+       const char *early_loglevel;
 
        const char *proghelp;
        void (*printhelp)(FILE *target);

diff --git a/lib/privs.c b/lib/privs.c
index cfe7d6d6f81bdf802191e2559f1438ab7c985e98..7c99742d3407bf3454b30ac2c50436b226a8d79f 100644 (file)
--- a/lib/privs.c
+++ b/lib/privs.c
@@ -824,6 +824,19 @@ void zprivs_init(struct zebra_privs_t *zprivs)
 
 #ifdef HAVE_CAPABILITIES
        zprivs_caps_init(zprivs);
+
+       /*
+        * If we have initialized the system with no requested
+        * capabilities, change will not have been set
+        * to anything by zprivs_caps_init, As such
+        * we should make sure that when we attempt
+        * to raize privileges that we actually have
+        * a do nothing function to call instead of a
+        * crash :).
+        */
+       if (!zprivs->change)
+               zprivs->change = zprivs_change_null;
+
 #else  /* !HAVE_CAPABILITIES */
        /* we dont have caps. we'll need to maintain rid and saved uid
         * and change euid back to saved uid (who we presume has all neccessary

diff --git a/lib/vty.c b/lib/vty.c
index 280b2ace51a64bd64c5df459b5291854bc2ab554..e9d1f2e323747eaf7879af78fb34b1e22513352e 100644 (file)
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -2462,12 +2462,13 @@ static FILE *vty_use_backup_config(const char *fullpath)
 }
 
 /* Read up configuration file from file_name. */
-void vty_read_config(const char *config_file, char *config_default_dir)
+bool vty_read_config(const char *config_file, char *config_default_dir)
 {
        char cwd[MAXPATHLEN];
        FILE *confp = NULL;
        const char *fullpath;
        char *tmp = NULL;
+       bool read_success = false;
 
        /* If -f flag specified. */
        if (config_file != NULL) {
@@ -2525,8 +2526,10 @@ void vty_read_config(const char *config_file, char *config_default_dir)
 
                if (strstr(config_default_dir, "vtysh") == NULL) {
                        ret = stat(integrate_default, &conf_stat);
-                       if (ret >= 0)
+                       if (ret >= 0) {
+                               read_success = true;
                                goto tmp_free_and_out;
+                       }
                }
 #endif /* VTYSH */
                confp = fopen(config_default_dir, "r");
@@ -2550,6 +2553,7 @@ void vty_read_config(const char *config_file, char *config_default_dir)
        }
 
        vty_read_file(confp);
+       read_success = true;
 
        fclose(confp);
 
@@ -2558,6 +2562,8 @@ void vty_read_config(const char *config_file, char *config_default_dir)
 tmp_free_and_out:
        if (tmp)
                XFREE(MTYPE_TMP, tmp);
+
+       return read_success;
 }
 
 /* Small utility function which output log to the VTY. */

diff --git a/lib/vty.h b/lib/vty.h
index d14ddf5908d69dbe4a7825ba061dd764288bff79..b55abf22043f4956617b05ce4aab9084e3807b47 100644 (file)
--- a/lib/vty.h
+++ b/lib/vty.h
@@ -242,7 +242,7 @@ extern void vty_frame(struct vty *, const char *, ...) PRINTF_ATTRIBUTE(2, 3);
 extern void vty_endframe(struct vty *, const char *);
 bool vty_set_include(struct vty *vty, const char *regexp);
 
-extern void vty_read_config(const char *, char *);
+extern bool vty_read_config(const char *, char *);
 extern void vty_time_print(struct vty *, int);
 extern void vty_serv_sock(const char *, unsigned short, const char *);
 extern void vty_close(struct vty *);

diff --git a/ospf6d/ospf6_intra.c b/ospf6d/ospf6_intra.c
index d99541ebaddfa8c28ae9a1423546b551921d3430..7898b109050ee506cf5706f7fc0de091b4630590 100644 (file)
--- a/ospf6d/ospf6_intra.c
+++ b/ospf6d/ospf6_intra.c
@@ -1323,6 +1323,8 @@ static void ospf6_intra_prefix_update_route_origin(struct ospf6_route *oa_route)
        g_route = ospf6_route_lookup(&oa_route->prefix,
                                     ospf6->route_table);
 
+       assert(g_route);
+
        for (ospf6_route_lock(g_route); g_route &&
             ospf6_route_is_prefix(&oa_route->prefix, g_route);
             g_route = nroute) {

diff --git a/ospfd/ospf_api.c b/ospfd/ospf_api.c
index 8369dde82215ad67cbe64511a14f7cda623554c3..b1175a2f68cc3f7e080fff2048a7a3945b439c49 100644 (file)
--- a/ospfd/ospf_api.c
+++ b/ospfd/ospf_api.c
@@ -510,17 +510,18 @@ struct msg *new_msg_originate_request(uint32_t seqnum, struct in_addr ifaddr,
        struct msg_originate_request *omsg;
        unsigned int omsglen;
        char buf[OSPF_API_MAX_MSG_SIZE];
+       size_t off_data = offsetof(struct msg_originate_request, data);
+       size_t data_maxs = sizeof(buf) - off_data;
+       struct lsa_header *omsg_data = (struct lsa_header *)&buf[off_data];
 
        omsg = (struct msg_originate_request *)buf;
        omsg->ifaddr = ifaddr;
        omsg->area_id = area_id;
 
        omsglen = ntohs(data->length);
-       if (omsglen
-           > sizeof(buf) - offsetof(struct msg_originate_request, data))
-               omsglen = sizeof(buf)
-                         - offsetof(struct msg_originate_request, data);
-       memcpy(&omsg->data, data, omsglen);
+       if (omsglen > data_maxs)
+               omsglen = data_maxs;
+       memcpy(omsg_data, data, omsglen);
        omsglen += sizeof(struct msg_originate_request)
                   - sizeof(struct lsa_header);
 
@@ -630,6 +631,9 @@ struct msg *new_msg_lsa_change_notify(uint8_t msgtype, uint32_t seqnum,
        uint8_t buf[OSPF_API_MAX_MSG_SIZE];
        struct msg_lsa_change_notify *nmsg;
        unsigned int len;
+       size_t off_data = offsetof(struct msg_lsa_change_notify, data);
+       size_t data_maxs = sizeof(buf) - off_data;
+       struct lsa_header *nmsg_data = (struct lsa_header *)&buf[off_data];
 
        assert(data);
 
@@ -640,10 +644,9 @@ struct msg *new_msg_lsa_change_notify(uint8_t msgtype, uint32_t seqnum,
        memset(&nmsg->pad, 0, sizeof(nmsg->pad));
 
        len = ntohs(data->length);
-       if (len > sizeof(buf) - offsetof(struct msg_lsa_change_notify, data))
-               len = sizeof(buf)
-                     - offsetof(struct msg_lsa_change_notify, data);
-       memcpy(&nmsg->data, data, len);
+       if (len > data_maxs)
+               len = data_maxs;
+       memcpy(nmsg_data, data, len);
        len += sizeof(struct msg_lsa_change_notify) - sizeof(struct lsa_header);
 
        return msg_new(msgtype, nmsg, seqnum, len);

diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c
index 7d748419faceb8ae6fe6ee2f37d58c3b4d54a264..ddf9133ed96ff30ae1dc3c50e6af8990904a18e3 100644 (file)
--- a/ospfd/ospf_vty.c
+++ b/ospfd/ospf_vty.c
@@ -8166,6 +8166,11 @@ DEFUN (ospf_redistribute_instance_source,
 
        source = proto_redistnum(AFI_IP, argv[idx_ospf_table]->text);
 
+       if (source < 0) {
+               vty_out(vty, "Unknown instance redistribution\n");
+               return CMD_WARNING_CONFIG_FAILED;
+       }
+
        instance = strtoul(argv[idx_number]->arg, NULL, 10);
 
        if ((source == ZEBRA_ROUTE_OSPF) && !ospf->instance) {

diff --git a/ospfd/ospf_zebra.c b/ospfd/ospf_zebra.c
index 141ece9c7a74b228c6f075dff134d233d8943742..0a7776cced054c5dac2eb563530f0e3c80c46b7f 100644 (file)
--- a/ospfd/ospf_zebra.c
+++ b/ospfd/ospf_zebra.c
@@ -670,6 +670,16 @@ int ospf_redistribute_set(struct ospf *ospf, int type, unsigned short instance,
        struct ospf_redist *red;
 
        red = ospf_redist_lookup(ospf, type, instance);
+
+       if (red == NULL) {
+               zlog_err(
+                        "Redistribute[%s][%d]: Lookup failed  Type[%d] , Metric[%d]",
+                        ospf_redist_string(type), instance,
+                        metric_type(ospf, type, instance),
+                        metric_value(ospf, type, instance));
+               return CMD_WARNING_CONFIG_FAILED;
+       }
+
        if (ospf_is_type_redistributed(ospf, type, instance)) {
                if (mtype != red->dmetric.type) {
                        red->dmetric.type = mtype;

diff --git a/sharpd/sharp_main.c b/sharpd/sharp_main.c
index a478b416bf7f687700db89c2493ec7f2b3cb6fef..22a19da0b3257114b300c4c5d70e904f2b5eb260 100644 (file)
--- a/sharpd/sharp_main.c
+++ b/sharpd/sharp_main.c
@@ -51,7 +51,6 @@ uint32_t installed_routes = 0;
 uint32_t removed_routes = 0;
 
 zebra_capabilities_t _caps_p[] = {
-       ZCAP_NET_RAW, ZCAP_BIND, ZCAP_NET_ADMIN,
 };
 
 struct zebra_privs_t sharp_privs = {

diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c
index 309493b13e9fc199d717c445868b967fc8185603..66b49800ddbfd58419b2308bfebd46e727619424 100644 (file)
--- a/vtysh/vtysh.c
+++ b/vtysh/vtysh.c
@@ -2415,10 +2415,11 @@ DEFUNSH(VTYSH_ALL, vtysh_log_syslog, vtysh_log_syslog_cmd,
 }
 
 DEFUNSH(VTYSH_ALL, no_vtysh_log_syslog, no_vtysh_log_syslog_cmd,
-       "no log syslog [LEVEL]", NO_STR
+       "no log syslog [<emergencies|alerts|critical|errors|warnings|notifications|informational|debugging>]",
+       NO_STR
        "Logging control\n"
        "Cancel logging to syslog\n"
-       "Logging level\n")
+       LOG_LEVEL_DESC)
 {
        return CMD_SUCCESS;
 }
@@ -2634,8 +2635,13 @@ static void backup_config_file(const char *fbackup)
        strcat(integrate_sav, CONF_BACKUP_EXT);
 
        /* Move current configuration file to backup config file. */
-       unlink(integrate_sav);
-       rename(fbackup, integrate_sav);
+       if (unlink(integrate_sav) != 0) {
+               vty_out(vty, "Warning: %s unlink failed\n", integrate_sav);
+       }
+       if (rename(fbackup, integrate_sav) != 0) {
+               vty_out(vty, "Error renaming %s to %s\n", fbackup,
+                       integrate_sav);
+       }
        free(integrate_sav);
 }