From 7869bb320e735547410a7d3e31061b9044389c53 Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Tue, 9 Apr 2024 15:00:41 +0200 Subject: [PATCH] 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 Change-Id: I1aee77f78c92c6c571dfe358435a2733cc3ba9d9 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54710 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Aurelien Degremont Reviewed-by: Oleg Drokin Reviewed-by: Andreas Dilger --- lustre/obdclass/upcall_cache.c | 31 ++++++++++++++----------------- lustre/tests/sanity-sec.sh | 18 +++++++++--------- 2 files changed, 23 insertions(+), 26 deletions(-) diff --git a/lustre/obdclass/upcall_cache.c b/lustre/obdclass/upcall_cache.c index f71901b..c431917 100644 --- 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 --- 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" -- 1.8.3.1