libmount: improve mnt_split_optstr() performance

This function is used by fstab (etc.) parser to split VFS, FS and
userspace options to separate lists. Unfortunately, the current
implementation reallocates the final string always when append a new
option to the string.

The new implementation pre-allocate memory for the final string
according to source string length (1/2 of the original string). This
dramatically reduces realloc() calls.

For example oss-fuzz (./test_mount_fuzz) uses 800K input string, old
version needs 28s to parse the string, new version 500ms.

Addresses: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=23861
Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/libmount/src/optstr.c b/libmount/src/optstr.c
index b85f3924062d3d31293acf43da425e40e09008cc..829b2849f29710a13947b605c142d1b69217ac5a 100644 (file)
--- a/libmount/src/optstr.c
+++ b/libmount/src/optstr.c
@@ -27,6 +27,7 @@
 
 #include "strutils.h"
 #include "mountP.h"
+#include "buffer.h"
 
 /*
  * Option location
@@ -174,47 +175,24 @@ int mnt_optstr_next_option(char **optstr, char **name, size_t *namesz,
        return mnt_optstr_parse_next(optstr, name, namesz, value, valuesz);
 }
 
-static int __mnt_optstr_append_option(char **optstr,
-                       const char *name, size_t nsz,
-                       const char *value, size_t vsz)
+static int __buffer_append_option(struct ul_buffer *buf,
+                       const char *name, size_t namesz,
+                       const char *val, size_t valsz)
 {
-       char *p;
-       size_t sz, osz;
-
-       assert(name);
-       assert(*name);
-       assert(nsz);
-       assert(optstr);
-
-       osz = *optstr ? strlen(*optstr) : 0;
-
-       sz = osz + nsz + 1;             /* 1: '\0' */
-       if (osz)
-               sz++;                   /* ',' options separator */
-       if (value)
-               sz += vsz + 1;          /* 1: '=' */
-
-       p = realloc(*optstr, sz);
-       if (!p)
-               return -ENOMEM;
-       *optstr = p;
-
-       if (osz) {
-               p += osz;
-               *p++ = ',';
-       }
-
-       memcpy(p, name, nsz);
-       p += nsz;
+       int rc = 0;
 
-       if (value) {
-               *p++ = '=';
-               memcpy(p, value, vsz);
-               p += vsz;
+       if (!ul_buffer_is_empty(buf))
+               rc = ul_buffer_append_data(buf, ",", 1);
+       if (!rc)
+               rc = ul_buffer_append_data(buf, name, namesz);
+       if (val && !rc) {
+               /* we need to append '=' is value is empty string, see
+                * 727c689908c5e68c92aa1dd65e0d3bdb6d91c1e5 */
+               rc = ul_buffer_append_data(buf, "=", 1);
+               if (!rc && valsz)
+                       rc = ul_buffer_append_data(buf, val, valsz);
        }
-       *p = '\0';
-
-       return 0;
+       return rc;
 }
 
 /**
@@ -228,7 +206,9 @@ static int __mnt_optstr_append_option(char **optstr,
  */
 int mnt_optstr_append_option(char **optstr, const char *name, const char *value)
 {
-       size_t vsz, nsz;
+       struct ul_buffer buf = UL_INIT_BUFFER;
+       int rc;
+       size_t nsz, vsz, osz;
 
        if (!optstr)
                return -EINVAL;
@@ -236,11 +216,17 @@ int mnt_optstr_append_option(char **optstr, const char *name, const char *value)
                return 0;
 
        nsz = strlen(name);
+       osz = *optstr ? strlen(*optstr) : 0;
        vsz = value ? strlen(value) : 0;
 
-       return __mnt_optstr_append_option(optstr, name, nsz, value, vsz);
-}
+       ul_buffer_refer_string(&buf, *optstr);
+       ul_buffer_set_chunksize(&buf, osz + nsz + vsz + 3);     /* to call realloc() only once */
+
+       rc = __buffer_append_option(&buf, name, nsz, value, vsz);
 
+       *optstr = ul_buffer_get_data(&buf);
+       return rc;
+}
 /**
  * mnt_optstr_prepend_option:
  * @optstr: option string or NULL, returns a reallocated string
@@ -252,30 +238,30 @@ int mnt_optstr_append_option(char **optstr, const char *name, const char *value)
  */
 int mnt_optstr_prepend_option(char **optstr, const char *name, const char *value)
 {
-       int rc = 0;
-       char *tmp;
+       struct ul_buffer buf = UL_INIT_BUFFER;
+       size_t nsz, vsz, osz;
+       int rc;
 
        if (!optstr)
                return -EINVAL;
        if (!name || !*name)
                return 0;
 
-       tmp = *optstr;
-       *optstr = NULL;
+       nsz = strlen(name);
+       osz = *optstr ? strlen(*optstr) : 0;
+       vsz = value ? strlen(value) : 0;
 
-       rc = mnt_optstr_append_option(optstr, name, value);
-       if (!rc && tmp && *tmp)
-               rc = mnt_optstr_append_option(optstr, tmp, NULL);
-       if (!rc) {
-               free(tmp);
-               return 0;
-       }
+       ul_buffer_set_chunksize(&buf, osz + nsz + vsz + 3);   /* to call realloc() only once */
 
-       free(*optstr);
-       *optstr = tmp;
+       rc = __buffer_append_option(&buf, name, nsz, value, vsz);
+       if (*optstr && !rc) {
+               rc = ul_buffer_append_data(&buf, ",", 1);
+               if (!rc)
+                       rc = ul_buffer_append_data(&buf, *optstr, osz);
+               free(*optstr);
+       }
 
-       DBG(OPTIONS, ul_debug("failed to prepend '%s[=%s]' to '%s'",
-                               name, value, *optstr));
+       *optstr = ul_buffer_get_data(&buf);
        return rc;
 }
 
