ping.
On Sun, 13 Sep 2015 14:50:19 +0100
bytevolc...@safe-mail.net wrote:
> Hello,
>
> Although a patch like this was submitted before, this one is more
> "focused". It may be useful for those moving from MFS who use this
> option to pre-populate an MFS mount with a collection of files. This
> will refuse to work if the path specified for -P is not a directory.
> If the copy is unsuccessful, unmount and inform the user.
>
> A few questions on this though:
>
> 1. Is there a better way than executing pax, to copy all the files in
> a directory?
>
> 2. The do_exec() function was sort of copied from sbin/newfs/newfs.c.
> It doesn't feel like the best way of handling this, but some research
> suggests it is better than calling system() due to security issues,
> one that springs to mind is a malformed input directory to the -P
> option. Is there a simple way to sanitise the input for use with
> system()?
>
> 3. I noticed a pattern of functions that have about 2-3 lines of code
> in them. Is it best to keep things that way, or is it best to merge
> them into a single function (eg. copy_dir() merged into the
> mount_tmpfs() function)?
>
> 4. Are variable declarations to be at the beginning of a function
> block, or the beginning of the immediate block? I feel limiting scope
> is a good idea, but would like some feedback on this as I cannot see
> anything in style(9).
>
> 5. Is mount_tmpfs.h really necessary, given nothing in the tree seems
> to use anything related to the mount_tmpfs() function which is
> internal to the mount_tmpfs binary?
>
>
> ---------
>
> Index: sbin/mount_tmpfs/mount_tmpfs.8
> ===================================================================
> RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
> retrieving revision 1.4
> diff -u -p -r1.4 mount_tmpfs.8
> --- sbin/mount_tmpfs/mount_tmpfs.8 16 Nov 2014 02:22:10
> -0000 1.4 +++ sbin/mount_tmpfs/mount_tmpfs.8 13 Sep
> 2015 13:03:34 -0000 @@ -41,6 +41,7 @@
> .Op Fl m Ar mode
> .Op Fl n Ar nodes
> .Op Fl o Ar options
> +.Op Fl P Ar directory
> .Op Fl s Ar size
> .Op Fl u Ar user
> .Ar tmpfs
> @@ -80,6 +81,8 @@ flag followed by a comma-separated strin
> See the
> .Xr mount 8
> man page for possible options and their meanings.
> +.It Fl P Ar directory
> +Populate the created tmpfs file system with the contents of the
> directory. .It Fl s Ar size
> Specifies the total file system size in bytes.
> If zero is given (the default), the available amount of memory
> (including @@ -136,6 +139,10 @@ and
> .Ox 5.5 .
> .Sh CAVEATS
> The update of mount options (through mount -u) is currently not
> supported. +The
> +.Fl P
> +option will produce an error if the mount is read-only, or if the
> files +cannot be copied from the specified template directory.
> .Sh BUGS
> File system meta-data is not pageable.
> If there is not enough main memory to hold this information, the
> system may Index: sbin/mount_tmpfs/mount_tmpfs.c
> ===================================================================
> RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 mount_tmpfs.c
> --- sbin/mount_tmpfs/mount_tmpfs.c 16 Jan 2015 06:39:59
> -0000 1.5 +++ sbin/mount_tmpfs/mount_tmpfs.c 13 Sep
> 2015 13:03:34 -0000 @@ -34,6 +34,7 @@
> #include <sys/types.h>
> #include <sys/mount.h>
> #include <sys/stat.h>
> +#include <sys/wait.h>
>
> #include <ctype.h>
> #include <err.h>
> @@ -41,6 +42,7 @@
> #include <grp.h>
> #include <mntopts.h>
> #include <pwd.h>
> +#include <signal.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -68,12 +70,15 @@ static int a_num(const char *, const cha
> static mode_t a_mask(const char *);
> static void pathadj(const char *, char *);
>
> +static int do_exec(const char *, const char *, char *const[]);
> +static int copy_dir(char *, char *);
> +
> /*
> ---------------------------------------------------------------------
> */ void
> mount_tmpfs_parseargs(int argc, char *argv[],
> struct tmpfs_args *args, int *mntflags,
> - char *canon_dev, char *canon_dir)
> + char *canon_dev, char *canon_dir, char *pop_dir)
> {
> int gidset, modeset, uidset; /* Ought to be 'bool'. */
> int ch;
> @@ -95,7 +100,7 @@ mount_tmpfs_parseargs(int argc, char *ar
> modeset = 0; mode = 0;
>
> optind = optreset = 1;
> - while ((ch = getopt(argc, argv, "g:m:n:o:s:u:")) != -1 ) {
> + while ((ch = getopt(argc, argv, "P:g:m:n:o:s:u:")) != -1 ) {
> switch (ch) {
> case 'g':
> gid = a_gid(optarg);
> @@ -131,6 +136,10 @@ mount_tmpfs_parseargs(int argc, char *ar
> uidset = 1;
> break;
>
> + case 'P':
> + strlcpy(pop_dir, optarg, PATH_MAX);
> + break;
> +
> case '?':
> default:
> usage();
> @@ -161,7 +170,8 @@ usage(void)
> extern char *__progname;
> (void)fprintf(stderr,
> "usage: %s [-g group] [-m mode] [-n nodes] [-o options]
> [-s size]\n"
> - " [-u user] tmpfs mount_point\n", __progname);
> + " [-P directory] [-u user] tmpfs
> mount_point\n",
> + __progname);
> exit(1);
> }
>
> @@ -172,14 +182,33 @@ mount_tmpfs(int argc, char *argv[])
> {
> struct tmpfs_args args;
> char canon_dev[PATH_MAX], canon_dir[PATH_MAX];
> + char pop_dir[PATH_MAX] = {0};
> int mntflags;
> + int ret;
> + struct stat st;
>
> mount_tmpfs_parseargs(argc, argv, &args, &mntflags,
> - canon_dev, canon_dir);
> + canon_dev, canon_dir, pop_dir);
> +
> + if(pop_dir[0] != '\0') {
> + if (stat(pop_dir, &st) != 0)
> + err(1, "cannot stat %s", pop_dir);
> + if (!S_ISDIR(st.st_mode))
> + errx(1, "%s: not a directory", pop_dir);
> + }
>
> if (mount(MOUNT_TMPFS, canon_dir, mntflags, &args) == -1)
> err(EXIT_FAILURE, "tmpfs on %s", canon_dir);
>
> + if (pop_dir[0] != '\0') {
> + ret = copy_dir(pop_dir, canon_dir);
> + if (ret != 0) {
> + if (unmount(canon_dir, 0) != 0)
> + warn("unmount %s", canon_dir);
> + errx(1, "copy %s to %s failed", pop_dir,
> canon_dir);
> + }
> + }
> +
> return EXIT_SUCCESS;
> }
>
> @@ -245,4 +274,55 @@ pathadj(const char *input, char *adjuste
> warnx("\"%s\" is a non-resolved or relative path.",
> input); warnx("using \"%s\" instead.", adjusted);
> }
> +}
> +
> +static int
> +do_exec(const char *dir, const char *cmd, char *const argv[])
> +{
> + pid_t pid;
> + int ret, status;
> + sig_t intsave, quitsave;
> +
> + switch (pid = fork()) {
> + case -1:
> + err(1, "fork");
> + case 0:
> + if (dir != NULL && chdir(dir) != 0)
> + err(1, "chdir");
> + if (execv(cmd, argv) != 0)
> + err(1, "%s", cmd);
> + break;
> + default:
> + intsave = signal(SIGINT, SIG_IGN);
> + quitsave = signal(SIGQUIT, SIG_IGN);
> + for (;;) {
> + ret = waitpid(pid, &status, 0);
> + if (ret == -1)
> + err(11, "waitpid");
> + if (WIFEXITED(status)) {
> + status = WEXITSTATUS(status);
> + if (status != 0)
> + warnx("%s: exited", cmd);
> + break;
> + } else if (WIFSIGNALED(status)) {
> + warnx("%s: %s", cmd,
> + strsignal(WTERMSIG(status)));
> + status = 1;
> + break;
> + }
> + }
> + signal(SIGINT, intsave);
> + signal(SIGQUIT, quitsave);
> + return (status);
> + }
> + /* NOTREACHED */
> + return (-1);
> +}
> +
> +static int
> +copy_dir(char *src, char *dst)
> +{
> + char *const argv[] = { "pax", "-rw", "-pe", ".", dst,
> NULL } ; +
> + return do_exec(src, "/bin/pax", argv);
> }
> Index: sbin/mount_tmpfs/mount_tmpfs.h
> ===================================================================
> RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.h,v
> retrieving revision 1.2
> diff -u -p -r1.2 mount_tmpfs.h
> --- sbin/mount_tmpfs/mount_tmpfs.h 3 Jun 2013 10:37:02
> -0000 1.2 +++ sbin/mount_tmpfs/mount_tmpfs.h 13 Sep
> 2015 13:03:34 -0000 @@ -31,6 +31,6 @@
>
> int mount_tmpfs(int, char **);
> void mount_tmpfs_parseargs(int, char **, struct tmpfs_args *,
> int *,
> - char *, char *);
> + char *, char *, char *);
>
> #endif /* _SBIN_MOUNT_TMPFS_MOUNT_TMPFS_H_ */
