Hardenize libibverbs initialisation

In order to hardenize my test application, I tried "fuzzing" its input with zzuf[1]. But my application mostly failed to initialize due to unhandled failure cases.

So here're are patches to clean the situation and let's application exit properly.

The patches here are against kernel.org's libibverbs repository[2]

[1] http://caca.zoy.org/wiki/zzuf
[2] http://git.kernel.org/?p=libs/infiniband/libibverbs.git

Yann Droneaud (5):
load_drivers: reset the list head after releasing items
ibverbs_init: reset the list head after releasing items
read_config: don't try to open file beginning with '.'
read_config: ignore directory entry with backup suffix (~)
read_config_file: ignore driver line without driver name

 src/init.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

1.7.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

In some failure cases, load_drivers() could be called multiple times. Each time, it will try to free item from driver_name_list, but it doesn't reset the list head.

Spotted using zzuf.

---
 src/init.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

index 4f0130e..9e235e8 100644
@@ -232,6 +232,8 @@ static void load_drivers(void)
 		free(name->name);
 		free(name);
 	}
+
+	driver_name_list = NULL;
 }
 
 static void read_config_file(const char *path)

1.7.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

In some cases, ibverbs_init() could be called multiple times. After the first time, find_sysfs_devs() will add sysfs dev entry to the invalid sysfs_dev_list head.
If you're really unlucky, the last sysfs_dev->next will point to sysfs_dev, and ibverbs_init() will be stuck in an infinite loop.

Spotted using zzuf.

---
 src/init.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

index 9e235e8..bdc1634 100644
@@ -539,5 +539,7 @@ out:
 		free(sysfs_dev);
 	}
 
+	sysfs_dev_list = NULL;
+
 	return num_devices;
 }

1.7.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Files beginning with a dot are mostly current and parent directories or, by convention, hidden files.

Those path are skipped in find_sysfs_dev().

---
 src/init.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

index bdc1634..46ff24c 100644
@@ -308,6 +308,9 @@ static void read_config(void)
 	while ((dent = readdir(conf_dir))) {
 		struct stat buf;
 
+		if (dent->d_name[0] == '.')
+			continue;
+
 		if (asprintf(&path, "%s/%s", IBV_CONFIG_DIR, dent->d_name) < 0) {
 			fprintf(stderr, PFX "Warning: couldn't read config file %s/%s.\n",
 				IBV_CONFIG_DIR, dent->d_name);

1.7.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Not sure we want to do this. We already skip non-regular files, and I'm not sure we want to exclude certain file names.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi,

For example, while testing libibverbs config parsing, I've used emacs to edit drivers file. And, when emacs open a buffer, it create a lock file with a leading dot.

So running a libibverbs program prints
libibverbs: Warning: couldn't stat config file
'/tmp/ib/etc/libibverbs.d/.#test'.

Here's a strace output:

open("/tmp/ib/etc/libibverbs.d", O_RDONLY|O_NONBLOCK|O_DIRECTORY| O_CLOEXEC) = 3
getdents(3, /* 3 entries */, 32768) = 80
stat("/tmp/ib/etc/libibverbs.d/.#test", 0x7fffe11e8c60) = -1 ENOENT (No such file or directory)
write(2, "libibverbs: Warning: couldn't st"..., 82) = 82 stat("/tmp/ib/etc/libibverbs.d/.", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0
stat("/tmp/ib/etc/libibverbs.d/..", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0
getdents(3, /* 0 entries */, 32768) = 0
close(3) = 0

Skipping that "normally" hidden file seems a good behavior. Same thing for those backup files with ~ suffix.

It's mostly harmless, but code which load dynamic modules makes me a little angry :)

For example, directory libibverbs.d should be owned by root with write limited write permission, likewise for the driver files.

Regards.

Yann Droneaud


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Try to protect libibverbs from hand modified configuration files.

---
 src/init.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

index 46ff24c..dd6fb00 100644
@@ -311,6 +311,9 @@ static void read_config(void)
 		if (dent->d_name[0] == '.')
 			continue;
 
+		if (dent->d_name[0] == '\0' || dent->d_name[strlen(dent->d_name) - 1] == '~')
+			continue;
+
 		if (asprintf(&path, "%s/%s", IBV_CONFIG_DIR, dent->d_name) < 0) {
 			fprintf(stderr, PFX "Warning: couldn't read config file %s/%s.\n",
 				IBV_CONFIG_DIR, dent->d_name);

1.7.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

If there's no driver name, strsep() will set config to NULL and later process of driver name will segfault.

Spotted with zzuf.

---
 src/init.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index dd6fb00..4d3d081 100644
@@ -259,7 +259,7 @@ static void read_config_file(const char *path)
 
 		field = strsep(&config, "\n\t ");
 
-		if (strcmp(field, "driver") == 0) {
+		if (strcmp(field, "driver") == 0 && config != NULL) {
 			struct ibv_driver_name *driver_name;
 
 			config += strspn(config, "\t ");

1.7.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks, applied this one.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html