bozo: Let the bnode operations allocate output strings

The GetInstanceParm and GetStatus bosserver RPCs return output strings
by first allocating a 256 byte buffer then calling the bnode operations
bnode_GetParm() and bnode_GetString() to fill those buffers.  The RPC
output strings are automatically freed by XDR.

In order to support large output strings in the future, change these
bnode operations to allocate the output strings and to use safe string
functions to duplicate and format strings.

Update the ez, cron, fs, and dafs getstring and getparm operations to
allocate the output string or return an error.

Return BZIO over the wire when a string cannot be allocated, since
ENOMEM is not a portable wire error code.  (Use BZIO to be consistent
existing code added in commit fda2bc874751ca479365dc6389c0eebb41a0bda1
(Allocate pathname buffers dynamically).

Commit ea276e83e3 (OPENAFS-SA-2019-001: Skip server OUT args on
error) is a prerequisite for this change.

Change-Id: Id16184efc95d614846b912177b220c8e87b7a88b
Reviewed-on: https://gerrit.openafs.org/14766
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

diff --git a/src/bozo/bnode.c b/src/bozo/bnode.c
index 0f430ce..e6fef33 100644 (file)
--- a/src/bozo/bnode.c
+++ b/src/bozo/bnode.c
@@ -158,17 +158,15 @@ SaveCore(struct bnode *abnode, struct bnode_proc
 }
 
 int
-bnode_GetString(struct bnode *abnode, char *abuffer,
-               afs_int32 alen)
+bnode_GetString(struct bnode *abnode, char **adesc)
 {
-    return BOP_GETSTRING(abnode, abuffer, alen);
+    return BOP_GETSTRING(abnode, adesc);
 }
 
 int
-bnode_GetParm(struct bnode *abnode, afs_int32 aindex,
-             char *abuffer, afs_int32 alen)
+bnode_GetParm(struct bnode *abnode, afs_int32 aindex, char **aparm)
 {
-    return BOP_GETPARM(abnode, aindex, abuffer, alen);
+    return BOP_GETPARM(abnode, aindex, aparm);
 }
 
 int

diff --git a/src/bozo/bnode_internal.h b/src/bozo/bnode_internal.h
index 0172176..a8d5161 100644 (file)
--- a/src/bozo/bnode_internal.h
+++ b/src/bozo/bnode_internal.h
@@ -12,8 +12,8 @@
 #define        BOP_SETSTAT(bnode, a)   ((*(bnode)->ops->setstat)((bnode),(a)))
 #define        BOP_DELETE(bnode)       ((*(bnode)->ops->delete)((bnode)))
 #define        BOP_PROCEXIT(bnode, a)  ((*(bnode)->ops->procexit)((bnode),(a)))
-#define        BOP_GETSTRING(bnode, a, b)      ((*(bnode)->ops->getstring)((bnode),(a), (b)))
-#define        BOP_GETPARM(bnode, n, b, l)     ((*(bnode)->ops->getparm)((bnode),(n),(b),(l)))
+#define        BOP_GETSTRING(bnode, a) ((*(bnode)->ops->getstring)((bnode),(a)))
+#define        BOP_GETPARM(bnode, n, b)        ((*(bnode)->ops->getparm)((bnode),(n),(b)))
 #define        BOP_RESTARTP(bnode)     ((*(bnode)->ops->restartp)((bnode)))
 #define BOP_HASCORE(bnode)     ((*(bnode)->ops->hascore)((bnode)))
 #define BOP_PROCSTARTED(bnode,p)       ((*(bnode)->ops->procstarted)((bnode),(p)))
@@ -27,9 +27,8 @@ struct bnode_ops {
     int (*setstat) ( struct bnode *, afs_int32 );
     int (*delete) ( struct bnode * );
     int (*procexit) ( struct bnode *, struct bnode_proc * );
-    int (*getstring) ( struct bnode *, char *abuffer, afs_int32 alen );
-    int (*getparm) ( struct bnode *, afs_int32 aindex, char *abuffer,
-                    afs_int32 alen);
+    int (*getstring) ( struct bnode *, char **adesc );
+    int (*getparm) ( struct bnode *, afs_int32 aindex, char **aparm);
     int (*restartp) ( struct bnode *);
     int (*hascore) ( struct bnode *);
     int (*procstarted) ( struct bnode *, struct bnode_proc * );

diff --git a/src/bozo/bosoprocs.c b/src/bozo/bosoprocs.c
index 2b79981..2d30f5e 100644 (file)
--- a/src/bozo/bosoprocs.c
+++ b/src/bozo/bosoprocs.c
@@ -1128,28 +1128,15 @@ SBOZO_GetStatus(struct rx_call *acall, char *ainstance, afs_int32 *astat,
     afs_int32 code;
 
     tb = bnode_FindInstance(ainstance);
-    if (!tb) {
-       code = BZNOENT;
-       goto fail;
-    }
+    if (!tb)
+       return BZNOENT;
 
     bnode_Hold(tb);
     code = bnode_GetStat(tb, astat);
-    if (code) {
-       bnode_Release(tb);
-       goto fail;
-    }
-
-    *astatDescr = malloc(BOZO_BSSIZE);
-    code = bnode_GetString(tb, *astatDescr, BOZO_BSSIZE);
+    if (code == 0)
+       code = bnode_GetString(tb, astatDescr);
     bnode_Release(tb);
-    if (code)
-       (*astatDescr)[0] = 0;   /* null string means no further info */
-    return 0;
 
-  fail:
-    *astatDescr = malloc(1);
-    **astatDescr = 0;
     return code;
 }
 
@@ -1421,24 +1408,26 @@ SBOZO_GetInstanceParm(struct rx_call *acall,
                      char **aparm)
 {
     struct bnode *tb;
-    char *tp;
     afs_int32 code;
 
-    tp = malloc(BOZO_BSSIZE);
-    *aparm = tp;
-    *tp = 0;                   /* null-terminate string in error case */
     tb = bnode_FindInstance(ainstance);
     if (!tb)
        return BZNOENT;
+
     bnode_Hold(tb);
     if (anum == 999) {
-       if (tb->notifier) {
-           memcpy(tp, tb->notifier, strlen(tb->notifier) + 1);
-           code = 0;
-       } else
-           code = BZNOENT;     /* XXXXX */
-    } else
-       code = bnode_GetParm(tb, anum, tp, BOZO_BSSIZE);
+       if (tb->notifier == NULL) {
+           code = BZNOENT;
+       } else {
+           *aparm = strdup(tb->notifier);
+           if (*aparm == NULL)
+               code = BZIO;
+           else
+               code = 0;
+       }
+    } else {
+       code = bnode_GetParm(tb, anum, aparm);
+    }
     bnode_Release(tb);
 
     /* Not Currently Audited */

diff --git a/src/bozo/bosprototypes.h b/src/bozo/bosprototypes.h
index b22ccff..0fbe249 100644 (file)
--- a/src/bozo/bosprototypes.h
+++ b/src/bozo/bosprototypes.h
@@ -14,9 +14,8 @@
 
 /* bnode.c */
 int bnode_CoreName(struct bnode *abnode, char *acoreName, char *abuffer);
-int bnode_GetString(struct bnode *abnode, char *abuffer, afs_int32 alen);
-int bnode_GetParm(struct bnode *abnode, afs_int32 aindex, char *abuffer,
-                 afs_int32 alen);
+int bnode_GetString(struct bnode *abnode, char **adesc);
+int bnode_GetParm(struct bnode *abnode, afs_int32 aindex, char **aparm);
 int bnode_GetStat(struct bnode *abnode, afs_int32 * astatus);
 int bnode_RestartP(struct bnode *abnode);
 int bnode_HasCore(struct bnode *abnode);

diff --git a/src/bozo/bosserver.c b/src/bozo/bosserver.c
index ff608b9..6c439a4 100644 (file)
--- a/src/bozo/bosserver.c
+++ b/src/bozo/bosserver.c
@@ -324,7 +324,6 @@ bzwrite(struct bnode *abnode, void *arock)
 {
     struct bztemp *at = (struct bztemp *)arock;
     int i;
-    char tbuffer[BOZO_BSSIZE];
     afs_int32 code;
 
     if (abnode->notifier)
@@ -334,13 +333,16 @@ bzwrite(struct bnode *abnode, void *arock)
        fprintf(at->file, "bnode %s %s %d\n", abnode->type->name,
                abnode->name, abnode->fileGoal);
     for (i = 0;; i++) {
-       code = bnode_GetParm(abnode, i, tbuffer, BOZO_BSSIZE);
+       char *parm = NULL;
+
+       code = bnode_GetParm(abnode, i, &parm);
        if (code) {
            if (code != BZDOM)
                return code;
            break;
        }
-       fprintf(at->file, "parm %s\n", tbuffer);
+       fprintf(at->file, "parm %s\n", parm);
+       free(parm);
     }
     fprintf(at->file, "end\n");
     return 0;

diff --git a/src/bozo/cronbnodeops.c b/src/bozo/cronbnodeops.c
index c46bf8c..51c1823 100644 (file)
--- a/src/bozo/cronbnodeops.c
+++ b/src/bozo/cronbnodeops.c
@@ -33,8 +33,8 @@ static int cron_getstat(struct bnode *bnode, afs_int32 *status);
 static int cron_setstat(struct bnode *bnode, afs_int32 status);
 static int cron_procstarted(struct bnode *bnode, struct bnode_proc *proc);
 static int cron_procexit(struct bnode *bnode, struct bnode_proc *proc);
-static int cron_getstring(struct bnode *bnode, char *abuffer, afs_int32 alen);
-static int cron_getparm(struct bnode *bnode, afs_int32, char *, afs_int32);
+static int cron_getstring(struct bnode *bnode, char **adesc);
+static int cron_getparm(struct bnode *bnode, afs_int32 aindex, char **aparm);
 
 #define        SDTIME          60      /* time in seconds given to a process to evaporate */
 
@@ -302,28 +302,38 @@ cron_procexit(struct bnode *bn, struct bnode_proc *aproc)
 }
 
 static int
-cron_getstring(struct bnode *bn, char *abuffer, afs_int32 alen)
+cron_getstring(struct bnode *bn, char **adesc)
 {
+    int code;
+    char *desc = NULL;
     struct cronbnode *abnode = (struct cronbnode *)bn;
+
     if (abnode->running)
-       strcpy(abuffer, "running now");
+       code = asprintf(&desc, "running now");
     else if (abnode->when == 0)
-       strcpy(abuffer, "waiting to run once");
+       code = asprintf(&desc, "waiting to run once");
     else
-       sprintf(abuffer, "run next at %s", ktime_DateOf(abnode->when));
+       code = asprintf(&desc, "run next at %s", ktime_DateOf(abnode->when));
+    if (code < 0)
+       return BZIO;
+    *adesc = desc;
     return 0;
 }
 
 static int
-cron_getparm(struct bnode *bn, afs_int32 aindex, char *abuffer,
-            afs_int32 alen)
+cron_getparm(struct bnode *bn, afs_int32 aindex, char **aparm)
 {
     struct cronbnode *abnode = (struct cronbnode *)bn;
+    char *parm;
+
     if (aindex == 0)
-       strcpy(abuffer, abnode->command);
-    else if (aindex == 1) {
-       strcpy(abuffer, abnode->whenString);
-    } else
+       parm = abnode->command;
+    else if (aindex == 1)
+       parm = abnode->whenString;
+    else
        return BZDOM;
+    *aparm = strdup(parm);
+    if (*aparm == NULL)
+       return BZIO;
     return 0;
 }

diff --git a/src/bozo/ezbnodeops.c b/src/bozo/ezbnodeops.c
index f2f1409..57d10a8 100644 (file)
--- a/src/bozo/ezbnodeops.c
+++ b/src/bozo/ezbnodeops.c
@@ -33,8 +33,8 @@ static int ez_timeout(struct bnode *bnode);
 static int ez_getstat(struct bnode *bnode, afs_int32 *status);
 static int ez_setstat(struct bnode *bnode, afs_int32 status);
 static int ez_procexit(struct bnode *bnode, struct bnode_proc *proc);
-static int ez_getstring(struct bnode *bnode, char *abuffer, afs_int32 alen);
-static int ez_getparm(struct bnode *bnode, afs_int32, char *, afs_int32);
+static int ez_getstring(struct bnode *bnode, char **adesc);
+static int ez_getparm(struct bnode *bnode, afs_int32 aindex, char **parm);
 static int ez_procstarted(struct bnode *bnode, struct bnode_proc *proc);
 
 #define        SDTIME          60      /* time in seconds given to a process to evaporate */
@@ -233,20 +233,25 @@ ez_procexit(struct bnode *bn, struct bnode_proc *aproc)
 }
 
 static int
-ez_getstring(struct bnode *abnode, char *abuffer, afs_int32 alen)
+ez_getstring(struct bnode *abnode, char **adesc)
 {
-    return -1;                 /* don't have much to add */
+    *adesc = strdup("");  /* Don't have much to add. */
+    if (*adesc == NULL)
+       return BZIO;
+    return 0;
 }
 
 static int
-ez_getparm(struct bnode *bn, afs_int32 aindex, char *abuffer,
-          afs_int32 alen)
+ez_getparm(struct bnode *bn, afs_int32 aindex, char **aparm)
 {
     struct ezbnode *abnode = (struct ezbnode *) bn;
 
     if (aindex != 0)
        return BZDOM;
 
-    strcpy(abuffer, abnode->command);
+    *aparm = strdup(abnode->command);
+    if (*aparm == NULL)
+       return BZIO;
+
     return 0;
 }

diff --git a/src/bozo/fsbnodeops.c b/src/bozo/fsbnodeops.c
index 0aa55ab..c6bcef9 100644 (file)
--- a/src/bozo/fsbnodeops.c
+++ b/src/bozo/fsbnodeops.c
@@ -100,11 +100,9 @@ static int fs_getstat(struct bnode *abnode, afs_int32 * astatus);
 static int fs_setstat(struct bnode *abnode, afs_int32 astatus);
 static int fs_procstarted(struct bnode *abnode, struct bnode_proc *aproc);
 static int fs_procexit(struct bnode *abnode, struct bnode_proc *aproc);
-static int fs_getstring(struct bnode *abnode, char *abuffer, afs_int32 alen);
-static int fs_getparm(struct bnode *abnode, afs_int32 aindex,
-                     char *abuffer, afs_int32 alen);
-static int dafs_getparm(struct bnode *abnode, afs_int32 aindex,
-                       char *abuffer, afs_int32 alen);
+static int fs_getstring(struct bnode *abnode, char **adesc);
+static int fs_getparm(struct bnode *abnode, afs_int32 aindex, char **aparm);
+static int dafs_getparm(struct bnode *abnode, afs_int32 aindex, char **aparm);
 
 static int SetSalFlag(struct fsbnode *abnode, int aflag);
 static int RestoreSalFlag(struct fsbnode *abnode);
@@ -1064,85 +1062,89 @@ NudgeProcs(struct fsbnode *abnode)
 }
 
 static int
