From: Thomas Franz <franz@dssgmbh.de>
Date: Mon, 19 Dec 2022 11:21:09 +0000 (+0100)
Subject: Fix org#2665 About memory leak on FreeBSD with extended attributes
X-Git-Tag: Release-13.0.2~47
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=62f437b8bcb9aaf1cd1ef64e907c13a42bc0d803;p=thirdparty%2Fbacula.git

Fix org#2665 About memory leak on FreeBSD with extended attributes

Extended attributes in FreeBSD exists for attrnamespace "user" and
"system". Bacula saves this data serializing extended attributes for every
file.

In the source bxattr_freebsd.c of bacula 13.0.1 the following problems could be
identified:

 - Bug missing data during backup: If a file has extended attributes of type
   "system" but not of type "user" then these are missing in the serializing
   stream for the backup. This is caused by the return statement when no
   extended attribute of type "user" exists.

 - Memory leak when using extattr_namespace_to_string() because this function
   calls strdup() and does never free the memory allocated by strdup(). This
   problem did not exists until Bacula 7.4.4, but the correct handling was
   dropped in Bacula 7.4.5.

 - Memory leak during backup of files having extended attributes of both types
   "user" and "system". In this case the pointer "xlist" is used first for
   "user" and then for "system". The space allocated for "user" remains
   allocated.

 - Memory leak during backup of files having extended attributes of type "user"
   but not of type "system". In this case a return statetement is done for type
   "system" and the allocated memory for "user" cannot be freed in the coding
   after "bail_out:".
---

diff --git a/bacula/src/filed/bxattr_freebsd.c b/bacula/src/filed/bxattr_freebsd.c
index 099f9dfbd..b83331fc5 100644
--- a/bacula/src/filed/bxattr_freebsd.c
+++ b/bacula/src/filed/bxattr_freebsd.c
@@ -76,7 +76,7 @@ BXATTR_FreeBSD::BXATTR_FreeBSD()
 bRC_BXATTR BXATTR_FreeBSD::os_backup_xattr (JCR *jcr, FF_PKT *ff_pkt){
 
    bRC_BXATTR rc;
-   POOLMEM *xlist;
+   POOLMEM *xlist = NULL;
    uint32_t xlen;
    char *name;
    uint32_t name_len;
@@ -84,7 +84,7 @@ bRC_BXATTR BXATTR_FreeBSD::os_backup_xattr (JCR *jcr, FF_PKT *ff_pkt){
    uint32_t value_len;
    POOLMEM *name_gen;
    uint32_t name_gen_len;
-   char * namespace_str;
+   char * namespace_str = NULL;
    int namespace_len;
    bool skip;
    alist *xattr_list = NULL;
@@ -103,12 +103,17 @@ bRC_BXATTR BXATTR_FreeBSD::os_backup_xattr (JCR *jcr, FF_PKT *ff_pkt){
          case bRC_BXATTR_skip:
          case bRC_BXATTR_cont:
             /* no xattr available, so skip rest of it */
-            return bRC_BXATTR_ok;
+            rc = bRC_XACL_ok;
+            continue;
          default:
-            return rc;
+            goto bail_out;
       }
 
-      /* get a string representation of the namespace */
+      /*
+       * Convert the numeric attrnamespace into a string representation and make
+       * a private copy of that string. The extattr_namespace_to_string functions
+       * returns a strdupped string which we need to free.
+       */
       if (extattr_namespace_to_string(os_xattr_namespaces[a], &namespace_str) != 0){
          Mmsg2(jcr->errmsg, _("Failed to convert %d into namespace on file \"%s\"\n"), os_xattr_namespaces[a], jcr->last_fname);
          Dmsg2(100, "Failed to convert %d into namespace on file \"%s\"\n", os_xattr_namespaces[a], jcr->last_fname);
@@ -139,7 +144,7 @@ bRC_BXATTR BXATTR_FreeBSD::os_backup_xattr (JCR *jcr, FF_PKT *ff_pkt){
             case bRC_BXATTR_skip:
                /* no xattr available, so skip rest of it */
                rc = bRC_BXATTR_ok;
-               goto bail_out;
+               continue;
             default:
                /* error / fatal */
                goto bail_out;
@@ -164,6 +169,13 @@ bRC_BXATTR BXATTR_FreeBSD::os_backup_xattr (JCR *jcr, FF_PKT *ff_pkt){
          xattr_list->append(xattr);
          xattr_count++;
       }
+
+      /*
+       * Drop the local copy of the current_attrnamespace.
+       */
+      actuallyfree(namespace_str);
+      namespace_str = NULL;
+
       if (xattr_count > 0){
          /* serialize the stream */
          rc = serialize_xattr_stream(jcr, len, xattr_list);
@@ -178,9 +190,16 @@ bRC_BXATTR BXATTR_FreeBSD::os_backup_xattr (JCR *jcr, FF_PKT *ff_pkt){
       } else {
          rc = bRC_BXATTR_ok;
       }
+      if (xlist != NULL){
+         free_pool_memory(xlist);
+         xlist = NULL;
+      }
    }
 bail_out:
    /* free allocated data */
+   if (namespace_str != NULL) {
+      actuallyfree(namespace_str);
+   }
    if (xattr_list != NULL){
       foreach_alist(xattr, xattr_list){
          if (xattr == NULL){