From: Karel Zak Date: Fri, 25 Sep 2020 15:23:18 +0000 (+0200) Subject: libmount: improve mnt_split_optstr() performance X-Git-Tag: v2.37-rc1~469 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4973aed09db05f41995851fd0a703cc071012d16;p=thirdparty%2Futil-linux.git 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 --- diff --git a/libmount/src/optstr.c b/libmount/src/optstr.c index b85f392406..829b2849f2 100644 --- 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; }