Path and name length should not be placed in constant
size buffer but in allocated memory.

Use also PATH_MAX macro from limits.h instead of internal
library macro. Handle overflows of snprintf in related funcitons.

Signed-off-by: Krzysztof Opasiak <k.opas...@samsung.com>
---
 include/usbg/usbg.h |    1 +
 src/usbg.c          |   97 +++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 71 insertions(+), 27 deletions(-)

diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 354ba38..f19b1b2 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -192,6 +192,7 @@ typedef enum  {
        USBG_ERROR_NO_DEV = -7,
        USBG_ERROR_BUSY = -8,
        USBG_ERROR_NOT_SUPPORTED = -9,
+       USBG_ERROR_PATH_TOO_LONG = -10,
        USBG_ERROR_OTHER_ERROR = -99
 } usbg_error;
 
diff --git a/src/usbg.c b/src/usbg.c
index 12ef3f3..50b30e1 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -25,6 +25,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <limits.h>
 
 #define STRINGS_DIR "strings"
 #define CONFIGS_DIR "configs"
@@ -60,8 +61,8 @@ struct usbg_config
        TAILQ_HEAD(bhead, usbg_binding) bindings;
        usbg_gadget *parent;
 
-       char name[USBG_MAX_NAME_LENGTH];
-       char path[USBG_MAX_PATH_LENGTH];
+       char *name;
+       char *path;
 };
 
 struct usbg_function
@@ -206,6 +207,9 @@ const char *usbg_error_name(usbg_error e)
        case USBG_ERROR_NOT_SUPPORTED:
                ret = "USBG_ERROR_NOT_SUPPORTED";
                break;
+       case USBG_ERROR_PATH_TOO_LONG:
+               ret = "USBG_ERROR_PATH_TOO_LONG";
+               break;
        case USBG_ERROR_OTHER_ERROR:
                ret = "USBG_ERROR_OTHER_ERROR";
                break;
@@ -249,6 +253,9 @@ const char *usbg_strerror(usbg_error e)
        case USBG_ERROR_NOT_SUPPORTED:
                ret = "Function not supported";
                break;
+       case USBG_ERROR_PATH_TOO_LONG:
+               ret = "Created path was too long to process it.";
+               break;
        case USBG_ERROR_OTHER_ERROR:
                ret = "Other error";
                break;
@@ -419,6 +426,8 @@ static void usbg_free_config(usbg_config *c)
                TAILQ_REMOVE(&c->bindings, b, bnode);
                usbg_free_binding(b);
        }
+       free(c->path);
+       free(c->name);
        free(c);
 }
 
@@ -478,6 +487,29 @@ static usbg_gadget *usbg_allocate_gadget(char *path, char 
*name,
        return g;
 }
 
