From 61d438e73857776a1f96a7334f56b132275a587b Mon Sep 17 00:00:00 2001 From: Sheetal Pamecha Date: Mon, 19 Aug 2019 15:27:57 +0530 Subject: libgfapi: return correct errno on invalid volume name glfs_init when called with volume name prefixed by '/' sets errno to 0. Setting errno to EINVAL to resolve the issue. Also volname is a parameter to glfs_new. Thus, validating volname in glfs_new itself and returning EINVAL from that function fixes: bz#1507896 Change-Id: I0d4d2423e26cc07644d50ec8cce788ecc639203d Signed-off-by: Sheetal Pamecha --- api/src/glfs.c | 15 ++++++++++- libglusterfs/src/graph.c | 13 +++++----- tests/basic/gfapi/bug-1507896.c | 49 ++++++++++++++++++++++++++++++++++++ tests/basic/gfapi/bug-1507896.t | 33 ++++++++++++++++++++++++ tests/basic/gfapi/glfsxmp-coverage.c | 21 ++++++++++------ 5 files changed, 115 insertions(+), 16 deletions(-) create mode 100644 tests/basic/gfapi/bug-1507896.c create mode 100644 tests/basic/gfapi/bug-1507896.t diff --git a/api/src/glfs.c b/api/src/glfs.c index 3f9622eea5d..82261369fe1 100644 --- a/api/src/glfs.c +++ b/api/src/glfs.c @@ -817,17 +817,30 @@ struct glfs * pub_glfs_new(const char *volname) { struct glfs *fs = NULL; + int i = 0; int ret = -1; glusterfs_ctx_t *ctx = NULL; xlator_t *old_THIS = NULL; char pname[16] = ""; char msg[32] = ""; - if (!volname) { + if (!volname || volname[0] == '/' || volname[0] == '-') { + if (strncmp(volname, "/snaps/", 7) == 0) { + goto label; + } errno = EINVAL; return NULL; } + for (i = 0; i < strlen(volname); i++) { + if (!isalnum(volname[i]) && (volname[i] != '_') && + (volname[i] != '-')) { + errno = EINVAL; + return NULL; + } + } + +label: /* * Do this as soon as possible in case something else depends on * pool allocations. diff --git a/libglusterfs/src/graph.c b/libglusterfs/src/graph.c index e6ae40db2ed..913f57804cc 100644 --- a/libglusterfs/src/graph.c +++ b/libglusterfs/src/graph.c @@ -568,14 +568,13 @@ glusterfs_graph_prepare(glusterfs_graph_t *graph, glusterfs_ctx_t *ctx, } else { ret = glusterfs_graph_settop(graph, volume_name, _gf_false); } - if (!ret) { - goto ok; - } - gf_msg("graph", GF_LOG_ERROR, 0, LG_MSG_GRAPH_ERROR, - "glusterfs graph settop failed"); - return -1; -ok: + if (ret) { + gf_msg("graph", GF_LOG_ERROR, EINVAL, LG_MSG_GRAPH_ERROR, + "glusterfs graph settop failed"); + errno = EINVAL; + return -1; + } /* XXX: WORM VOLUME */ ret = glusterfs_graph_worm(graph, ctx); diff --git a/tests/basic/gfapi/bug-1507896.c b/tests/basic/gfapi/bug-1507896.c new file mode 100644 index 00000000000..1cc20849c2b --- /dev/null +++ b/tests/basic/gfapi/bug-1507896.c @@ -0,0 +1,49 @@ +#include +#include +#include +#include +#include + +#define VALIDATE_AND_GOTO_LABEL_ON_ERROR(func, ret, label) \ + do { \ + if (ret < 0) { \ + fprintf(stderr, "%s : returned error %d (%s)\n", func, ret, \ + strerror(errno)); \ + goto label; \ + } \ + } while (0) + +int +main(int argc, char *argv[]) +{ + int ret = -1; + glfs_t *fs = NULL; + char *volname = NULL; + char *logfile = NULL; + char *hostname = NULL; + + hostname = argv[1]; + volname = argv[2]; + logfile = argv[3]; + + fs = glfs_new(volname); + if (!fs) + VALIDATE_AND_GOTO_LABEL_ON_ERROR("glfs_new(fs)", ret, out); + + ret = glfs_set_volfile_server(fs, "tcp", hostname, 24007); + VALIDATE_AND_GOTO_LABEL_ON_ERROR("glfs_set_volfile_server(fs)", ret, out); + + ret = glfs_set_logging(fs, logfile, 7); + VALIDATE_AND_GOTO_LABEL_ON_ERROR("glfs_set_logging(fs)", ret, out); + + ret = glfs_init(fs); + VALIDATE_AND_GOTO_LABEL_ON_ERROR("glfs_init(fs)", ret, out); + +out: + if (fs) { + ret = glfs_fini(fs); + if (ret) + fprintf(stderr, "glfs_fini(fs) returned %d\n", ret); + } + return ret; +} diff --git a/tests/basic/gfapi/bug-1507896.t b/tests/basic/gfapi/bug-1507896.t new file mode 100644 index 00000000000..4764e650232 --- /dev/null +++ b/tests/basic/gfapi/bug-1507896.t @@ -0,0 +1,33 @@ +#!/bin/bash + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc + +cleanup; + +TEST glusterd + +TEST $CLI volume create $V0 $H0:$B0/brick1; +EXPECT 'Created' volinfo_field $V0 'Status'; + +TEST $CLI volume start $V0; +EXPECT 'Started' volinfo_field $V0 'Status'; + +logdir=`gluster --print-logdir` + +TEST build_tester $(dirname $0)/bug-1507896.c -lgfapi + +TEST ./$(dirname $0)/bug-1507896 $H0 $V0 $logdir/bug-1507896.log + +#volume name precedding with '/' +TEST ! ./$(dirname $0)/bug-1507896 $H0 /$V0 $logdir/bug-1507896.log + +#volume name passed with any special characters +TEST ! ./$(dirname $0)/bug-1507896 $H0 test@_$V0 $logdir/bug-1507896.log + +cleanup_tester $(dirname $0)/bug-1507896 + +TEST $CLI volume stop $V0 +TEST $CLI volume delete $V0 + +cleanup; diff --git a/tests/basic/gfapi/glfsxmp-coverage.c b/tests/basic/gfapi/glfsxmp-coverage.c index 474ba9c0949..51650023efd 100644 --- a/tests/basic/gfapi/glfsxmp-coverage.c +++ b/tests/basic/gfapi/glfsxmp-coverage.c @@ -1843,20 +1843,25 @@ main(int argc, char *argv[]) sleep(2); - fs2 = glfs_new(argv[1]); - if (!fs2) { - fprintf(stderr, "glfs_new: returned NULL\n"); - return 1; - } if (argc == 2) { + /* Generally glfs_new() requires volume name as an argument */ + fs2 = glfs_new("test_only_volume"); + if (!fs2) { + fprintf(stderr, "glfs_new(fs2): returned NULL\n"); + return 1; + } ret = glfs_set_volfile(fs2, argv[1]); if (ret) - fprintf(stderr, "glfs_set_volfile failed\n"); + fprintf(stderr, "glfs_set_volfile failed(fs2)\n"); } else { - // ret = glfs_set_volfile_server (fs2, "unix", "/tmp/gluster.sock", 0); + fs2 = glfs_new(argv[1]); + if (!fs2) { + fprintf(stderr, "glfs_new(fs2): returned NULL\n"); + return 1; + } ret = glfs_set_volfile_server(fs2, "tcp", argv[2], 24007); if (ret) - fprintf(stderr, "glfs_set_volfile_server failed\n"); + fprintf(stderr, "glfs_set_volfile_server failed(fs2)\n"); } ret = glfs_set_statedump_path(fs2, "/tmp"); -- cgit