-fs_getstring(struct bnode *bn, char *abuffer, afs_int32 alen)
+fs_getstring(struct bnode *bn, char **adesc)
 {
     struct fsbnode *abnode = (struct fsbnode *)bn;
+    const char *desc;
 
-    if (alen < 40)
-       return -1;
     if (abnode->b.goal == 1) {
        if (abnode->fileRunning) {
            if (abnode->fileSDW)
-               strcpy(abuffer, "file server shutting down");
+               desc = "file server shutting down";
            else if (abnode->scancmd) {
                if (!abnode->volRunning && !abnode->scanRunning)
-                   strcpy(abuffer,
-                          "file server up; volser and scanner down");
+                   desc = "file server up; volser and scanner down";
                else if (abnode->volRunning && !abnode->scanRunning)
-                   strcpy(abuffer,
-                          "file server up; volser up; scanner down");
+                   desc = "file server up; volser up; scanner down";
                else if (!abnode->volRunning && abnode->scanRunning)
-                   strcpy(abuffer,
-                          "file server up; volser down; scanner up");
-
+                   desc = "file server up; volser down; scanner up";
                else
-                   strcpy(abuffer, "file server running");
+                   desc = "file server running";
            } else if (!abnode->volRunning)
-               strcpy(abuffer, "file server up; volser down");
+               desc = "file server up; volser down";
            else
-               strcpy(abuffer, "file server running");
+               desc = "file server running";
        } else if (abnode->salRunning) {
-           strcpy(abuffer, "salvaging file system");
+           desc = "salvaging file system";
        } else
-           strcpy(abuffer, "starting file server");
+           desc = "starting file server";
     } else {
        /* shutting down */
        if (abnode->fileRunning || abnode->volRunning || abnode->scanRunning) {
-           strcpy(abuffer, "file server shutting down");
+           desc = "file server shutting down";
        } else if (abnode->salRunning)
-           strcpy(abuffer, "salvager shutting down");
+           desc = "salvager shutting down";
        else
-           strcpy(abuffer, "file server shut down");
+           desc = "file server shut down";
     }
+    *adesc = strdup(desc);
+    if (*adesc == NULL)
+       return BZIO;
     return 0;
 }
 
 static int
