afsd: fix afsd -help crash

afsd crashes after the usage is displayed with the -help option.

    $ afsd -help
    Usage: ./afsd [-blocks <1024 byte blocks in cache>] [-files <files in cache>]
    ...
    Segmentation fault (core dumped)

The backtrace shows the crash occurs when calling afsconf_Open() with an
invalid pointer argument, even though afsconf_Open() is not even needed
when -help is given.

    (gdb) bt
    #0  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:32
    #1  0x00007ffff726fc36 in *__GI___strdup (s=0x0) at strdup.c:42
    #2  0x0000000000408383 in afsconf_Open (adir=0x0) at cellconfig.c:444
    #3  0x00000000004054d5 in afsd_run () at afsd.c:1926
    #4  0x0000000000407dc5 in main (argc=2, argv=0x7fffffffe348) at afsd_kernel.c:577

afsconf_Open() is called with an uninitialized pointer because commit
d72df5a18e0bb8bbcbf23df3e8591072f0cdb770 changed the libcmd
cmd_Dispatch() to return 0 after displaying the command usage when the
-help option is specified.  (That fix was needed for scripts which use
the -help option to inspect command options with the -help option.)

The afsd_kernel main function then incorrectly calls the afsd_run()
function, even though mainproc() was not called, which sets up the afsd
option variables.  The afsconf_Open() is the first function we call in
afsd_run().

Commit f77c078a291025d593f3170c57b6be5f257fc3e5 split afsd into afsd.c
and afsd_kernel.c to support libuafs (and fuse).  This split the parsing
of the command line arguments and the running of the afsd command into
two functions.  The mainproc(), which originally did both, was split
into two functions; one (still called mainproc) to check the option
values given and setup/auto-tune values, and another (called afsd_run)
to do the actual running of the afsd command. The afsd_parse() function
was introduced as a wrapper around cmd_Dispatch() which "dispatches"
mainproc.

With this fix, take the opportunity to rename mainproc() to the now more
accurately named CheckOptions() and change afsd_parse() to parse the
command line options with cmd_Parse(), instead of abusing
cmd_Dispatch().

Change the main fuction to avoid running afsd_run() when afsd_parse()
returns the CMD_HELP code which indicates the -help option was given.

afsd.fuse splits the command line arguments into afsd recognized options
and fuse options (everything else), so only afsd recognized arguments
are passed to afsd_parse(), via uafs_ParseArgs(). The -help argument is
processed as part of that splitting of arguments, so afsd.fuse never
passes -help as an argument to afsd_parse(). This means we to not need
to check for CMD_HELP as a return value from uafs_ParseArgs().  But
since this is all a bit confusing, at least check the return value in
uafs_ParseArgs().

Change-Id: If510f8dc337e441c19b5e28685e2e818ff57ef5a
Reviewed-on: https://gerrit.openafs.org/12360
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

diff --git a/src/afsd/afsd.c b/src/afsd/afsd.c
index 1478651..dd63963 100644 (file)
--- a/src/afsd/afsd.c
+++ b/src/afsd/afsd.c
@@ -1684,8 +1684,16 @@ rmtsysd_thread(void *rock)
 }
 #endif /* !UKERNEL */
 
-int
-mainproc(struct cmd_syndesc *as, void *arock)
+/**
+ * Check the command line and cacheinfo options.
+ *
+ * @param[in] as  parsed command line arguments
+ *
+ * @note Invokes the shutdown syscall and exits with 0 when
+ *       -shutdown is given.
+ */
+static int
+CheckOptions(struct cmd_syndesc *as)
 {
     afs_int32 code;            /*Result of fork() */
 #ifdef AFS_SUN5_ENV
@@ -2520,7 +2528,7 @@ afsd_init(void)
 {
     struct cmd_syndesc *ts;
 
-    ts = cmd_CreateSyntax(NULL, mainproc, NULL, 0, "start AFS");
+    ts = cmd_CreateSyntax(NULL, NULL, NULL, 0, "start AFS");
 
     /* 0 - 10 */
     cmd_AddParmAtOffset(ts, OPT_blocks, "-blocks", CMD_SINGLE,
@@ -2618,10 +2626,24 @@ afsd_init(void)
                        "Set inode number calculation method");
 }
 
+/**
+ * Parse and check the command line options.
+ *
+ * @note The -shutdown command is handled in CheckOptions().
+ */
 int
 afsd_parse(int argc, char **argv)
 {
-    return cmd_Dispatch(argc, argv);
+    struct cmd_syndesc *ts = NULL;
+    int code;
+
+    code = cmd_Parse(argc, argv, &ts);
+    if (code) {
+       return code;
+    }
+    code = CheckOptions(ts);
+    cmd_FreeOptions(&ts);
+    return code;
 }
 
 /**

diff --git a/src/afsd/afsd_fuse.c b/src/afsd/afsd_fuse.c
index 7b3968b..6c03476 100644 (file)
--- a/src/afsd/afsd_fuse.c
+++ b/src/afsd/afsd_fuse.c
@@ -608,7 +608,18 @@ main(int argc, char **argv)
 
        split_args(&args);
 
-       uafs_ParseArgs(afsd_args.argc, afsd_args.argv);
+       code = uafs_ParseArgs(afsd_args.argc, afsd_args.argv);
+       if (code != 0) {
+           /*
+            * split_args() failed to populate afds_args correctly.
+            * We do not not bother to check for CMD_HELP here, since
+            * split_args() exits when -help is given.
+            */
+           fprintf(stderr,
+                   "afsd.fuse: Could not parse command line options; code %d\n",
+                   code);
+           return 1;
+       }
 
        /* pass "-- /mount/dir" to fuse to specify dir to mount; "--" is
         * just to make sure fuse doesn't interpret the mount dir as a flag

diff --git a/src/afsd/afsd_kernel.c b/src/afsd/afsd_kernel.c
index f3cfab6..5799f3d 100644 (file)
--- a/src/afsd/afsd_kernel.c
+++ b/src/afsd/afsd_kernel.c
@@ -570,7 +570,10 @@ main(int argc, char **argv)
     afsd_init();
 
     code = afsd_parse(argc, argv);
-    if (code) {
+    if (code == CMD_HELP) {
+       return 0; /* Displaying help is not an error. */
+    }
+    if (code != 0) {
        return -1;
     }