Fix race condition between setip and the access to CLIENT::address()

Fix concurrency issue between setAddress() and address()
we need to copy the address in an argument, the variable can be
freed at any time.

diff --git a/bacula/src/dird/backup.c b/bacula/src/dird/backup.c
index 10f301f011a1a49b6f54ab8a1e0baecb93a39746..47888eecc37f2939a2bb48d8bae532b5f13fc74a 100644 (file)
--- a/bacula/src/dird/backup.c
+++ b/bacula/src/dird/backup.c
@@ -356,6 +356,7 @@ bool send_client_addr_to_sd(JCR *jcr)
 {
    int tls_need = BNET_TLS_NONE;
    BSOCK *sd = jcr->store_bsock;
+   POOL_MEM buf;
 
    /* TLS Requirement for the client */
    if (jcr->client->tls_enable) {
@@ -368,7 +369,7 @@ bool send_client_addr_to_sd(JCR *jcr)
    /*
     * Send Client address to the SD
     */
-   sd->fsend(clientaddr, jcr->client->address(), jcr->client->FDport, tls_need);
+   sd->fsend(clientaddr, jcr->client->address(buf.addr()), jcr->client->FDport, tls_need);
    if (!response(jcr, sd, OKclient, "Client", DISPLAY_ERROR)) {
       return false;
    }

diff --git a/bacula/src/dird/dird_conf.c b/bacula/src/dird/dird_conf.c
index 155126860c3c6fc969d52a6fc93ded6bb3c651e0..02fae0babab3fa2514f7cc5165133b1753bd7e7a 100644 (file)
--- a/bacula/src/dird/dird_conf.c
+++ b/bacula/src/dird/dird_conf.c
@@ -123,19 +123,17 @@ void CLIENT::setNumConcurrentJobs(int32_t num)
       num, globals->name);
 }
 