@@ -518,9 +504,13 @@ int mnt_optstr_remove_option(char **optstr, const char *name)
 int mnt_split_optstr(const char *optstr, char **user, char **vfs,
                     char **fs, int ignore_user, int ignore_vfs)
 {
+       int rc = 0;
        char *name, *val, *str = (char *) optstr;
-       size_t namesz, valsz;
+       size_t namesz, valsz, chunsz;
        struct libmnt_optmap const *maps[2];
+       struct ul_buffer xvfs = UL_INIT_BUFFER,
+                        xfs = UL_INIT_BUFFER,
+                        xuser = UL_INIT_BUFFER;
 
        if (!optstr)
                return -EINVAL;
@@ -528,15 +518,10 @@ int mnt_split_optstr(const char *optstr, char **user, char **vfs,
        maps[0] = mnt_get_builtin_optmap(MNT_LINUX_MAP);
        maps[1] = mnt_get_builtin_optmap(MNT_USERSPACE_MAP);
 
-       if (vfs)
-               *vfs = NULL;
-       if (fs)
-               *fs = NULL;
-       if (user)
-               *user = NULL;
+       chunsz = strlen(optstr) / 2;
 
        while (!mnt_optstr_next_option(&str, &name, &namesz, &val, &valsz)) {
-               int rc = 0;
+               struct ul_buffer *buf = NULL;
                const struct libmnt_optmap *ent = NULL;
                const struct libmnt_optmap *m =
                         mnt_optmap_get_entry(maps, 2, name, namesz, &ent);
@@ -551,34 +536,40 @@ int mnt_split_optstr(const char *optstr, char **user, char **vfs,
                if (ent && m && m == maps[0] && vfs) {
                        if (ignore_vfs && (ent->mask & ignore_vfs))
                                continue;
-                       rc = __mnt_optstr_append_option(vfs, name, namesz,
-                                                               val, valsz);
+                       if (vfs)
+                               buf = &xvfs;
                } else if (ent && m && m == maps[1] && user) {
                        if (ignore_user && (ent->mask & ignore_user))
                                continue;
-                       rc = __mnt_optstr_append_option(user, name, namesz,
-                                                               val, valsz);
-               } else if (!m && fs)
-                       rc = __mnt_optstr_append_option(fs, name, namesz,
-                                                               val, valsz);
-               if (rc) {
-                       if (vfs) {
-                               free(*vfs);
-                               *vfs = NULL;
-                       }
-                       if (fs) {
-                               free(*fs);
-                               *fs = NULL;
-                       }
-                       if (user) {
-                               free(*user);
-                               *user = NULL;
-                       }
-                       return rc;
+                       if (user)
+                               buf = &xuser;
+               } else if (!m && fs) {
+                       if (fs)
+                               buf = &xfs;
                }
+
+               if (buf) {
+                       if (ul_buffer_is_empty(buf))
+                               ul_buffer_set_chunksize(buf, chunsz);
+                       rc = __buffer_append_option(buf, name, namesz, val, valsz);
+               }
+               if (rc)
+                       break;
        }
 
-       return 0;
+       if (vfs)
+               *vfs  = rc ? NULL : ul_buffer_get_data(&xvfs);
+       if (fs)
+               *fs   = rc ? NULL : ul_buffer_get_data(&xfs);
+       if (user)
+               *user = rc ? NULL : ul_buffer_get_data(&xuser);
+       if (rc) {
+               ul_buffer_free_data(&xvfs);
+               ul_buffer_free_data(&xfs);
+               ul_buffer_free_data(&xuser);
+       }
+
+       return rc;
 }
 
 /**
@@ -603,17 +594,19 @@ int mnt_optstr_get_options(const char *optstr, char **subset,
                            const struct libmnt_optmap *map, int ignore)
 {
        struct libmnt_optmap const *maps[1];
+       struct ul_buffer buf = UL_INIT_BUFFER;
        char *name, *val, *str = (char *) optstr;
        size_t namesz, valsz;
+       int rc = 0;
 
        if (!optstr || !subset)
                return -EINVAL;
 
        maps[0] = map;
-       *subset = NULL;
 
-       while(!mnt_optstr_next_option(&str, &name, &namesz, &val, &valsz)) {
-               int rc = 0;
+       ul_buffer_set_chunksize(&buf, strlen(optstr)/2);
+
+       while (!mnt_optstr_next_option(&str, &name, &namesz, &val, &valsz)) {
                const struct libmnt_optmap *ent;
 
                mnt_optmap_get_entry(maps, 1, name, namesz, &ent);
@@ -628,14 +621,15 @@ int mnt_optstr_get_options(const char *optstr, char **subset,
                if (valsz && mnt_optmap_entry_novalue(ent))
                        continue;
 
-               rc = __mnt_optstr_append_option(subset, name, namesz, val, valsz);
-               if (rc) {
-                       free(*subset);
-                       return rc;
-               }
+               rc = __buffer_append_option(&buf, name, namesz, val, valsz);
+               if (rc)
+                       break;
        }
 
-       return 0;
+       *subset  = rc ? NULL : ul_buffer_get_data(&buf);
+       if (rc)
+               ul_buffer_free_data(&buf);
+       return rc;
 }