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.opasiak@xxxxxxxxxxx>
---
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