-fs_getparm(struct bnode *bn, afs_int32 aindex, char *abuffer,
-          afs_int32 alen)
+fs_getparm(struct bnode *bn, afs_int32 aindex, char **aparm)
 {
     struct fsbnode *abnode = (struct fsbnode *)bn;
+    char *parm;
 
     if (aindex == 0)
-       strcpy(abuffer, abnode->filecmd);
+       parm = abnode->filecmd;
     else if (aindex == 1)
-       strcpy(abuffer, abnode->volcmd);
+       parm = abnode->volcmd;
     else if (aindex == 2)
-       strcpy(abuffer, abnode->salcmd);
+       parm = abnode->salcmd;
     else if (aindex == 3 && abnode->scancmd)
-       strcpy(abuffer, abnode->scancmd);
+       parm = abnode->scancmd;
     else
        return BZDOM;
+    *aparm = strdup(parm);
+    if (*aparm == NULL)
+       return BZIO;
     return 0;
 }
 
 static int
-dafs_getparm(struct bnode *bn, afs_int32 aindex, char *abuffer,
-            afs_int32 alen)
+dafs_getparm(struct bnode *bn, afs_int32 aindex, char **aparm)
 {
     struct fsbnode *abnode = (struct fsbnode *)bn;
+    char *parm;
 
     if (aindex == 0)
-       strcpy(abuffer, abnode->filecmd);
+       parm = abnode->filecmd;
     else if (aindex == 1)
-       strcpy(abuffer, abnode->volcmd);
+       parm = abnode->volcmd;
     else if (aindex == 2)
-       strcpy(abuffer, abnode->salsrvcmd);
+       parm = abnode->salsrvcmd;
     else if (aindex == 3)
-       strcpy(abuffer, abnode->salcmd);
+       parm = abnode->salcmd;
     else if (aindex == 4 && abnode->scancmd)
-       strcpy(abuffer, abnode->scancmd);
+       parm = abnode->scancmd;
     else
        return BZDOM;
+    *aparm = strdup(parm);
+    if (*aparm == NULL)
+       return BZIO;
     return 0;
 }