-char *CLIENT::address()
+char *CLIENT::address(POOLMEM *&buf)
 {
-   if (!globals) {
-      return client_address;
-   }
-   if (!globals->SetIPaddress) {
-      return client_address;
+   P(globals_mutex);
+   if (!globals || !globals->SetIPaddress) {
+      pm_strcpy(buf, client_address);
+
+   } else {
+      pm_strcpy(buf, globals->SetIPaddress);
    }
-   /* TODO: Fix concurrency issue between setAddress() and address()
-    * we need to copy the address in an argument, the variable can be
-    * freed at any time.
-    */
-   return globals->SetIPaddress;
+   V(globals_mutex);
+   return buf;
 }
 
 void CLIENT::setAddress(char *addr)
@@ -833,6 +831,7 @@ void dump_resource(int type, RES *ares, void sendit(void *sock, const char *fmt,
    char ed1[100], ed2[100], ed3[100];
    DEVICE *dev;
    UAContext *ua = (UAContext *)sock;
+   POOLMEM *buf;
 
    if (res == NULL) {
       sendit(sock, _("No %s resource defined\n"), res_to_str(type));
@@ -881,10 +880,12 @@ void dump_resource(int type, RES *ares, void sendit(void *sock, const char *fmt,
       if (!acl_access_ok(ua, Client_ACL, res->res_client.name())) {
          break;
       }
+      buf = get_pool_memory(PM_FNAME);
       sendit(sock, _("Client: Name=%s Enabled=%d Address=%s FDport=%d MaxJobs=%u NumJobs=%u\n"),
          res->res_client.name(), res->res_client.is_enabled(),
-         res->res_client.address(), res->res_client.FDport,
+         res->res_client.address(buf), res->res_client.FDport,
          res->res_client.MaxConcurrentJobs, res->res_client.getNumConcurrentJobs());
+      free_pool_memory(buf);
       sendit(sock, _("      JobRetention=%s FileRetention=%s AutoPrune=%d\n"),
          edit_utime(res->res_client.JobRetention, ed1, sizeof(ed1)),
          edit_utime(res->res_client.FileRetention, ed2, sizeof(ed2)),
@@ -2510,6 +2511,7 @@ extern "C" char *job_code_callback_director(JCR *jcr, const char* param, char *b
    if (jcr == NULL) {
       return nothing;
    }
+   ASSERTD(buflen < 255, "buflen must be long enough to hold an ip address");
    switch (param[0]) {
       case 'f':
          if (jcr->fileset) {
@@ -2517,8 +2519,11 @@ extern "C" char *job_code_callback_director(JCR *jcr, const char* param, char *b
          }
          break;
       case 'h':
-         if (jcr->client && jcr->client->address()) {
-            return jcr->client->address();
+         if (jcr->client) {
+            POOL_MEM tmp;
+            jcr->client->address(tmp.addr());
+            bstrncpy(buf, tmp.c_str(), buflen);
+            return buf;
          }
          break;
       case 'p':

diff --git a/bacula/src/dird/dird_conf.h b/bacula/src/dird/dird_conf.h
index 67aca36078ce043c5f55de6a6c316202d48637a0..5174a7a148066d5546b818ddd4172e1ee4884b31 100644 (file)
--- a/bacula/src/dird/dird_conf.h
+++ b/bacula/src/dird/dird_conf.h
@@ -293,7 +293,7 @@ public:
    void create_client_globals();
    int32_t getNumConcurrentJobs();
    void setNumConcurrentJobs(int32_t num);
-   char *address();
+   char *address(POOLMEM *&buf);
    void setAddress(char *addr);
    bool is_enabled();
    void setEnabled(bool val);

diff --git a/bacula/src/dird/fd_cmds.c b/bacula/src/dird/fd_cmds.c
index d3cdb6885957c46cf033c02640a52d56fda9ea89..1b64be4cd4cfe1e4dbf3706dfb3975cb6e159533 100644 (file)
--- a/bacula/src/dird/fd_cmds.c
+++ b/bacula/src/dird/fd_cmds.c
@@ -93,6 +93,7 @@ int connect_to_file_daemon(JCR *jcr, int retry_interval, int max_retry_time,
 
    if (!is_bsock_open(jcr->file_bsock)) {
       char name[MAX_NAME_LENGTH + 100];
+      POOL_MEM buf;
 
       if (!fd) {
          fd = jcr->file_bsock = new_bsock();
@@ -101,8 +102,13 @@ int connect_to_file_daemon(JCR *jcr, int retry_interval, int max_retry_time,
       bstrncat(name, jcr->client->name(), sizeof(name));
 
       fd->set_source_address(director->DIRsrc_addr);
-      if (!fd->connect(jcr,retry_interval,max_retry_time, heart_beat, name, jcr->client->address(),
-           NULL, jcr->client->FDport, verbose)) {
+      if (!fd->connect(jcr,retry_interval,
+                       max_retry_time,
+                       heart_beat, name,
+                       jcr->client->address(buf.addr()),
+                       NULL,
+                       jcr->client->FDport,
+                       verbose)) {
          fd->close();
          jcr->setJobStatus(JS_ErrorTerminated);
          return 0;

diff --git a/bacula/src/dird/snapshot.c b/bacula/src/dird/snapshot.c
index 3dc3b6d10b0fec396a033218251d76a174ab4036..5aa7a769887fd68884066cba080b06ad66edb434 100644 (file)
--- a/bacula/src/dird/snapshot.c
+++ b/bacula/src/dird/snapshot.c
@@ -149,6 +149,7 @@ bool send_snapshot_retention(JCR *jcr, utime_t val)
 /* Called from delete_cmd() in ua_cmd.c */
 int delete_snapshot(UAContext *ua)
 {
+   POOL_MEM     buf;
    POOLMEM     *out;
    SNAPSHOT_DBR snapdbr;
    CLIENT      *client;
@@ -178,7 +179,7 @@ int delete_snapshot(UAContext *ua)
 
    /* Try to connect for 15 seconds */
    ua->send_msg(_("Connecting to Client %s at %s:%d\n"),
-      client->name(), client->address(), client->FDport);
+                client->name(), client->address(buf.addr()), client->FDport);
    if (!connect_to_file_daemon(ua->jcr, 1, 15, 0)) {
       ua->error_msg(_("Failed to connect to Client.\n"));
       ua->jcr->client = NULL;
@@ -210,6 +211,7 @@ int delete_snapshot(UAContext *ua)
  */
 int list_snapshot(UAContext *ua, alist *snap_list)
 {
+   POOL_MEM     tmp;
    SNAPSHOT_DBR snap;
    POOLMEM     *buf;
    CLIENT      *client;
@@ -225,7 +227,8 @@ int list_snapshot(UAContext *ua, alist *snap_list)
 
    /* Try to connect for 15 seconds */
    ua->send_msg(_("Connecting to Client %s at %s:%d\n"),
-      client->name(), client->address(), client->FDport);
+                client->name(), client->address(tmp.addr()), client->FDport);
+
    if (!connect_to_file_daemon(ua->jcr, 1, 15, 0)) {
       ua->error_msg(_("Failed to connect to Client.\n"));
       return 0;
@@ -276,6 +279,7 @@ int prune_snapshot(UAContext *ua)
    CLIENT *client = NULL;
    BSOCK  *fd = NULL;
    POOLMEM *buf = NULL;
+   POOL_MEM tmp;
    SNAPSHOT_DBR snapdbr;
    alist *lst;
    intptr_t id;
@@ -318,7 +322,7 @@ int prune_snapshot(UAContext *ua)
 
             /* Try to connect for 15 seconds */
             ua->send_msg(_("Connecting to Client %s at %s:%d\n"),
-                         client->name(), client->address(), client->FDport);
+                         client->name(), client->address(tmp.addr()), client->FDport);
             if (!connect_to_file_daemon(ua->jcr, 1, 15, 0)) {
                ua->error_msg(_("Failed to connect to Client.\n"));
                free_bsock(ua->jcr->file_bsock);

diff --git a/bacula/src/dird/ua_cmds.c b/bacula/src/dird/ua_cmds.c
index 870cc7691c15065f39f7285f09acc50090627a1d..4ff054295048c93b58f2bc0cfb8dcccba94c2e2b 100644 (file)
--- a/bacula/src/dird/ua_cmds.c
+++ b/bacula/src/dird/ua_cmds.c
@@ -695,6 +695,7 @@ extern char *configfile;
 
 static int setbwlimit_client(UAContext *ua, CLIENT *client, char *Job, int64_t limit)
 {
+   POOL_MEM buf;
    CLIENT *old_client;
    char ed1[50];
    if (!client) {
@@ -708,7 +709,7 @@ static int setbwlimit_client(UAContext *ua, CLIENT *client, char *Job, int64_t l
 
    /* Try to connect for 15 seconds */
    ua->send_msg(_("Connecting to Client %s at %s:%d\n"),
-      client->name(), client->address(), client->FDport);
+                client->name(), client->address(buf.addr()), client->FDport);
    if (!connect_to_file_daemon(ua->jcr, 1, 15, 0)) {
       ua->error_msg(_("Failed to connect to Client.\n"));
       goto bail_out;
@@ -814,8 +815,8 @@ static int setip_cmd(UAContext *ua, const char *cmd)
    }
    /* MA Bug 6 remove ifdef */
    sockaddr_to_ascii(&(ua->UA_sock->client_addr),
-         sizeof(ua->UA_sock->client_addr), addr, sizeof(addr));
-   client->setAddress(bstrdup(addr));
+                     sizeof(ua->UA_sock->client_addr), addr, sizeof(addr));
+   client->setAddress(addr);
    ua->send_msg(_("Client \"%s\" address set to %s\n"),
             client->name(), addr);
 get_out:
@@ -988,6 +989,7 @@ static void do_client_setdebug(UAContext *ua, CLIENT *client,
                int64_t level, int trace, int hangup, int blowup,
                char *options, char *tags)
 {
+   POOL_MEM buf;
    CLIENT *old_client;
    BSOCK *fd;
 
@@ -997,7 +999,8 @@ static void do_client_setdebug(UAContext *ua, CLIENT *client,
    ua->jcr->client = client;
    /* Try to connect for 15 seconds */
    ua->send_msg(_("Connecting to Client %s at %s:%d\n"),
-      client->name(), client->address(), client->FDport);
+                client->name(), client->address(buf.addr()), client->FDport);
+
    if (!connect_to_file_daemon(ua->jcr, 1, 15, 0)) {
       ua->error_msg(_("Failed to connect to Client.\n"));
       ua->jcr->client = old_client;
@@ -1029,6 +1032,7 @@ static void do_all_setdebug(UAContext *ua, int64_t level,
 {
    STORE *store, **unique_store;
    CLIENT *client, **unique_client;
+   POOL_MEM buf1, buf2;
    int i, j, found;
    int64_t t=0;
 
@@ -1086,7 +1090,7 @@ static void do_all_setdebug(UAContext *ua, int64_t level,
    while ((client = (CLIENT *)GetNextRes(R_CLIENT, (RES *)client))) {
       found = 0;
       for (j=0; j<i; j++) {
-         if (strcmp(unique_client[j]->address(), client->address()) == 0 &&
+         if (strcmp(unique_client[j]->address(buf1.addr()), client->address(buf2.addr())) == 0 &&
              unique_client[j]->FDport == client->FDport) {
             found = 1;
             break;
@@ -1094,7 +1098,7 @@ static void do_all_setdebug(UAContext *ua, int64_t level,
       }
       if (!found) {
          unique_client[i++] = client;
-         Dmsg2(140, "Stuffing: %s:%d\n", client->address(), client->FDport);
+         Dmsg2(140, "Stuffing: %s:%d\n", client->address(buf1.addr()), client->FDport);
       }
    }
    UnlockRes();
@@ -1306,6 +1310,7 @@ static int estimate_cmd(UAContext *ua, const char *cmd)
    JOB *job = NULL;
    CLIENT *client = NULL;
    FILESET *fileset = NULL;
+   POOL_MEM buf;
    int listing = 0;
    char since[MAXSTRING];
    JCR *jcr = ua->jcr;
@@ -1447,7 +1452,7 @@ static int estimate_cmd(UAContext *ua, const char *cmd)
    get_level_since_time(ua->jcr, since, sizeof(since));
 
    ua->send_msg(_("Connecting to Client %s at %s:%d\n"),
-      jcr->client->name(), jcr->client->address(), jcr->client->FDport);
+                jcr->client->name(), jcr->client->address(buf.addr()), jcr->client->FDport);
    if (!connect_to_file_daemon(jcr, 1, 15, 0)) {
       ua->error_msg(_("Failed to connect to Client.\n"));
       return 1;

diff --git a/bacula/src/dird/ua_dotcmds.c b/bacula/src/dird/ua_dotcmds.c
index f684e3b094e4e80b6c6771d1c20d703441a31a66..5ff3a91a5e696ba1a21dfe07bef1d92b79cecfa7 100644 (file)
--- a/bacula/src/dird/ua_dotcmds.c
+++ b/bacula/src/dird/ua_dotcmds.c
@@ -196,6 +196,7 @@ bool do_a_dot_command(UAContext *ua)
  */
 static bool dot_ls_cmd(UAContext *ua, const char *cmd)
 {
+   POOL_MEM buf;
    CLIENT *client = NULL;
    char *path = NULL;
    JCR *jcr = ua->jcr;
@@ -235,7 +236,7 @@ static bool dot_ls_cmd(UAContext *ua, const char *cmd)
    init_jcr_job_record(jcr);           // need job
 
    ua->send_msg(_("Connecting to Client %s at %s:%d\n"),
-      jcr->client->name(), jcr->client->address(), jcr->client->FDport);
+                jcr->client->name(), jcr->client->address(buf.addr()), jcr->client->FDport);
 
    if (!connect_to_file_daemon(jcr, 1, 15, 0)) {
       ua->error_msg(_("Failed to connect to Client.\n"));
@@ -1267,13 +1268,13 @@ static void do_storage_cmd(UAContext *ua, STORE *store, const char *cmd)
 static void do_client_cmd(UAContext *ua, CLIENT *client, const char *cmd)
 {
    BSOCK *fd;
-
+   POOL_MEM buf;
    /* Connect to File daemon */
 
    ua->jcr->client = client;
    /* Try to connect for 15 seconds */
    ua->send_msg(_("Connecting to Client %s at %s:%d\n"),
-      client->name(), client->address(), client->FDport);
+                client->name(), client->address(buf.addr()), client->FDport);
    if (!connect_to_file_daemon(ua->jcr, 1, 15, 0)) {
       ua->error_msg(_("Failed to connect to Client.\n"));
       return;
@@ -1659,6 +1660,7 @@ static bool clientscmd(UAContext *ua, const char *cmd)
    bool    found=false;
    alist  *clientlist = NULL;
    TmpClient *elt;
+   POOL_MEM buf;
 
    if ((i = find_arg_with_value(ua, "address")) >= 0) {
       ip = ua->argv[i];
@@ -1673,7 +1675,7 @@ static bool clientscmd(UAContext *ua, const char *cmd)
    foreach_res(client, R_CLIENT) {
       if (acl_access_client_ok(ua, client->name(), JT_BACKUP_RESTORE)) {
          if (ip) {
-            elt = new TmpClient(client->name(), client->address());
+            elt = new TmpClient(client->name(), client->address(buf.addr()));
             clientlist->append(elt);
 
          } else {
@@ -2120,8 +2122,9 @@ static bool defaultscmd(UAContext *ua, const char *cmd)
       }
       CLIENT *client = (CLIENT *)GetResWithName(R_CLIENT, ua->argv[1]);
       if (client) {
+         POOL_MEM buf;
          ua->send_msg("client=%s", client->name());
-         ua->send_msg("address=%s", client->address());
+         ua->send_msg("address=%s", client->address(buf.addr()));
          ua->send_msg("fdport=%d", client->FDport);
          ua->send_msg("file_retention=%s", edit_uint64(client->FileRetention, ed1));
          ua->send_msg("job_retention=%s", edit_uint64(client->JobRetention, ed1));

diff --git a/bacula/src/dird/ua_status.c b/bacula/src/dird/ua_status.c
index 03f55e93168c23fdd6a4235712d7d8b6b84f070b..0aee6ab57ac4b865b505f8e4e81bdb729f22fe30 100644 (file)
--- a/bacula/src/dird/ua_status.c
+++ b/bacula/src/dird/ua_status.c
@@ -123,6 +123,7 @@ static int do_network_status(UAContext *ua)
    char *store_address, ed1[50];
    uint32_t store_port;
    uint64_t nb = 50 * 1024 * 1024;
+   POOL_MEM buf;
 
    int i = find_arg_with_value(ua, "bytes");
    if (i > 0) {
@@ -170,7 +171,7 @@ static int do_network_status(UAContext *ua)
 
    if (!ua->api) {
       ua->send_msg(_("Connecting to Client %s at %s:%d\n"),
-                   client->name(), client->address(), client->FDport);
+                   client->name(), client->address(buf.addr()), client->FDport);
    }
 
    if (!connect_to_file_daemon(jcr, 1, 15, 0)) {
@@ -345,6 +346,7 @@ static void do_all_status(UAContext *ua)
 {
    STORE *store, **unique_store;
    CLIENT *client, **unique_client;
+   POOL_MEM buf1, buf2;
    int i, j;
    bool found;
 
@@ -399,7 +401,7 @@ static void do_all_status(UAContext *ua)
          continue;
       }
       for (j=0; j<i; j++) {
-         if (strcmp(unique_client[j]->address(), client->address()) == 0 &&
+         if (strcmp(unique_client[j]->address(buf1.addr()), client->address(buf2.addr())) == 0 &&
              unique_client[j]->FDport == client->FDport) {
             found = true;
             break;
@@ -407,7 +409,7 @@ static void do_all_status(UAContext *ua)
       }
       if (!found) {
          unique_client[i++] = client;
-         Dmsg2(40, "Stuffing: %s:%d\n", client->address(), client->FDport);
+         Dmsg2(40, "Stuffing: %s:%d\n", client->address(buf1.addr()), client->FDport);
       }
    }
    UnlockRes();
@@ -590,6 +592,7 @@ static void do_storage_status(UAContext *ua, STORE *store, char *cmd)
 static void do_client_status(UAContext *ua, CLIENT *client, char *cmd)
 {
    BSOCK *fd;
+   POOL_MEM buf;
 
    if (!acl_access_client_ok(ua, client->name(), JT_BACKUP_RESTORE)) {
       ua->error_msg(_("No authorization for Client \"%s\"\n"), client->name());
@@ -606,7 +609,7 @@ static void do_client_status(UAContext *ua, CLIENT *client, char *cmd)
 
    /* Try to connect for 15 seconds */
    if (!ua->api) ua->send_msg(_("Connecting to Client %s at %s:%d\n"),
-      client->name(), client->address(), client->FDport);
+                              client->name(), client->address(buf.addr()), client->FDport);
    if (!connect_to_file_daemon(ua->jcr, 1, 15, 0)) {
       ua->send_msg(_("Failed to connect to Client %s.\n====\n"),
          client->name());

diff --git a/bacula/src/lib/util.c b/bacula/src/lib/util.c
index 12b3d35420f11dcc5ba281e7d6210dfc956fc562..2c425aa4cc54e677b3c86c8301c01758c104f8a5 100644 (file)
--- a/bacula/src/lib/util.c
+++ b/bacula/src/lib/util.c
@@ -865,7 +865,7 @@ POOLMEM *edit_job_codes(JCR *jcr, char *omsg, char *imsg, const char *to, job_co
    char *p, *q;
    const char *str;
    char add[50];
-   char name[MAX_NAME_LENGTH];
+   char name[MAX_ESCAPE_NAME_LENGTH];
    int i;
 
    *omsg = 0;
@@ -976,7 +976,7 @@ POOLMEM *edit_job_codes(JCR *jcr, char *omsg, char *imsg, const char *to, job_co
          default:
             str = NULL;
             if (callback != NULL) {
-               str = callback(jcr, p, add, sizeof(add));
+               str = callback(jcr, p, name, sizeof(name));
             }
 
             if (!str) {