+static usbg_config *usbg_allocate_config(char *path, char *name,
+               usbg_gadget *parent)
+{
+       usbg_config *c;
+
+       c = malloc(sizeof(usbg_config));
+       if (c) {
+               TAILQ_INIT(&c->bindings);
+               c->name = strdup(name);
+               c->path = strdup(path);
+               c->parent = parent;
+
+               if (!(c->name) || !(c->path)) {
+                       free(c->name);
+                       free(c->path);
+                       free(c);
+                       c = NULL;
+               }
+       }
+
+       return c;
+}
+
 static int usbg_parse_function_net_attrs(usbg_function *f,
                usbg_function_attrs *f_attrs)
 {
@@ -686,20 +718,20 @@ static int usbg_parse_configs(char *path, usbg_gadget *g)
        int i, n;
        int ret = USBG_SUCCESS;
        struct dirent **dent;
-       char cpath[USBG_MAX_PATH_LENGTH];
+       char cpath[PATH_MAX];
 
-       sprintf(cpath, "%s/%s/%s", path, g->name, CONFIGS_DIR);
+       n = snprintf(cpath, PATH_MAX, "%s/%s/%s", path, g->name, CONFIGS_DIR);
+       if (n >= PATH_MAX) {
+               ret = USBG_ERROR_PATH_TOO_LONG;
+               goto out;
+       }
 
        n = scandir(cpath, &dent, file_select, alphasort);
        if (n >= 0) {
                for (i = 0; i < n; i++) {
                        if (ret == USBG_SUCCESS) {
-                               c = malloc(sizeof(usbg_config));
+                               c = usbg_allocate_config(cpath, 
dent[i]->d_name, g);
                                if (c) {
-                                       c->parent = g;
-                                       strcpy(c->name, dent[i]->d_name);
-                                       strcpy(c->path, cpath);
-                                       TAILQ_INIT(&c->bindings);
                                        ret = usbg_parse_config_bindings(c);
                                        if (ret == USBG_SUCCESS)
                                                TAILQ_INSERT_TAIL(&g->configs, 
c, cnode);
@@ -716,6 +748,7 @@ static int usbg_parse_configs(char *path, usbg_gadget *g)
                ret = usbg_translate_error(errno);
        }
 
+out:
        return ret;
 }
 
@@ -1374,12 +1407,13 @@ int usbg_create_function(usbg_gadget *g, 
usbg_function_type type,
 int usbg_create_config(usbg_gadget *g, char *name,
                usbg_config_attrs *c_attrs, usbg_config_strs *c_strs, 
usbg_config **c)
 {
-       char cpath[USBG_MAX_PATH_LENGTH];
+       char cpath[PATH_MAX];
        usbg_config *conf;
        int ret = USBG_ERROR_INVALID_PARAM;
+       int n;
 
        if (!g || !c)
-               return ret;
+               goto out;
 
        /**
         * @todo Check for legal configuration name
@@ -1387,19 +1421,28 @@ int usbg_create_config(usbg_gadget *g, char *name,
        conf = usbg_get_config(g, name);
        if (conf) {
                ERROR("duplicate configuration name\n");
-               return USBG_ERROR_EXIST;
+               ret = USBG_ERROR_EXIST;
+               goto out;
        }
 
-       sprintf(cpath, "%s/%s/%s/%s", g->path, g->name, CONFIGS_DIR, name);
+       n = snprintf(cpath, PATH_MAX, "%s/%s/%s", g->path, g->name,
+                       CONFIGS_DIR);
+       if (n >= PATH_MAX) {
+               ret =  USBG_ERROR_PATH_TOO_LONG;
+               goto out;
+       }
 
-       *c = malloc(sizeof(usbg_config));
+       *c = usbg_allocate_config(cpath, name, g);
        conf = *c;
-       if (conf) {
-               TAILQ_INIT(&conf->bindings);
-               strcpy(conf->name, name);
-               sprintf(conf->path, "%s/%s/%s", g->path, g->name, CONFIGS_DIR);
+       if (!conf) {
+               ERRORNO("allocating configuration\n");
+               ret = USBG_ERROR_NO_MEM;
+               goto out;
+       }
 
-               ret = mkdir(cpath, S_IRWXU|S_IRWXG|S_IRWXO);
+       n = snprintf(&(cpath[n]), PATH_MAX, "/%s", name);
+       if (n < PATH_MAX) {
+               ret = mkdir(cpath, S_IRWXU | S_IRWXG | S_IRWXO);
                if (!ret) {
                        ret = USBG_SUCCESS;
                        if (c_attrs)
@@ -1408,20 +1451,20 @@ int usbg_create_config(usbg_gadget *g, char *name,
                        if (ret == USBG_SUCCESS && c_strs)
                                ret = usbg_set_config_string(conf, LANG_US_ENG,
                                                c_strs->configuration);
-
                } else {
                        ret = usbg_translate_error(errno);
                }
-
-               if (ret == USBG_SUCCESS)
-                       INSERT_TAILQ_STRING_ORDER(&g->configs, chead, name, 
conf, cnode);
-               else
-                       usbg_free_config(conf);
        } else {
-               ERRORNO("allocating configuration\n");
-               ret = USBG_ERROR_NO_MEM;
+               ret = USBG_ERROR_PATH_TOO_LONG;
        }
 
+       if (ret == USBG_SUCCESS)
+               INSERT_TAILQ_STRING_ORDER(&g->configs, chead, name,
+                               conf, cnode);
+       else
+               usbg_free_config(conf);
+
+out:
        return ret;
 }
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to