Hi
Here I'm sending the second version of the patch for allocated error
strings.
In the previous version, there was a bug that if we went down the "can't
change device type" path, we didn't set ti_error and it resulted in random
address being freed and a crash.
Mikulas
From: Mikulas Patocka <mpatocka@xxxxxxxxxx>
Previously, ti->error and ti_error strings were pointing to statically
allocated memory (the .rodata section) and it was not possible to add
parameters to the error strings.
This patch makes possible to allocate error strings dynamically using the
"kasprintf" function, so we can add arbitrary parameters to the strings.
We need to free the error string only if is allocated with kasprintf. So,
we introduce a function "dm_free_error" that tests if the string points to
the module area (and doesn't free the string if it does), then it calls
kfree_const. kfree_const detects if the string points to the kernel
.rodata section and frees the string if it is not in .rodata.
Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
---
drivers/md/dm-ioctl.c | 10 +++++++++-
drivers/md/dm-table.c | 16 ++++++++++------
include/linux/device-mapper.h | 15 ++++++++++++++-
3 files changed, 33 insertions(+), 8 deletions(-)
Index: linux-2.6/drivers/md/dm-ioctl.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-ioctl.c 2022-10-03 15:22:15.000000000 +0200
+++ linux-2.6/drivers/md/dm-ioctl.c 2022-10-03 15:44:04.000000000 +0200
@@ -1465,7 +1465,8 @@ static int table_load(struct file *filp,
DMERR("can't replace immutable target type %s",
immutable_target_type->name);
r = -EINVAL;
- ti_error = "can't replace immutable target";
+ ti_error = kasprintf(GFP_NOIO, "can't replace immutable target type %s",
+ immutable_target_type->name);
goto err_unlock_md_type;
}
@@ -1481,6 +1482,8 @@ static int table_load(struct file *filp,
DMERR("can't change device type (old=%u vs new=%u) after initial table load.",
dm_get_md_type(md), dm_table_get_type(t));
r = -EINVAL;
+ ti_error = kasprintf(GFP_NOIO, "can't change device type (old=%u vs new=%u) after initial table load.",
+ dm_get_md_type(md), dm_table_get_type(t));
goto err_unlock_md_type;
}
@@ -1521,11 +1524,16 @@ err_destroy_table:
err:
dm_put(md);
err0:
+ if (!ti_error)
+ ti_error = "can't allocate error string";
if (param->flags & DM_RETURN_ERROR_FLAG) {
param->flags &= ~DM_RETURN_ERROR_FLAG;
strlcpy(param->name, ti_error, sizeof param->name);
param->error = r;
+ dm_free_error(ti_error);
return 0;
+ } else {
+ dm_free_error(ti_error);
}
return r;
}
Index: linux-2.6/drivers/md/dm-table.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-table.c 2022-10-03 15:22:15.000000000 +0200
+++ linux-2.6/drivers/md/dm-table.c 2022-10-03 15:22:15.000000000 +0200
@@ -656,32 +656,32 @@ int dm_table_add_target(struct dm_table
ti->type = dm_get_target_type(type);
if (!ti->type) {
- ti->error = "unknown target type";
+ ti->error = kasprintf(GFP_NOIO, "unknown target type %s", type);
r = -EINVAL;
goto bad0;
}
if (dm_target_needs_singleton(ti->type)) {
if (t->num_targets) {
- ti->error = "singleton target type must appear alone in table";
+ ti->error = kasprintf(GFP_NOIO, "singleton target type %s must appear alone in table", type);
goto bad1;
}
t->singleton = true;
}
if (dm_target_always_writeable(ti->type) && !(t->mode & FMODE_WRITE)) {
- ti->error = "target type may not be included in a read-only table";
+ ti->error = kasprintf(GFP_NOIO, "target type %s may not be included in a read-only table", type);
goto bad1;
}
if (t->immutable_target_type) {
if (t->immutable_target_type != ti->type) {
- ti->error = "immutable target type cannot be mixed with other target types";
+ ti->error = kasprintf(GFP_NOIO, "immutable target type %s cannot be mixed with other target types", type);
goto bad1;
}
} else if (dm_target_is_immutable(ti->type)) {
if (t->num_targets) {
- ti->error = "immutable target type cannot be mixed with other target types";
+ ti->error = kasprintf(GFP_NOIO, "immutable target type %s cannot be mixed with other target types", type);
goto bad1;
}
t->immutable_target_type = ti->type;
@@ -705,7 +705,7 @@ int dm_table_add_target(struct dm_table
r = dm_split_args(&argc, &argv, params);
if (r) {
- ti->error = "couldn't split parameters";
+ ti->error = kasprintf(GFP_NOIO, "couldn't split parameters for target %s", type);
goto bad1;
}
@@ -728,9 +728,13 @@ int dm_table_add_target(struct dm_table
bad1:
dm_put_target_type(ti->type);
bad0:
+ if (!ti->error)
+ ti->error = "can't allocate error string";
DMERR("%s: %s: %s (%pe)", dm_device_name(t->md), type, ti->error, ERR_PTR(r));
if (ti_error)
*ti_error = ti->error;
+ else
+ dm_free_error(ti->error);
return r;
}
Index: linux-2.6/include/linux/device-mapper.h
===================================================================
--- linux-2.6.orig/include/linux/device-mapper.h 2022-10-03 15:22:15.000000000 +0200
+++ linux-2.6/include/linux/device-mapper.h 2022-10-03 15:45:15.000000000 +0200
@@ -342,7 +342,12 @@ struct dm_target {
/* target specific data */
void *private;
- /* Used to provide an error string from the ctr */
+ /*
+ * Used to provide an error string from the ctr. It may pointe to a
+ * statically allocated string in the kernel's or module's .rodata
+ * section, or to a dynamically allocated string using kmalloc (for
+ * example, using kasprintf). It must not point to vmallocated memory.
+ */
char *error;
/*
@@ -679,4 +684,12 @@ static inline unsigned long to_bytes(sec
return (n << SECTOR_SHIFT);
}
+#include <asm/sections.h>
+
+static inline void dm_free_error(char *ti_error)
+{
+ if (!is_vmalloc_or_module_addr(ti_error))
+ kfree_const(ti_error);
+}
+
#endif /* _LINUX_DEVICE_MAPPER_H */
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
