Am 11.11.2013 19:03, schrieb Tristan Schmelcher:
> From: Tristan Schmelcher <tschmelc...@google.com>
>
> Inferring the mount hierarchy correctly from /proc/mounts is hard when MS_MOVE
> may have been used, and the previous code did it wrongly. This change
> simplifies
> the logic to only require that /dev/shm be _on_ tmpfs (which can be checked
> trivially with statfs) rather than that it be a _mountpoint_ of tmpfs, since
> there isn't a compelling reason to be that strict. We also now check for tmpfs
> on whatever directory we ultimately use so that the user is better informed.
>
> This change also moves the more standard TMPDIR environment variable check
> ahead
> of the others.
>
> Applies to 3.12.
>
> Signed-off-by: Tristan Schmelcher <tschmelc...@google.com>
I like what I see but so far I didn't had to time to review your changes
carefully.
Stay tuned. :)
Thanks,
//richard
> ---
> arch/um/os-Linux/mem.c | 372
> ++++++++++---------------------------------------
> 1 file changed, 75 insertions(+), 297 deletions(-)
>
> diff --git a/arch/um/os-Linux/mem.c b/arch/um/os-Linux/mem.c
> index 3c4af77..897e9ad 100644
> --- a/arch/um/os-Linux/mem.c
> +++ b/arch/um/os-Linux/mem.c
> @@ -12,337 +12,117 @@
> #include <string.h>
> #include <sys/stat.h>
> #include <sys/mman.h>
> -#include <sys/param.h>
> +#include <sys/vfs.h>
> +#include <linux/magic.h>
> #include <init.h>
> #include <os.h>
>
> -/* Modified by which_tmpdir, which is called during early boot */
> -static char *default_tmpdir = "/tmp";
> -
> -/*
> - * Modified when creating the physical memory file and when checking
> - * the tmp filesystem for usability, both happening during early boot.
> - */
> +/* Set by make_tempfile() during early boot. */
> static char *tempdir = NULL;
>
> -static void __init find_tempdir(void)
> +/* Check if dir is on tmpfs. Return 0 if yes, -1 if no or error. */
> +static int __init check_tmpfs(const char *dir)
> {
> - const char *dirs[] = { "TMP", "TEMP", "TMPDIR", NULL };
> - int i;
> - char *dir = NULL;
> -
> - if (tempdir != NULL)
> - /* We've already been called */
> - return;
> - for (i = 0; dirs[i]; i++) {
> - dir = getenv(dirs[i]);
> - if ((dir != NULL) && (*dir != '\0'))
> - break;
> - }
> - if ((dir == NULL) || (*dir == '\0'))
> - dir = default_tmpdir;
> + struct statfs st;
>
> - tempdir = malloc(strlen(dir) + 2);
> - if (tempdir == NULL) {
> - fprintf(stderr, "Failed to malloc tempdir, "
> - "errno = %d\n", errno);
> - return;
> - }
> - strcpy(tempdir, dir);
> - strcat(tempdir, "/");
> -}
> -
> -/*
> - * Remove bytes from the front of the buffer and refill it so that if
> there's a
> - * partial string that we care about, it will be completed, and we can
> recognize
> - * it.
> - */
> -static int pop(int fd, char *buf, size_t size, size_t npop)
> -{
> - ssize_t n;
> - size_t len = strlen(&buf[npop]);
> -
> - memmove(buf, &buf[npop], len + 1);
> - n = read(fd, &buf[len], size - len - 1);
> - if (n < 0)
> - return -errno;
> -
> - buf[len + n] = '\0';
> - return 1;
> -}
> -
> -/*
> - * This will return 1, with the first character in buf being the
> - * character following the next instance of c in the file. This will
> - * read the file as needed. If there's an error, -errno is returned;
> - * if the end of the file is reached, 0 is returned.
> - */
> -static int next(int fd, char *buf, size_t size, char c)
> -{
> - ssize_t n;
> - char *ptr;
> -
> - while ((ptr = strchr(buf, c)) == NULL) {
> - n = read(fd, buf, size - 1);
> - if (n == 0)
> - return 0;
> - else if (n < 0)
> - return -errno;
> -
> - buf[n] = '\0';
> + printf("Checking if %s is on tmpfs...", dir);
> + if (statfs(dir, &st) < 0) {
> + printf("%s\n", strerror(errno));
> + } else if (st.f_type != TMPFS_MAGIC) {
> + printf("no\n");
> + } else {
> + printf("OK\n");
> + return 0;
> }
> -
> - return pop(fd, buf, size, ptr - buf + 1);
> + return -1;
> }
>
> /*
> - * Decode an octal-escaped and space-terminated path of the form used by
> - * /proc/mounts. May be used to decode a path in-place. "out" must be at
> least
> - * as large as the input. The output is always null-terminated. "len" gets
> the
> - * length of the output, excluding the trailing null. Returns 0 if a full
> path
> - * was successfully decoded, otherwise an error.
> + * Choose the tempdir to use. We want something on tmpfs so that our memory
> is
> + * not subject to the host's vm.dirty_ratio. If a tempdir is specified in the
> + * environment, we use that even if it's not on tmpfs, but we warn the user.
> + * Otherwise, we try common tmpfs locations, and if no tmpfs directory is
> found
> + * then we fall back to /tmp.
> */
> -static int decode_path(const char *in, char *out, size_t *len)
> +static char * __init choose_tempdir(void)
> {
> - char *first = out;
> - int c;
> + static const char * const vars[] = {
> + "TMPDIR",
> + "TMP",
> + "TEMP",
> + NULL
> + };
> + static const char fallback_dir[] = "/tmp";
> + static const char * const tmpfs_dirs[] = {
> + "/dev/shm",
> + fallback_dir,
> + NULL
> + };
> int i;
> - int ret = -EINVAL;
> - while (1) {
> - switch (*in) {
> - case '\0':
> - goto out;
> -
> - case ' ':
> - ret = 0;
> - goto out;
> -
> - case '\\':
> - in++;
> - c = 0;
> - for (i = 0; i < 3; i++) {
> - if (*in < '0' || *in > '7')
> - goto out;
> - c = (c << 3) | (*in++ - '0');
> - }
> - *(unsigned char *)out++ = (unsigned char) c;
> - break;
> -
> - default:
> - *out++ = *in++;
> - break;
> + const char *dir;
> +
> + printf("Checking environment variables for a tempdir...");
> + for (i = 0; vars[i]; i++) {
> + dir = getenv(vars[i]);
> + if ((dir != NULL) && (*dir != '\0')) {
> + printf("%s\n", dir);
> + if (check_tmpfs(dir) >= 0)
> + goto done;
> + else
> + goto warn;
> }
> }
> + printf("none found\n");
>
> -out:
> - *out = '\0';
> - *len = out - first;
> - return ret;
> -}
> -
> -/*
> - * Computes the length of s when encoded with three-digit octal escape
> sequences
> - * for the characters in chars.
> - */
> -static size_t octal_encoded_length(const char *s, const char *chars)
> -{
> - size_t len = strlen(s);
> - while ((s = strpbrk(s, chars)) != NULL) {
> - len += 3;
> - s++;
> - }
> -
> - return len;
> -}
> -
> -enum {
> - OUTCOME_NOTHING_MOUNTED,
> - OUTCOME_TMPFS_MOUNT,
> - OUTCOME_NON_TMPFS_MOUNT,
> -};
> -
> -/* Read a line of /proc/mounts data looking for a tmpfs mount at "path". */
> -static int read_mount(int fd, char *buf, size_t bufsize, const char *path,
> - int *outcome)
> -{
> - int found;
> - int match;
> - char *space;
> - size_t len;
> -
> - enum {
> - MATCH_NONE,
> - MATCH_EXACT,
> - MATCH_PARENT,
> - };
> -
> - found = next(fd, buf, bufsize, ' ');
> - if (found != 1)
> - return found;
> -
> - /*
> - * If there's no following space in the buffer, then this path is
> - * truncated, so it can't be the one we're looking for.
> - */
> - space = strchr(buf, ' ');
> - if (space) {
> - match = MATCH_NONE;
> - if (!decode_path(buf, buf, &len)) {
> - if (!strcmp(buf, path))
> - match = MATCH_EXACT;
> - else if (!strncmp(buf, path, len)
> - && (path[len] == '/' || !strcmp(buf, "/")))
> - match = MATCH_PARENT;
> - }
> -
> - found = pop(fd, buf, bufsize, space - buf + 1);
> - if (found != 1)
> - return found;
> -
> - switch (match) {
> - case MATCH_EXACT:
> - if (!strncmp(buf, "tmpfs", strlen("tmpfs")))
> - *outcome = OUTCOME_TMPFS_MOUNT;
> - else
> - *outcome = OUTCOME_NON_TMPFS_MOUNT;
> - break;
> -
> - case MATCH_PARENT:
> - /* This mount obscures any previous ones. */
> - *outcome = OUTCOME_NOTHING_MOUNTED;
> - break;
> - }
> + for (i = 0; tmpfs_dirs[i]; i++) {
> + dir = tmpfs_dirs[i];
> + if (check_tmpfs(dir) >= 0)
> + goto done;
> }
>
> - return next(fd, buf, bufsize, '\n');
> + dir = fallback_dir;
> +warn:
> + printf("Warning: tempdir %s is not on tmpfs\n", dir);
> +done:
> + /* Make a copy since getenv results may not remain valid forever. */
> + return strdup(dir);
> }
>
> -/* which_tmpdir is called only during early boot */
> -static int checked_tmpdir = 0;
> -
> /*
> - * Look for a tmpfs mounted at /dev/shm. I couldn't find a cleaner
> - * way to do this than to parse /proc/mounts. statfs will return the
> - * same filesystem magic number and fs id for both /dev and /dev/shm
> - * when they are both tmpfs, so you can't tell if they are different
> - * filesystems. Also, there seems to be no other way of finding the
> - * mount point of a filesystem from within it.
> - *
> - * If a /dev/shm tmpfs entry is found, then we switch to using it.
> - * Otherwise, we stay with the default /tmp.
> + * Create an unlinked tempfile in a suitable tempdir. template must be the
> + * basename part of the template with a leading '/'.
> */
> -static void which_tmpdir(void)
> +static int __init make_tempfile(const char *template)
> {
> + char *tempname;
> int fd;
> - int found;
> - int outcome;
> - char *path;
> - char *buf;
> - size_t bufsize;
>
> - if (checked_tmpdir)
> - return;
> -
> - checked_tmpdir = 1;
> -
> - printf("Checking for tmpfs mount on /dev/shm...");
> -
> - path = realpath("/dev/shm", NULL);
> - if (!path) {
> - printf("failed to check real path, errno = %d\n", errno);
> - return;
> - }
> - printf("%s...", path);
> -
> - /*
> - * The buffer needs to be able to fit the full octal-escaped path, a
> - * space, and a trailing null in order to successfully decode it.
> - */
> - bufsize = octal_encoded_length(path, " \t\n\\") + 2;
> -
> - if (bufsize < 128)
> - bufsize = 128;
> -
> - buf = malloc(bufsize);
> - if (!buf) {
> - printf("malloc failed, errno = %d\n", errno);
> - goto out;
> - }
> - buf[0] = '\0';
> -
> - fd = open("/proc/mounts", O_RDONLY);
> - if (fd < 0) {
> - printf("failed to open /proc/mounts, errno = %d\n", errno);
> - goto out1;
> - }
> -
> - outcome = OUTCOME_NOTHING_MOUNTED;
> - while (1) {
> - found = read_mount(fd, buf, bufsize, path, &outcome);
> - if (found != 1)
> - break;
> - }
> -
> - if (found < 0) {
> - printf("read returned errno %d\n", -found);
> - } else {
> - switch (outcome) {
> - case OUTCOME_TMPFS_MOUNT:
> - printf("OK\n");
> - default_tmpdir = "/dev/shm";
> - break;
> -
> - case OUTCOME_NON_TMPFS_MOUNT:
> - printf("not tmpfs\n");
> - break;
> -
> - default:
> - printf("nothing mounted on /dev/shm\n");
> - break;
> + if (tempdir == NULL) {
> + tempdir = choose_tempdir();
> + if (tempdir == NULL) {
> + fprintf(stderr, "Failed to choose tempdir: %s\n",
> + strerror(errno));
> + return -1;
> }
> }
>
> - close(fd);
> -out1:
> - free(buf);
> -out:
> - free(path);
> -}
> -
> -static int __init make_tempfile(const char *template, char **out_tempname,
> - int do_unlink)
> -{
> - char *tempname;
> - int fd;
> -
> - which_tmpdir();
> - tempname = malloc(MAXPATHLEN);
> + tempname = malloc(strlen(tempdir) + strlen(template) + 1);
> if (tempname == NULL)
> return -1;
>
> - find_tempdir();
> - if ((tempdir == NULL) || (strlen(tempdir) >= MAXPATHLEN))
> - goto out;
> -
> - if (template[0] != '/')
> - strcpy(tempname, tempdir);
> - else
> - tempname[0] = '\0';
> - strncat(tempname, template, MAXPATHLEN-1-strlen(tempname));
> + strcpy(tempname, tempdir);
> + strcat(tempname, template);
> fd = mkstemp(tempname);
> if (fd < 0) {
> fprintf(stderr, "open - cannot create %s: %s\n", tempname,
> strerror(errno));
> goto out;
> }
> - if (do_unlink && (unlink(tempname) < 0)) {
> + if (unlink(tempname) < 0) {
> perror("unlink");
> goto close;
> }
> - if (out_tempname) {
> - *out_tempname = tempname;
> - } else
> - free(tempname);
> + free(tempname);
> return fd;
> close:
> close(fd);
> @@ -351,14 +131,14 @@ out:
> return -1;
> }
>
> -#define TEMPNAME_TEMPLATE "vm_file-XXXXXX"
> +#define TEMPNAME_TEMPLATE "/vm_file-XXXXXX"
>
> static int __init create_tmp_file(unsigned long long len)
> {
> int fd, err;
> char zero;
>
> - fd = make_tempfile(TEMPNAME_TEMPLATE, NULL, 1);
> + fd = make_tempfile(TEMPNAME_TEMPLATE);
> if (fd < 0)
> exit(1);
>
> @@ -402,7 +182,6 @@ int __init create_mem_file(unsigned long long len)
> return fd;
> }
>
> -
> void __init check_tmpexec(void)
> {
> void *addr;
> @@ -410,14 +189,13 @@ void __init check_tmpexec(void)
>
> addr = mmap(NULL, UM_KERN_PAGE_SIZE,
> PROT_READ | PROT_WRITE | PROT_EXEC, MAP_PRIVATE, fd, 0);
> - printf("Checking PROT_EXEC mmap in %s...",tempdir);
> - fflush(stdout);
> + printf("Checking PROT_EXEC mmap in %s...", tempdir);
> if (addr == MAP_FAILED) {
> err = errno;
> - perror("failed");
> + printf("%s\n", strerror(err));
> close(fd);
> if (err == EPERM)
> - printf("%s must be not mounted noexec\n",tempdir);
> + printf("%s must be not mounted noexec\n", tempdir);
> exit(1);
> }
> printf("OK\n");
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
