LU-17718 obdclass: potential string overflow upcall_cache.c

Use strncpy() in upcall_cache_set_upcall() to quiet Coverity warning.
And reorganize the function so that the code flow is more linear in
the success case.

CoverityID: 424705: ("String overflow")

Fixes: 2153e86541 ("LU-17497 obdclass: check upcall incorrect values")
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
Change-Id: I1aee77f78c92c6c571dfe358435a2733cc3ba9d9
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54710
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Aurelien Degremont <adegremont@nvidia.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

diff --git a/lustre/obdclass/upcall_cache.c b/lustre/obdclass/upcall_cache.c
index f71901b..c431917 100644 (file)
--- a/lustre/obdclass/upcall_cache.c
+++ b/lustre/obdclass/upcall_cache.c
@@ -155,6 +155,7 @@ int upcall_cache_set_upcall(struct upcall_cache *cache, const char *buffer,
                            size_t count, bool path_only)
 {
        char *upcall;
+       int rc = 0;
 
        if (count >= UC_CACHE_UPCALL_MAXPATH)
                return -E2BIG;
@@ -165,30 +166,26 @@ int upcall_cache_set_upcall(struct upcall_cache *cache, const char *buffer,
 
        /* Remove any extraneous bits from the upcall (e.g. linefeeds) */
        if (sscanf(buffer, "%s", upcall) != 1)
-               goto invalid;
-
-       if (upcall[0] == '/')
-               goto valid;
-
-       if (path_only)
-               goto invalid;
+               GOTO(out, rc = -EINVAL);
 
-       if (strcasecmp(upcall, "NONE") == 0) {
-               snprintf(upcall, count + 1, "NONE");
-               goto valid;
+       /* Accepted values are:
+        * - an absolute path to an executable
+        * - if path_only is false: "none", case insensitive
+        */
+       if (upcall[0] != '/') {
+               if (!path_only && strcasecmp(upcall, "NONE") == 0)
+                       snprintf(upcall, count + 1, "NONE");
+               else
+                       GOTO(out, rc = -EINVAL);
        }
 
-invalid:
-       OBD_FREE(upcall, count + 1);
-       return -EINVAL;
-
-valid:
        down_write(&cache->uc_upcall_rwsem);
-       strcpy(cache->uc_upcall, upcall);
+       strncpy(cache->uc_upcall, upcall, count + 1);
        up_write(&cache->uc_upcall_rwsem);
 
+out:
        OBD_FREE(upcall, count + 1);
-       return 0;
+       return rc;
 }
 EXPORT_SYMBOL(upcall_cache_set_upcall);
 

diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh
index 659a110..b5d31e8 100755 (executable)
--- a/lustre/tests/sanity-sec.sh
+++ b/lustre/tests/sanity-sec.sh
@@ -6272,29 +6272,29 @@ test_69() {
        orig="$(do_facet mds1 "$LCTL get_param -n $param")"
        stack_trap "do_facet mds1 $LCTL set_param $param=$orig" EXIT
 
-       # identity_upcall accepts a path to an executable,
+       # identity_upcall accepts an absolute path to an executable,
        # or NONE (case insensitive)
        do_facet mds1 $LCTL set_param $param=/path/to/prog ||
-               error "set_param $param=/path/to/prog failed"
+               error "set_param $param=/path/to/prog failed (1)"
        do_facet mds1 $LCTL set_param $param=prog &&
-               error "set_param $param=prog should failed"
+               error "set_param $param=prog should fail (1)"
        do_facet mds1 $LCTL set_param $param=NONE ||
-               error "set_param $param=NONE failed"
+               error "set_param $param=NONE failed (1)"
        do_facet mds1 $LCTL set_param $param=none ||
-               error "set_param $param=none failed"
+               error "set_param $param=none failed (1)"
 
        if $GSS; then
                param="sptlrpc.gss.rsi_upcall"
                orig="$(do_facet mds1 "$LCTL get_param -n $param")"
                stack_trap "do_facet mds1 $LCTL set_param $param=$orig" EXIT
 
-               # rsi_upcall only accepts a path to an executable
+               # rsi_upcall only accepts an absolute path to an executable
                do_facet mds1 $LCTL set_param $param=prog &&
-                       error "set_param $param=prog should failed"
+                       error "set_param $param=prog should fail (2)"
                do_facet mds1 $LCTL set_param $param=NONE &&
-                       error "set_param $param=NONE should fail"
+                       error "set_param $param=NONE should fail (2)"
                do_facet mds1 $LCTL set_param $param=/path/to/prog ||
-                       error "set_param $param=/path/to/prog failed"
+                       error "set_param $param=/path/to/prog failed (2)"
        fi
 }
 run_test 69 "check upcall incorrect values"