* [PATCH] mdopen: use safe functions in create_mddev
@ 2021-09-21 7:52 Mariusz Tkaczyk
2021-11-02 16:51 ` Tkaczyk, Mariusz
2021-11-02 16:51 ` Jes Sorensen
0 siblings, 2 replies; 6+ messages in thread
From: Mariusz Tkaczyk @ 2021-09-21 7:52 UTC (permalink / raw)
To: jes; +Cc: linux-raid
To avoid buffer overflows, add sizes and use safe functions.
Buffers are now limited to NAME_MAX. Potentially, it may
cause regression in non-standard usecases.
Adapt description to kernel-doc standard.
Add verification for name and dev to ensure that at least one of them
is set.
Remove redundant chosen update after verification. It is set already.
Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
Assemble.c | 2 +-
Build.c | 2 +-
Create.c | 3 +-
Incremental.c | 10 ++--
mdadm.h | 5 +-
mdopen.c | 132 ++++++++++++++++++++++++++++++++------------------
6 files changed, 96 insertions(+), 58 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index 0df46244..77bfc6f7 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1572,7 +1572,7 @@ try_again:
name = strchr(name, ':')+1;
mdfd = create_mddev(mddev, name, ident->autof, trustworthy,
- chosen_name, 0);
+ chosen_name, sizeof(chosen_name), 0);
}
if (mdfd < 0) {
st->ss->free_super(st);
diff --git a/Build.c b/Build.c
index 962c2e37..0561beb5 100644
--- a/Build.c
+++ b/Build.c
@@ -97,7 +97,7 @@ int Build(char *mddev, struct mddev_dev *devlist,
/* We need to create the device. It can have no name. */
map_lock(&map);
mdfd = create_mddev(mddev, NULL, c->autof, LOCAL,
- chosen_name, 0);
+ chosen_name, sizeof(chosen_name), 0);
if (mdfd < 0) {
map_unlock(&map);
return 1;
diff --git a/Create.c b/Create.c
index f5d57f8c..12e41b06 100644
--- a/Create.c
+++ b/Create.c
@@ -627,7 +627,8 @@ int Create(struct supertype *st, char *mddev,
/* We need to create the device */
map_lock(&map);
- mdfd = create_mddev(mddev, name, c->autof, LOCAL, chosen_name, 1);
+ mdfd = create_mddev(mddev, name, c->autof, LOCAL, chosen_name,
+ sizeof(chosen_name), 1);
if (mdfd < 0) {
map_unlock(&map);
return 1;
diff --git a/Incremental.c b/Incremental.c
index cd9cc0fc..e849db01 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -297,7 +297,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
/* Couldn't find an existing array, maybe make a new one */
mdfd = create_mddev(match ? match->devname : NULL,
- name_to_use, c->autof, trustworthy, chosen_name, 0);
+ name_to_use, c->autof, trustworthy,
+ chosen_name, sizeof(chosen_name), 0);
if (mdfd < 0)
goto out_unlock;
@@ -1568,10 +1569,9 @@ static int Incremental_container(struct supertype *st, char *devname,
trustworthy = LOCAL;
mdfd = create_mddev(match ? match->devname : NULL,
- ra->name,
- c->autof,
- trustworthy,
- chosen_name, 0);
+ ra->name, c->autof, trustworthy,
+ chosen_name, sizeof(chosen_name),
+ 0);
}
if (only && (!mp || strcmp(mp->devnm, only) != 0))
continue;
diff --git a/mdadm.h b/mdadm.h
index 8f8841d8..fa550e4d 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1599,8 +1599,9 @@ extern char *get_md_name(char *devnm);
extern char DefaultConfFile[];
-extern int create_mddev(char *dev, char *name, int autof, int trustworthy,
- char *chosen, int block_udev);
+extern int create_mddev(const char *dev, const char *name, int autof,
+ int trustworthy, char *chosen, const size_t chosen_size,
+ int block_udev);
/* values for 'trustworthy' */
#define LOCAL 1
#define LOCAL_ANY 10
diff --git a/mdopen.c b/mdopen.c
index 245be537..7ab23dbf 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -25,6 +25,7 @@
#include "mdadm.h"
#include "md_p.h"
#include <ctype.h>
+#include <linux/limits.h>
void make_parts(char *dev, int cnt)
{
@@ -128,7 +129,16 @@ int create_named_array(char *devnm)
return 1;
}
-/*
+/**
+ * create_mddev() - Create new MD device.
+ * @dev: name given by the user, will be used to determine wanted /dev/md/name.
+ * @name: secondary name specifier, used to determine md-device numer.
+ * @autof: obsolete.
+ * @trustworthy: trustworthy type.
+ * @chosen: pointer to buffer.
+ * @chosen_size: size of @chosen. Minimal length is %DEV_MD_PATH + %NAME_MAX.
+ * @block_udev: if set udev will be blocked.
+ *
* We need a new md device to assemble/build/create an array.
* 'dev' is a name given us by the user (command line or mdadm.conf)
* It might start with /dev or /dev/md any might end with a digit
@@ -160,10 +170,13 @@ int create_named_array(char *devnm)
* /dev/mdXX in 'chosen'.
*
* When we create devices, we use uid/gid/umask from config file.
+ *
+ * Return: O_EXCL file descriptor or negative integer.
+ *
+ * Null terminated name for the volume is returned via *chosen.
*/
-
-int create_mddev(char *dev, char *name, int autof, int trustworthy,
- char *chosen, int block_udev)
+int create_mddev(const char *dev, const char *name, int autof, int trustworthy,
+ char *chosen, const size_t chosen_size, int block_udev)
{
int mdfd;
struct stat stb;
@@ -171,16 +184,24 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
int use_mdp = -1;
struct createinfo *ci = conf_get_create_info();
int parts;
+ const size_t cname_size = NAME_MAX;
char *cname;
- char devname[37];
- char devnm[32];
- char cbuf[400];
+ char devname[NAME_MAX + 5];
+ char devnm[NAME_MAX] = "\0";
+ static const char dev_md_path[] = "/dev/md/";
if (!use_udev())
block_udev = 0;
- if (chosen == NULL)
- chosen = cbuf;
+ if (chosen == NULL) {
+ dprintf("Chosen buffer cannot be NULL.\n");
+ return -1;
+ }
+
+ if (!dev && !name) {
+ dprintf("Both dev and name cannot be NULL.\n");
+ return -1;
+ }
if (autof == 0)
autof = ci->autof;
@@ -188,35 +209,48 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
parts = autof >> 3;
autof &= 7;
- strcpy(chosen, "/dev/md/");
- cname = chosen + strlen(chosen);
+ if (chosen_size <= strlen(dev_md_path) + cname_size) {
+ dprintf("Chosen buffer is to small.\n");
+ return -1;
+ }
+
+ strncpy(chosen, dev_md_path, chosen_size);
+ cname = chosen + strlen(dev_md_path);
if (dev) {
- if (strncmp(dev, "/dev/md/", 8) == 0) {
- strcpy(cname, dev+8);
- } else if (strncmp(dev, "/dev/", 5) == 0) {
- char *e = dev + strlen(dev);
+ if (strncmp(dev, dev_md_path, 8) == 0)
+ strncpy(cname, dev+8, cname_size);
+ else if (strncmp(dev, dev_md_path, 5) == 0) {
+ const char *e = dev + 5 + strnlen(dev + 5, NAME_MAX);
+
while (e > dev && isdigit(e[-1]))
e--;
if (e[0])
num = strtoul(e, NULL, 10);
- strcpy(cname, dev+5);
+ strncpy(cname, dev + 5, cname_size);
cname[e-(dev+5)] = 0;
+
/* name *must* be mdXX or md_dXX in this context */
if (num < 0 ||
- (strcmp(cname, "md") != 0 && strcmp(cname, "md_d") != 0)) {
+ (strncmp(cname, "md", 2) != 0 &&
+ strncmp(cname, "md_d", 4) != 0)) {
pr_err("%s is an invalid name for an md device. Try /dev/md/%s\n",
dev, dev+5);
return -1;
}
- if (strcmp(cname, "md") == 0)
+ if (strncmp(cname, "md", 2) == 0)
use_mdp = 0;
else
use_mdp = 1;
/* recreate name: /dev/md/0 or /dev/md/d0 */
- sprintf(cname, "%s%d", use_mdp?"d":"", num);
+ snprintf(cname, cname_size, "%s%d",
+ use_mdp ? "d" : "", num);
} else
- strcpy(cname, dev);
+ strncpy(cname, dev, cname_size);
+ /*
+ * Force null termination for long names.
+ */
+ cname[cname_size - 1] = '\0';
/* 'cname' must not contain a slash, and may not be
* empty.
@@ -271,8 +305,9 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
* 'md' or '/dev/md', use that for num
* if it is not already in use */
char *ep;
- char *n2 = name;
- if (strncmp(n2, "/dev/", 5) == 0)
+ const char *n2 = name;
+
+ if (strncmp(n2, dev_md_path, 5) == 0)
n2 += 5;
if (strncmp(n2, "md", 2) == 0)
n2 += 2;
@@ -282,7 +317,8 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
if (ep == n2 || *ep)
num = -1;
else {
- sprintf(devnm, "md%s%d", use_mdp ? "_d":"", num);
+ snprintf(devnm, sizeof(devnm), "md%s%d",
+ use_mdp ? "_d" : "", num);
if (mddev_busy(devnm))
num = -1;
}
@@ -290,16 +326,17 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
if (cname[0] == 0 && name) {
/* Need to find a name if we can
- * We don't completely trust 'name'. Truncate to
- * reasonable length and remove '/'
+ * We don't completely trust 'name'.
*/
char *cp;
struct map_ent *map = NULL;
int conflict = 1;
int unum = 0;
int cnlen;
- strncpy(cname, name, 200);
- cname[200] = 0;
+
+ strncpy(cname, name, cname_size);
+ cname[cname_size - 1] = '\0';
+
for (cp = cname; *cp ; cp++)
switch (*cp) {
case '/':
@@ -317,15 +354,17 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
if (map_by_name(&map, cname) == NULL)
conflict = 0;
}
- cnlen = strlen(cname);
+ cnlen = strnlen(cname, cname_size);
while (conflict) {
if (trustworthy == METADATA && !isdigit(cname[cnlen-1]))
- sprintf(cname+cnlen, "%d", unum);
+ snprintf(cname + cnlen, cname_size - cnlen,
+ "%d", unum);
else
/* add _%d to FOREIGN array that don't
* a 'host:' prefix
*/
- sprintf(cname+cnlen, "_%d", unum);
+ snprintf(cname + cnlen, cname_size - cnlen,
+ "_%d", unum);
unum++;
if (map_by_name(&map, cname) == NULL)
conflict = 0;
@@ -333,8 +372,8 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
}
devnm[0] = 0;
- if (num < 0 && cname && ci->names) {
- sprintf(devnm, "md_%s", cname);
+ if (num < 0 && ci->names) {
+ snprintf(devnm, sizeof(devnm), "md_%s", cname);
if (block_udev)
udev_block(devnm);
if (!create_named_array(devnm)) {
@@ -343,7 +382,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
}
}
if (num >= 0) {
- sprintf(devnm, "md%d", num);
+ snprintf(devnm, sizeof(devnm), "md%d", num);
if (block_udev)
udev_block(devnm);
if (!create_named_array(devnm)) {
@@ -359,12 +398,13 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
pr_err("No avail md devices - aborting\n");
return -1;
}
- strcpy(devnm, _devnm);
+ strncpy(devnm, _devnm, sizeof(devnm) - 1);
} else {
- sprintf(devnm, "%s%d", use_mdp?"md_d":"md", num);
+ snprintf(devnm, sizeof(devnm), "%s%d",
+ use_mdp ? "md_d" : "md", num);
if (mddev_busy(devnm)) {
pr_err("%s is already in use.\n",
- dev);
+ devnm);
return -1;
}
}
@@ -372,12 +412,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
udev_block(devnm);
}
- sprintf(devname, "/dev/%s", devnm);
-
- if (dev && dev[0] == '/')
- strcpy(chosen, dev);
- else if (cname[0] == 0)
- strcpy(chosen, devname);
+ snprintf(devname, sizeof(devname), "/dev/%s", devnm);
/* We have a device number and name.
* If we cannot detect udev, we need to make
@@ -410,7 +445,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
if (use_mdp == 1)
make_parts(devname, parts);
- if (strcmp(chosen, devname) != 0) {
+ if (strncmp(chosen, devname, sizeof(devname)) != 0) {
if (mkdir("/dev/md",0700) == 0) {
if (chown("/dev/md", ci->uid, ci->gid))
perror("chown /dev/md");
@@ -418,27 +453,28 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
perror("chmod /dev/md");
}
- if (dev && strcmp(chosen, dev) == 0)
+ if (dev && strncmp(chosen, dev, chosen_size) == 0)
/* We know we are allowed to use this name */
unlink(chosen);
if (lstat(chosen, &stb) == 0) {
- char buf[300];
+ char buf[PATH_MAX];
ssize_t link_len = readlink(chosen, buf, sizeof(buf)-1);
if (link_len >= 0)
buf[link_len] = '\0';
if ((stb.st_mode & S_IFMT) != S_IFLNK ||
link_len < 0 ||
- strcmp(buf, devname) != 0) {
+ strncmp(buf, devname, sizeof(devname)) != 0) {
pr_err("%s exists - ignoring\n",
chosen);
- strcpy(chosen, devname);
+ strncpy(chosen, devname, chosen_size);
}
} else if (symlink(devname, chosen) != 0)
pr_err("failed to create %s: %s\n",
chosen, strerror(errno));
- if (use_mdp && strcmp(chosen, devname) != 0)
+ if (use_mdp &&
+ strncmp(chosen, devname, sizeof(devname)) != 0)
make_parts(chosen, parts);
}
}
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mdopen: use safe functions in create_mddev
2021-09-21 7:52 [PATCH] mdopen: use safe functions in create_mddev Mariusz Tkaczyk
@ 2021-11-02 16:51 ` Tkaczyk, Mariusz
2021-11-02 17:12 ` Jes Sorensen
2021-11-02 16:51 ` Jes Sorensen
1 sibling, 1 reply; 6+ messages in thread
From: Tkaczyk, Mariusz @ 2021-11-02 16:51 UTC (permalink / raw)
To: jes; +Cc: linux-raid
Hi Jes,
If you considering it as risky, let me know.
It can wait to 4.2.
Thanks,
Mariusz
On 21.09.2021 09:52, Mariusz Tkaczyk wrote:
> To avoid buffer overflows, add sizes and use safe functions.
> Buffers are now limited to NAME_MAX. Potentially, it may
> cause regression in non-standard usecases.
> Adapt description to kernel-doc standard.
> Add verification for name and dev to ensure that at least one of them
> is set.
> Remove redundant chosen update after verification. It is set already.
>
> Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
> ---
> Assemble.c | 2 +-
> Build.c | 2 +-
> Create.c | 3 +-
> Incremental.c | 10 ++--
> mdadm.h | 5 +-
> mdopen.c | 132 ++++++++++++++++++++++++++++++++------------------
> 6 files changed, 96 insertions(+), 58 deletions(-)
>
> diff --git a/Assemble.c b/Assemble.c
> index 0df46244..77bfc6f7 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1572,7 +1572,7 @@ try_again:
> name = strchr(name, ':')+1;
>
> mdfd = create_mddev(mddev, name, ident->autof, trustworthy,
> - chosen_name, 0);
> + chosen_name, sizeof(chosen_name), 0);
> }
> if (mdfd < 0) {
> st->ss->free_super(st);
> diff --git a/Build.c b/Build.c
> index 962c2e37..0561beb5 100644
> --- a/Build.c
> +++ b/Build.c
> @@ -97,7 +97,7 @@ int Build(char *mddev, struct mddev_dev *devlist,
> /* We need to create the device. It can have no name. */
> map_lock(&map);
> mdfd = create_mddev(mddev, NULL, c->autof, LOCAL,
> - chosen_name, 0);
> + chosen_name, sizeof(chosen_name), 0);
> if (mdfd < 0) {
> map_unlock(&map);
> return 1;
> diff --git a/Create.c b/Create.c
> index f5d57f8c..12e41b06 100644
> --- a/Create.c
> +++ b/Create.c
> @@ -627,7 +627,8 @@ int Create(struct supertype *st, char *mddev,
>
> /* We need to create the device */
> map_lock(&map);
> - mdfd = create_mddev(mddev, name, c->autof, LOCAL, chosen_name, 1);
> + mdfd = create_mddev(mddev, name, c->autof, LOCAL, chosen_name,
> + sizeof(chosen_name), 1);
> if (mdfd < 0) {
> map_unlock(&map);
> return 1;
> diff --git a/Incremental.c b/Incremental.c
> index cd9cc0fc..e849db01 100644
> --- a/Incremental.c
> +++ b/Incremental.c
> @@ -297,7 +297,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
>
> /* Couldn't find an existing array, maybe make a new one */
> mdfd = create_mddev(match ? match->devname : NULL,
> - name_to_use, c->autof, trustworthy, chosen_name, 0);
> + name_to_use, c->autof, trustworthy,
> + chosen_name, sizeof(chosen_name), 0);
>
> if (mdfd < 0)
> goto out_unlock;
> @@ -1568,10 +1569,9 @@ static int Incremental_container(struct supertype *st, char *devname,
> trustworthy = LOCAL;
>
> mdfd = create_mddev(match ? match->devname : NULL,
> - ra->name,
> - c->autof,
> - trustworthy,
> - chosen_name, 0);
> + ra->name, c->autof, trustworthy,
> + chosen_name, sizeof(chosen_name),
> + 0);
> }
> if (only && (!mp || strcmp(mp->devnm, only) != 0))
> continue;
> diff --git a/mdadm.h b/mdadm.h
> index 8f8841d8..fa550e4d 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -1599,8 +1599,9 @@ extern char *get_md_name(char *devnm);
>
> extern char DefaultConfFile[];
>
> -extern int create_mddev(char *dev, char *name, int autof, int trustworthy,
> - char *chosen, int block_udev);
> +extern int create_mddev(const char *dev, const char *name, int autof,
> + int trustworthy, char *chosen, const size_t chosen_size,
> + int block_udev);
> /* values for 'trustworthy' */
> #define LOCAL 1
> #define LOCAL_ANY 10
> diff --git a/mdopen.c b/mdopen.c
> index 245be537..7ab23dbf 100644
> --- a/mdopen.c
> +++ b/mdopen.c
> @@ -25,6 +25,7 @@
> #include "mdadm.h"
> #include "md_p.h"
> #include <ctype.h>
> +#include <linux/limits.h>
>
> void make_parts(char *dev, int cnt)
> {
> @@ -128,7 +129,16 @@ int create_named_array(char *devnm)
> return 1;
> }
>
> -/*
> +/**
> + * create_mddev() - Create new MD device.
> + * @dev: name given by the user, will be used to determine wanted /dev/md/name.
> + * @name: secondary name specifier, used to determine md-device numer.
> + * @autof: obsolete.
> + * @trustworthy: trustworthy type.
> + * @chosen: pointer to buffer.
> + * @chosen_size: size of @chosen. Minimal length is %DEV_MD_PATH + %NAME_MAX.
> + * @block_udev: if set udev will be blocked.
> + *
> * We need a new md device to assemble/build/create an array.
> * 'dev' is a name given us by the user (command line or mdadm.conf)
> * It might start with /dev or /dev/md any might end with a digit
> @@ -160,10 +170,13 @@ int create_named_array(char *devnm)
> * /dev/mdXX in 'chosen'.
> *
> * When we create devices, we use uid/gid/umask from config file.
> + *
> + * Return: O_EXCL file descriptor or negative integer.
> + *
> + * Null terminated name for the volume is returned via *chosen.
> */
> -
> -int create_mddev(char *dev, char *name, int autof, int trustworthy,
> - char *chosen, int block_udev)
> +int create_mddev(const char *dev, const char *name, int autof, int trustworthy,
> + char *chosen, const size_t chosen_size, int block_udev)
> {
> int mdfd;
> struct stat stb;
> @@ -171,16 +184,24 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> int use_mdp = -1;
> struct createinfo *ci = conf_get_create_info();
> int parts;
> + const size_t cname_size = NAME_MAX;
> char *cname;
> - char devname[37];
> - char devnm[32];
> - char cbuf[400];
> + char devname[NAME_MAX + 5];
> + char devnm[NAME_MAX] = "\0";
> + static const char dev_md_path[] = "/dev/md/";
>
> if (!use_udev())
> block_udev = 0;
>
> - if (chosen == NULL)
> - chosen = cbuf;
> + if (chosen == NULL) {
> + dprintf("Chosen buffer cannot be NULL.\n");
> + return -1;
> + }
> +
> + if (!dev && !name) {
> + dprintf("Both dev and name cannot be NULL.\n");
> + return -1;
> + }
>
> if (autof == 0)
> autof = ci->autof;
> @@ -188,35 +209,48 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> parts = autof >> 3;
> autof &= 7;
>
> - strcpy(chosen, "/dev/md/");
> - cname = chosen + strlen(chosen);
> + if (chosen_size <= strlen(dev_md_path) + cname_size) {
> + dprintf("Chosen buffer is to small.\n");
> + return -1;
> + }
> +
> + strncpy(chosen, dev_md_path, chosen_size);
> + cname = chosen + strlen(dev_md_path);
>
> if (dev) {
> - if (strncmp(dev, "/dev/md/", 8) == 0) {
> - strcpy(cname, dev+8);
> - } else if (strncmp(dev, "/dev/", 5) == 0) {
> - char *e = dev + strlen(dev);
> + if (strncmp(dev, dev_md_path, 8) == 0)
> + strncpy(cname, dev+8, cname_size);
> + else if (strncmp(dev, dev_md_path, 5) == 0) {
> + const char *e = dev + 5 + strnlen(dev + 5, NAME_MAX);
> +
> while (e > dev && isdigit(e[-1]))
> e--;
> if (e[0])
> num = strtoul(e, NULL, 10);
> - strcpy(cname, dev+5);
> + strncpy(cname, dev + 5, cname_size);
> cname[e-(dev+5)] = 0;
> +
> /* name *must* be mdXX or md_dXX in this context */
> if (num < 0 ||
> - (strcmp(cname, "md") != 0 && strcmp(cname, "md_d") != 0)) {
> + (strncmp(cname, "md", 2) != 0 &&
> + strncmp(cname, "md_d", 4) != 0)) {
> pr_err("%s is an invalid name for an md device. Try /dev/md/%s\n",
> dev, dev+5);
> return -1;
> }
> - if (strcmp(cname, "md") == 0)
> + if (strncmp(cname, "md", 2) == 0)
> use_mdp = 0;
> else
> use_mdp = 1;
> /* recreate name: /dev/md/0 or /dev/md/d0 */
> - sprintf(cname, "%s%d", use_mdp?"d":"", num);
> + snprintf(cname, cname_size, "%s%d",
> + use_mdp ? "d" : "", num);
> } else
> - strcpy(cname, dev);
> + strncpy(cname, dev, cname_size);
> + /*
> + * Force null termination for long names.
> + */
> + cname[cname_size - 1] = '\0';
>
> /* 'cname' must not contain a slash, and may not be
> * empty.
> @@ -271,8 +305,9 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> * 'md' or '/dev/md', use that for num
> * if it is not already in use */
> char *ep;
> - char *n2 = name;
> - if (strncmp(n2, "/dev/", 5) == 0)
> + const char *n2 = name;
> +
> + if (strncmp(n2, dev_md_path, 5) == 0)
> n2 += 5;
> if (strncmp(n2, "md", 2) == 0)
> n2 += 2;
> @@ -282,7 +317,8 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> if (ep == n2 || *ep)
> num = -1;
> else {
> - sprintf(devnm, "md%s%d", use_mdp ? "_d":"", num);
> + snprintf(devnm, sizeof(devnm), "md%s%d",
> + use_mdp ? "_d" : "", num);
> if (mddev_busy(devnm))
> num = -1;
> }
> @@ -290,16 +326,17 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
>
> if (cname[0] == 0 && name) {
> /* Need to find a name if we can
> - * We don't completely trust 'name'. Truncate to
> - * reasonable length and remove '/'
> + * We don't completely trust 'name'.
> */
> char *cp;
> struct map_ent *map = NULL;
> int conflict = 1;
> int unum = 0;
> int cnlen;
> - strncpy(cname, name, 200);
> - cname[200] = 0;
> +
> + strncpy(cname, name, cname_size);
> + cname[cname_size - 1] = '\0';
> +
> for (cp = cname; *cp ; cp++)
> switch (*cp) {
> case '/':
> @@ -317,15 +354,17 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> if (map_by_name(&map, cname) == NULL)
> conflict = 0;
> }
> - cnlen = strlen(cname);
> + cnlen = strnlen(cname, cname_size);
> while (conflict) {
> if (trustworthy == METADATA && !isdigit(cname[cnlen-1]))
> - sprintf(cname+cnlen, "%d", unum);
> + snprintf(cname + cnlen, cname_size - cnlen,
> + "%d", unum);
> else
> /* add _%d to FOREIGN array that don't
> * a 'host:' prefix
> */
> - sprintf(cname+cnlen, "_%d", unum);
> + snprintf(cname + cnlen, cname_size - cnlen,
> + "_%d", unum);
> unum++;
> if (map_by_name(&map, cname) == NULL)
> conflict = 0;
> @@ -333,8 +372,8 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> }
>
> devnm[0] = 0;
> - if (num < 0 && cname && ci->names) {
> - sprintf(devnm, "md_%s", cname);
> + if (num < 0 && ci->names) {
> + snprintf(devnm, sizeof(devnm), "md_%s", cname);
> if (block_udev)
> udev_block(devnm);
> if (!create_named_array(devnm)) {
> @@ -343,7 +382,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> }
> }
> if (num >= 0) {
> - sprintf(devnm, "md%d", num);
> + snprintf(devnm, sizeof(devnm), "md%d", num);
> if (block_udev)
> udev_block(devnm);
> if (!create_named_array(devnm)) {
> @@ -359,12 +398,13 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> pr_err("No avail md devices - aborting\n");
> return -1;
> }
> - strcpy(devnm, _devnm);
> + strncpy(devnm, _devnm, sizeof(devnm) - 1);
> } else {
> - sprintf(devnm, "%s%d", use_mdp?"md_d":"md", num);
> + snprintf(devnm, sizeof(devnm), "%s%d",
> + use_mdp ? "md_d" : "md", num);
> if (mddev_busy(devnm)) {
> pr_err("%s is already in use.\n",
> - dev);
> + devnm);
> return -1;
> }
> }
> @@ -372,12 +412,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> udev_block(devnm);
> }
>
> - sprintf(devname, "/dev/%s", devnm);
> -
> - if (dev && dev[0] == '/')
> - strcpy(chosen, dev);
> - else if (cname[0] == 0)
> - strcpy(chosen, devname);
> + snprintf(devname, sizeof(devname), "/dev/%s", devnm);
>
> /* We have a device number and name.
> * If we cannot detect udev, we need to make
> @@ -410,7 +445,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> if (use_mdp == 1)
> make_parts(devname, parts);
>
> - if (strcmp(chosen, devname) != 0) {
> + if (strncmp(chosen, devname, sizeof(devname)) != 0) {
> if (mkdir("/dev/md",0700) == 0) {
> if (chown("/dev/md", ci->uid, ci->gid))
> perror("chown /dev/md");
> @@ -418,27 +453,28 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> perror("chmod /dev/md");
> }
>
> - if (dev && strcmp(chosen, dev) == 0)
> + if (dev && strncmp(chosen, dev, chosen_size) == 0)
> /* We know we are allowed to use this name */
> unlink(chosen);
>
> if (lstat(chosen, &stb) == 0) {
> - char buf[300];
> + char buf[PATH_MAX];
> ssize_t link_len = readlink(chosen, buf, sizeof(buf)-1);
> if (link_len >= 0)
> buf[link_len] = '\0';
>
> if ((stb.st_mode & S_IFMT) != S_IFLNK ||
> link_len < 0 ||
> - strcmp(buf, devname) != 0) {
> + strncmp(buf, devname, sizeof(devname)) != 0) {
> pr_err("%s exists - ignoring\n",
> chosen);
> - strcpy(chosen, devname);
> + strncpy(chosen, devname, chosen_size);
> }
> } else if (symlink(devname, chosen) != 0)
> pr_err("failed to create %s: %s\n",
> chosen, strerror(errno));
> - if (use_mdp && strcmp(chosen, devname) != 0)
> + if (use_mdp &&
> + strncmp(chosen, devname, sizeof(devname)) != 0)
> make_parts(chosen, parts);
> }
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mdopen: use safe functions in create_mddev
2021-09-21 7:52 [PATCH] mdopen: use safe functions in create_mddev Mariusz Tkaczyk
2021-11-02 16:51 ` Tkaczyk, Mariusz
@ 2021-11-02 16:51 ` Jes Sorensen
2021-11-03 9:08 ` Tkaczyk, Mariusz
1 sibling, 1 reply; 6+ messages in thread
From: Jes Sorensen @ 2021-11-02 16:51 UTC (permalink / raw)
To: Mariusz Tkaczyk; +Cc: linux-raid
On 9/21/21 3:52 AM, Mariusz Tkaczyk wrote:
> To avoid buffer overflows, add sizes and use safe functions.
> Buffers are now limited to NAME_MAX. Potentially, it may
> cause regression in non-standard usecases.
> Adapt description to kernel-doc standard.
> Add verification for name and dev to ensure that at least one of them
> is set.
> Remove redundant chosen update after verification. It is set already.
>
> Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
> ---
> Assemble.c | 2 +-
> Build.c | 2 +-
> Create.c | 3 +-
> Incremental.c | 10 ++--
> mdadm.h | 5 +-
> mdopen.c | 132 ++++++++++++++++++++++++++++++++------------------
> 6 files changed, 96 insertions(+), 58 deletions(-)
I've been wanting to get back to this one for a while. Sorry it's taken
so long.
The switch to using NAME_MAX has some side effects I am a little
concerned about, however the code is also really tricky to get your head
around (not your fault).
> @@ -160,10 +170,13 @@ int create_named_array(char *devnm)
> * /dev/mdXX in 'chosen'.
> *
> * When we create devices, we use uid/gid/umask from config file.
> + *
> + * Return: O_EXCL file descriptor or negative integer.
> + *
> + * Null terminated name for the volume is returned via *chosen.
> */
> -
> -int create_mddev(char *dev, char *name, int autof, int trustworthy,
> - char *chosen, int block_udev)
> +int create_mddev(const char *dev, const char *name, int autof, int trustworthy,
> + char *chosen, const size_t chosen_size, int block_udev)
> {
> int mdfd;
> struct stat stb;
> @@ -171,16 +184,24 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> int use_mdp = -1;
> struct createinfo *ci = conf_get_create_info();
> int parts;
> + const size_t cname_size = NAME_MAX;
> char *cname;
> - char devname[37];
> - char devnm[32];
> - char cbuf[400];
> + char devname[NAME_MAX + 5];
> + char devnm[NAME_MAX] = "\0";
> + static const char dev_md_path[] = "/dev/md/";
This is quite a lot of additional stack space used going from 32+37 to
512, however reducing the arbitrary 400 bytes size of cbuf is a good thing.
> @@ -188,35 +209,48 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
> parts = autof >> 3;
> autof &= 7;
>
> - strcpy(chosen, "/dev/md/");
> - cname = chosen + strlen(chosen);
> + if (chosen_size <= strlen(dev_md_path) + cname_size) {
> + dprintf("Chosen buffer is to small.\n");
> + return -1;
> + }
cname_size = NAME_MAX = 255
Ie. if something calls create_mddev() with a chosen_size smaller than
263, this check will fail, which seems rather arbitrary. It does look
like we always use a chosen_name[1024] which is silly wasteful, but
there much be a better solution to this. Maybe malloc() and return the
buffer instead of relying on those large stack frames?
> + strncpy(chosen, dev_md_path, chosen_size);
> + cname = chosen + strlen(dev_md_path);
>
> if (dev) {
> - if (strncmp(dev, "/dev/md/", 8) == 0) {
> - strcpy(cname, dev+8);
> - } else if (strncmp(dev, "/dev/", 5) == 0) {
> - char *e = dev + strlen(dev);
> + if (strncmp(dev, dev_md_path, 8) == 0)
> + strncpy(cname, dev+8, cname_size);
sizeof(dev_md_path) instead of the hardcoded 8?
> + else if (strncmp(dev, dev_md_path, 5) == 0) {
> + const char *e = dev + 5 + strnlen(dev + 5, NAME_MAX);
> +> while (e > dev && isdigit(e[-1]))
> e--;> if (e[0])
> num = strtoul(e, NULL, 10);
> - strcpy(cname, dev+5);
> + strncpy(cname, dev + 5, cname_size);
> cname[e-(dev+5)] = 0;
> +
> /* name *must* be mdXX or md_dXX in this context */
> if (num < 0 ||
> - (strcmp(cname, "md") != 0 && strcmp(cname, "md_d") != 0)) {
> + (strncmp(cname, "md", 2) != 0 &&
> + strncmp(cname, "md_d", 4) != 0)) {
> pr_err("%s is an invalid name for an md device. Try /dev/md/%s\n",
> dev, dev+5);
> return -1;
> }
> - if (strcmp(cname, "md") == 0)
> + if (strncmp(cname, "md", 2) == 0)
> use_mdp = 0;
> else
> use_mdp = 1;
> /* recreate name: /dev/md/0 or /dev/md/d0 */
> - sprintf(cname, "%s%d", use_mdp?"d":"", num);
> + snprintf(cname, cname_size, "%s%d",
> + use_mdp ? "d" : "", num);
> } else
> - strcpy(cname, dev);
> + strncpy(cname, dev, cname_size);
> + /*
> + * Force null termination for long names.
> + */
> + cname[cname_size - 1] = '\0';
>
> /* 'cname' must not contain a slash, and may not be
> * empty.
My head started spinning by the time I got to here.
The more I think about it, the more I think we should allocate an
appropriate buffer in here and return that, rather than play all those
bounds checking games.
Thoughts?
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mdopen: use safe functions in create_mddev
2021-11-02 16:51 ` Tkaczyk, Mariusz
@ 2021-11-02 17:12 ` Jes Sorensen
0 siblings, 0 replies; 6+ messages in thread
From: Jes Sorensen @ 2021-11-02 17:12 UTC (permalink / raw)
To: Tkaczyk, Mariusz; +Cc: linux-raid
On 11/2/21 12:51 PM, Tkaczyk, Mariusz wrote:
> Hi Jes,
> If you considering it as risky, let me know.
> It can wait to 4.2.
>
> Thanks,
> Mariusz
Sounds good, I'll ignore it for now.
I'll try and cut -rc3 tomorrow hopefully. If anyone has something really
important, now would be a good time to nag me.
Cheers,
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mdopen: use safe functions in create_mddev
2021-11-02 16:51 ` Jes Sorensen
@ 2021-11-03 9:08 ` Tkaczyk, Mariusz
2021-11-04 1:41 ` Jes Sorensen
0 siblings, 1 reply; 6+ messages in thread
From: Tkaczyk, Mariusz @ 2021-11-03 9:08 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
> The switch to using NAME_MAX has some side effects I am a little
> concerned about, however the code is also really tricky to get your head
> around (not your fault).
>
My first idea was to rewrite it at all but there is more nits
(like --name parameter and config). It is not a task for rc stage.
>> @@ -160,10 +170,13 @@ int create_named_array(char *devnm)
>> * /dev/mdXX in 'chosen'.
>> *
>> * When we create devices, we use uid/gid/umask from config file.
>> + *
>> + * Return: O_EXCL file descriptor or negative integer.
>> + *
>> + * Null terminated name for the volume is returned via *chosen.
>> */
>> -
>> -int create_mddev(char *dev, char *name, int autof, int trustworthy,
>> - char *chosen, int block_udev)
>> +int create_mddev(const char *dev, const char *name, int autof, int trustworthy,
>> + char *chosen, const size_t chosen_size, int block_udev)
>> {
>> int mdfd;
>> struct stat stb;
>> @@ -171,16 +184,24 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
>> int use_mdp = -1;
>> struct createinfo *ci = conf_get_create_info();
>> int parts;
>> + const size_t cname_size = NAME_MAX;
>> char *cname;
>> - char devname[37];
>> - char devnm[32];
>> - char cbuf[400];
>> + char devname[NAME_MAX + 5];
>> + char devnm[NAME_MAX] = "\0";
>> + static const char dev_md_path[] = "/dev/md/";
>
> This is quite a lot of additional stack space used going from 32+37 to
> 512, however reducing the arbitrary 400 bytes size of cbuf is a good thing.
Just wanted to use one size for names, i understand that it can be too big
so if you have other proposal, let me know.
>
>> @@ -188,35 +209,48 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
>> parts = autof >> 3;
>> autof &= 7;
>>
>> - strcpy(chosen, "/dev/md/");
>> - cname = chosen + strlen(chosen);
>> + if (chosen_size <= strlen(dev_md_path) + cname_size) {
>> + dprintf("Chosen buffer is to small.\n");
>> + return -1;
>> + }
>
> cname_size = NAME_MAX = 255
>
> Ie. if something calls create_mddev() with a chosen_size smaller than
> 263, this check will fail, which seems rather arbitrary. It does look
> like we always use a chosen_name[1024] which is silly wasteful, but
> there much be a better solution to this. Maybe malloc() and return the
> buffer instead of relying on those large stack frames?
>
With malloc, I will need to add free() everywhere, I don't think
that it is good option.
I agree that this check can be removed, especially that now it is
always called with size 1024.
>> + strncpy(chosen, dev_md_path, chosen_size);
>> + cname = chosen + strlen(dev_md_path);
>>
>> if (dev) {
>> - if (strncmp(dev, "/dev/md/", 8) == 0) {
>> - strcpy(cname, dev+8);
>> - } else if (strncmp(dev, "/dev/", 5) == 0) {
>> - char *e = dev + strlen(dev);
>> + if (strncmp(dev, dev_md_path, 8) == 0)
>> + strncpy(cname, dev+8, cname_size);
>
> sizeof(dev_md_path) instead of the hardcoded 8?
>
>> + else if (strncmp(dev, dev_md_path, 5) == 0) {
>> + const char *e = dev + 5 + strnlen(dev + 5, NAME_MAX);
>> +> while (e > dev && isdigit(e[-1]))
>> e--;> if (e[0])
>> num = strtoul(e, NULL, 10);
>> - strcpy(cname, dev+5);
>> + strncpy(cname, dev + 5, cname_size);
>> cname[e-(dev+5)] = 0;
>> +
>> /* name *must* be mdXX or md_dXX in this context */
>> if (num < 0 ||
>> - (strcmp(cname, "md") != 0 && strcmp(cname, "md_d") != 0)) {
>> + (strncmp(cname, "md", 2) != 0 &&
>> + strncmp(cname, "md_d", 4) != 0)) {
>> pr_err("%s is an invalid name for an md device. Try /dev/md/%s\n",
>> dev, dev+5);
>> return -1;
>> }
>> - if (strcmp(cname, "md") == 0)
>> + if (strncmp(cname, "md", 2) == 0)
>> use_mdp = 0;
>> else
>> use_mdp = 1;
>> /* recreate name: /dev/md/0 or /dev/md/d0 */
>> - sprintf(cname, "%s%d", use_mdp?"d":"", num);
>> + snprintf(cname, cname_size, "%s%d",
>> + use_mdp ? "d" : "", num);
>> } else
>> - strcpy(cname, dev);
>> + strncpy(cname, dev, cname_size);
>> + /*
>> + * Force null termination for long names.
>> + */
>> + cname[cname_size - 1] = '\0';
>>
>> /* 'cname' must not contain a slash, and may not be
>> * empty.
>
> My head started spinning by the time I got to here.
>
> The more I think about it, the more I think we should allocate an
> appropriate buffer in here and return that, rather than play all those
> bounds checking games.
>
> Thoughts?
>
We need to return mdfd too, so cannot return from here.
First we need to determine how it should work.
The code now is quite unpredictable but it is working for
years. I just tried to fix static code analysis errors for
now. My concerns are here:
#mdadm -CR /dev/mdx --name=test ...
#mdadm -CR name --name=test ...
#mdadm -CR /dev/md/name --name=test ...
We can define volume by *dev and *name (--name=) and it
is not well defined how it should behave. I will need
to start with determining it first and later adapt
other stuff (assemble and incremental).
So, it requires separate discussion and will
be a release blocker.
Thanks,
Mariusz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mdopen: use safe functions in create_mddev
2021-11-03 9:08 ` Tkaczyk, Mariusz
@ 2021-11-04 1:41 ` Jes Sorensen
0 siblings, 0 replies; 6+ messages in thread
From: Jes Sorensen @ 2021-11-04 1:41 UTC (permalink / raw)
To: Tkaczyk, Mariusz; +Cc: linux-raid
On 11/3/21 5:08 AM, Tkaczyk, Mariusz wrote:
>> The switch to using NAME_MAX has some side effects I am a little
>> concerned about, however the code is also really tricky to get your head
>> around (not your fault).
>>
> My first idea was to rewrite it at all but there is more nits
> (like --name parameter and config). It is not a task for rc stage.
Agree lets look at this after 4.2
>>> @@ -188,35 +209,48 @@ int create_mddev(char *dev, char *name, int
>>> autof, int trustworthy,
>>> parts = autof >> 3;
>>> autof &= 7;
>>> - strcpy(chosen, "/dev/md/");
>>> - cname = chosen + strlen(chosen);
>>> + if (chosen_size <= strlen(dev_md_path) + cname_size) {
>>> + dprintf("Chosen buffer is to small.\n");
>>> + return -1;
>>> + }
>>
>> cname_size = NAME_MAX = 255
>>
>> Ie. if something calls create_mddev() with a chosen_size smaller than
>> 263, this check will fail, which seems rather arbitrary. It does look
>> like we always use a chosen_name[1024] which is silly wasteful, but
>> there much be a better solution to this. Maybe malloc() and return the
>> buffer instead of relying on those large stack frames?
>
> With malloc, I will need to add free() everywhere, I don't think
> that it is good option.
> I agree that this check can be removed, especially that now it is
> always called with size 1024.
I'd love to get rid of those 1024 byte arrays too.
>>> + strncpy(chosen, dev_md_path, chosen_size);
>>> + cname = chosen + strlen(dev_md_path);
>>> if (dev) {
>>> - if (strncmp(dev, "/dev/md/", 8) == 0) {
>>> - strcpy(cname, dev+8);
>>> - } else if (strncmp(dev, "/dev/", 5) == 0) {
>>> - char *e = dev + strlen(dev);
>>> + if (strncmp(dev, dev_md_path, 8) == 0)
>>> + strncpy(cname, dev+8, cname_size);
>>
>> sizeof(dev_md_path) instead of the hardcoded 8?
>>
>>> + else if (strncmp(dev, dev_md_path, 5) == 0) {
>>> + const char *e = dev + 5 + strnlen(dev + 5, NAME_MAX);
>>> +> while (e > dev && isdigit(e[-1]))
>>> e--;> if (e[0])
>>> num = strtoul(e, NULL, 10);
>>> - strcpy(cname, dev+5);
>>> + strncpy(cname, dev + 5, cname_size);
>>> cname[e-(dev+5)] = 0;
>>> +
>>> /* name *must* be mdXX or md_dXX in this context */
>>> if (num < 0 ||
>>> - (strcmp(cname, "md") != 0 && strcmp(cname, "md_d")
>>> != 0)) {
>>> + (strncmp(cname, "md", 2) != 0 &&
>>> + strncmp(cname, "md_d", 4) != 0)) {
>>> pr_err("%s is an invalid name for an md device.
>>> Try /dev/md/%s\n",
>>> dev, dev+5);
>>> return -1;
>>> }
>>> - if (strcmp(cname, "md") == 0)
>>> + if (strncmp(cname, "md", 2) == 0)
>>> use_mdp = 0;
>>> else
>>> use_mdp = 1;
>>> /* recreate name: /dev/md/0 or /dev/md/d0 */
>>> - sprintf(cname, "%s%d", use_mdp?"d":"", num);
>>> + snprintf(cname, cname_size, "%s%d",
>>> + use_mdp ? "d" : "", num);
>>> } else
>>> - strcpy(cname, dev);
>>> + strncpy(cname, dev, cname_size);
>>> + /*
>>> + * Force null termination for long names.
>>> + */
>>> + cname[cname_size - 1] = '\0';
>>> /* 'cname' must not contain a slash, and may not be
>>> * empty.
>>
>> My head started spinning by the time I got to here.
>>
>> The more I think about it, the more I think we should allocate an
>> appropriate buffer in here and return that, rather than play all those
>> bounds checking games.
>>
>> Thoughts?
>>
> We need to return mdfd too, so cannot return from here.
We can still return it in a pointer argument.
> First we need to determine how it should work.
> The code now is quite unpredictable but it is working for
> years. I just tried to fix static code analysis errors for
> now. My concerns are here:
> #mdadm -CR /dev/mdx --name=test ...
> #mdadm -CR name --name=test ...
> #mdadm -CR /dev/md/name --name=test ...
>
> We can define volume by *dev and *name (--name=) and it
> is not well defined how it should behave. I will need
> to start with determining it first and later adapt
> other stuff (assemble and incremental).
>
> So, it requires separate discussion and will
> be a release blocker.
Yes this is why reading the code made my head spin :)
Cheers,
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-04 1:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-21 7:52 [PATCH] mdopen: use safe functions in create_mddev Mariusz Tkaczyk
2021-11-02 16:51 ` Tkaczyk, Mariusz
2021-11-02 17:12 ` Jes Sorensen
2021-11-02 16:51 ` Jes Sorensen
2021-11-03 9:08 ` Tkaczyk, Mariusz
2021-11-04 1:41 ` Jes Sorensen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.