Skip to content

Commit f23c95c

Browse files
kanard38knard38
kanard38
authored and
knard38
committed
DAOS-18323 ddb: Incorrect parsing of vos path with DDB
Fix ddb vos file path parsing. Features: recovery Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
1 parent 0f34795 commit f23c95c

9 files changed

Lines changed: 351 additions & 71 deletions

File tree

‎src/utils/ddb/ddb_parse.c‎

Lines changed: 105 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
#include <wordexp.h>
1010
#include <getopt.h>
11+
#include <regex.h>
12+
#include <errno.h>
13+
#include <string.h>
1114

1215
#include <daos_errno.h>
1316
#include <daos_srv/bio.h>
@@ -23,60 +26,123 @@ safe_strcat(char *dst, const char *src, size_t dst_size)
2326
strncat(dst, src, remaining_space);
2427
}
2528

29+
static void
30+
print_regx_error(int rc, regex_t *preg, const char *regex_buf)
31+
{
32+
char *buf;
33+
size_t buf_size;
34+
35+
buf_size = regerror(rc, preg, NULL, 0);
36+
D_ALLOC_ARRAY(buf, buf_size);
37+
D_ASSERT(buf != NULL);
38+
regerror(rc, preg, buf, buf_size);
39+
D_CRIT("Invalid regex '%s': %s", regex_buf, buf);
40+
D_FREE(buf);
41+
}
42+
2643
int
2744
vos_path_parse(const char *path, struct vos_file_parts *vos_file_parts)
2845
{
29-
uint32_t path_len = strlen(path) + 1;
30-
char *path_copy;
31-
char *tok;
32-
int rc = -DER_INVAL;
46+
enum {
47+
DB_PATH_IDX = 1,
48+
POOL_UUID_IDX = 3,
49+
VOS_FILE_NAME_IDX = 5,
50+
TARGET_IDX_IDX = 6,
51+
MATCH_SIZE = 7,
52+
POOL_UUID_LEN = 36
53+
};
54+
const char *regex_buf =
55+
"^(/?([^/]+/)*)([0-9a-fA-F]{8}(-[0-9a-fA-F]{4}){3}-[0-9a-fA-F]{12})/"
56+
"(vos-([0-9]|([1-9][0-9]+)))$";
57+
char *endptr;
58+
uint64_t target_idx;
59+
regex_t preg;
60+
char pool_uuid[POOL_UUID_LEN + 1];
61+
regmatch_t match[MATCH_SIZE + 1];
62+
size_t vos_file_name_len;
63+
int rc;
3364

3465
D_ASSERT(path != NULL && vos_file_parts != NULL);
3566

36-
D_ALLOC(path_copy, path_len);
37-
if (path_copy == NULL)
38-
return -DER_NOMEM;
39-
strcpy(path_copy, path);
40-
41-
tok = strtok(path_copy, "/");
42-
while (tok != NULL && rc != 0) {
43-
rc = uuid_parse(tok, vos_file_parts->vf_pool_uuid);
44-
if (!SUCCESS(rc)) {
45-
safe_strcat(vos_file_parts->vf_db_path, "/", DB_PATH_LEN);
46-
safe_strcat(vos_file_parts->vf_db_path, tok, DB_PATH_LEN);
47-
}
48-
tok = strtok(NULL, "/");
67+
rc = regcomp(&preg, regex_buf, REG_EXTENDED);
68+
if (rc != 0) {
69+
print_regx_error(rc, &preg, regex_buf);
70+
rc = -DER_INVAL;
71+
goto out;
4972
}
5073

51-
if (rc != 0 || tok == NULL) {
52-
D_ERROR("Incomplete path: %s\n", path);
53-
D_GOTO(done, rc = -DER_INVAL);
74+
rc = regexec(&preg, path, MATCH_SIZE, match, 0);
75+
if (rc == REG_NOMATCH) {
76+
D_ERROR("Innvalid VOS path: '%s'\n", path);
77+
rc = -DER_INVAL;
78+
goto out_preg;
79+
}
80+
D_ASSERT(SUCCESS(rc));
81+
82+
vos_file_parts->vf_db_path[0] = '\0';
83+
if ((match[DB_PATH_IDX].rm_eo - match[DB_PATH_IDX].rm_so) != 0) {
84+
D_ASSERT(match[DB_PATH_IDX].rm_so == 0);
85+
if (match[DB_PATH_IDX].rm_eo > DB_PATH_SIZE) {
86+
D_ERROR("DB path '%.*s' too long in VOS path '%s': get=%i, max=%i\n",
87+
match[DB_PATH_IDX].rm_eo - 1, &path[match[DB_PATH_IDX].rm_so], path,
88+
match[DB_PATH_IDX].rm_eo - 1, DB_PATH_SIZE - 1);
89+
rc = -DER_INVAL;
90+
goto out_preg;
91+
}
92+
memcpy(vos_file_parts->vf_db_path, path, match[DB_PATH_IDX].rm_eo - 1);
93+
vos_file_parts->vf_db_path[match[DB_PATH_IDX].rm_eo - 1] = '\0';
5494
}
5595

56-
strncpy(vos_file_parts->vf_vos_file, tok, ARRAY_SIZE(vos_file_parts->vf_vos_file) - 1);
57-
58-
if (strcmp(vos_file_parts->vf_vos_file, "rdb-pool") == 0) {
59-
vos_file_parts->vf_target_idx = BIO_SYS_TGT_ID;
60-
goto done;
96+
D_ASSERT(match[POOL_UUID_IDX].rm_so != (regoff_t)-1);
97+
D_ASSERT(match[POOL_UUID_IDX].rm_eo - match[POOL_UUID_IDX].rm_so == POOL_UUID_LEN);
98+
memcpy(pool_uuid, &path[match[POOL_UUID_IDX].rm_so], POOL_UUID_LEN);
99+
pool_uuid[POOL_UUID_LEN] = '\0';
100+
rc = uuid_parse(pool_uuid, vos_file_parts->vf_pool_uuid);
101+
if (!SUCCESS(rc)) {
102+
D_CRIT("Invalid Pool UUID '%s' in VOS path '%s'\n", pool_uuid, path);
103+
rc = -DER_INVAL;
104+
goto out_preg;
61105
}
62106

63-
/*
64-
* file name should be vos-N ... split on "-"
65-
* If not, might be test, just assume target of 0
66-
*/
67-
strtok(tok, "-");
68-
tok = strtok(NULL, "-");
69-
if (tok != NULL) {
70-
D_WARN("vos file name not in correct format: %s\n", vos_file_parts->vf_vos_file);
71-
vos_file_parts->vf_target_idx = atoi(tok);
107+
D_ASSERT(match[VOS_FILE_NAME_IDX].rm_so != (regoff_t)-1);
108+
vos_file_name_len = match[VOS_FILE_NAME_IDX].rm_eo - match[VOS_FILE_NAME_IDX].rm_so;
109+
if (vos_file_name_len + 1 > VOS_FILE_NAME_SIZE) {
110+
D_ERROR("VOS file name '%.*s' too long in VOS path '%s': get=%zu, max=%i\n",
111+
(int)vos_file_name_len, &path[match[VOS_FILE_NAME_IDX].rm_so], path,
112+
vos_file_name_len, VOS_FILE_NAME_SIZE - 1);
113+
rc = -DER_INVAL;
114+
goto out_preg;
115+
}
116+
memcpy(vos_file_parts->vf_vos_file_name, &path[match[VOS_FILE_NAME_IDX].rm_so],
117+
vos_file_name_len);
118+
vos_file_parts->vf_vos_file_name[vos_file_name_len] = '\0';
119+
120+
D_ASSERT(match[TARGET_IDX_IDX].rm_so != (regoff_t)-1);
121+
errno = 0;
122+
target_idx = strtoull(&path[match[TARGET_IDX_IDX].rm_so], &endptr, 10);
123+
if (errno != 0 || endptr == &path[match[TARGET_IDX_IDX].rm_so] || *endptr != '\0') {
124+
D_CRIT("Invalid target index '%s' in VOS path '%s': %s\n",
125+
&path[match[TARGET_IDX_IDX].rm_so], path, strerror(errno));
126+
rc = -DER_INVAL;
127+
goto out_preg;
72128
}
129+
if (target_idx > UINT32_MAX) {
130+
D_ERROR("Target index " DF_U64
131+
"' out of range in VOS path '%s': min=0 , max=%" PRIu32 "\n",
132+
target_idx, path, UINT32_MAX);
133+
rc = -DER_INVAL;
134+
goto out_preg;
135+
}
136+
vos_file_parts->vf_target_idx = target_idx;
73137

74-
done:
75-
if (!SUCCESS(rc)) {
76-
/* Reset to if not valid */
138+
rc = -DER_SUCCESS;
139+
140+
out_preg:
141+
regfree(&preg);
142+
out:
143+
/* Reset to zero if not valid */
144+
if (!SUCCESS(rc))
77145
memset(vos_file_parts, 0, sizeof(*vos_file_parts));
78-
}
79-
D_FREE(path_copy);
80146
return rc;
81147
}
82148

‎src/utils/ddb/ddb_parse.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@
1515
#include <daos_types.h>
1616
#include "ddb_common.h"
1717

18-
#define DB_PATH_LEN 256
18+
enum { DB_PATH_SIZE = 256, VOS_FILE_NAME_SIZE = 16 };
1919
struct vos_file_parts {
20-
char vf_db_path[DB_PATH_LEN];
21-
uuid_t vf_pool_uuid;
22-
char vf_vos_file[16];
23-
uint32_t vf_target_idx;
20+
char vf_db_path[DB_PATH_SIZE];
21+
uuid_t vf_pool_uuid;
22+
char vf_vos_file_name[VOS_FILE_NAME_SIZE];
23+
uint32_t vf_target_idx;
2424
};
2525

2626
/* Parse a path to a VOS file to get needed parts for initializing vos */

‎src/utils/ddb/ddb_vos.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ dv_pool_destroy(const char *path, const char *db_path)
8585
return rc;
8686
}
8787

88-
if (strncmp(path_parts.vf_vos_file, "rdb", 3) == 0)
88+
if (strncmp(path_parts.vf_vos_file_name, "rdb", 3) == 0)
8989
flags |= VOS_POF_RDB;
9090

9191
rc = vos_pool_destroy_ex(path, path_parts.vf_pool_uuid, flags);

‎src/utils/ddb/tests/SConscript‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,11 @@ def scons():
5050
prereqs.require(tenv, 'argobots', 'spdk', 'pmdk')
5151
# Required for dtx_act_discard_invalid tests.
5252
# This function is validated by its respective unit tests.
53-
tenv.AppendUnique(LINKFLAGS=['-Wl,--wrap=vos_dtx_discard_invalid'])
53+
tenv.AppendUnique(LINKFLAGS=['-Wl,--wrap=vos_dtx_discard_invalid', '-Wl,--wrap=regcomp',
54+
'-Wl,--wrap=uuid_parse', '-Wl,--wrap=strtoull'])
5455

5556
libs = ['uuid', 'daos_common_pmem', 'gurt', 'vea', 'abt', 'bio', 'cmocka', 'pthread', 'pmemobj']
56-
src = ['ddb_ut.c', 'ddb_vos_ut.c']
57+
src = ['ddb_ut.c', 'ddb_vos_ut.c', 'ddb_parse_ut.c']
5758
# vos and mock object files to wrap vos symbols
5859
vos_obj = tenv.Object(Glob('../../../vos/*.c'))
5960
mock_obj = tenv.Object(Glob('../../../dtx/tests/*_mock.c'))

‎src/utils/ddb/tests/ddb_parse_tests.c‎

Lines changed: 79 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@
1313
#include "ddb_cmocka.h"
1414
#include "ddb_test_driver.h"
1515

16+
/*
17+
* -----------------------------------------------
18+
* Mock implementations
19+
* -----------------------------------------------
20+
*/
21+
22+
#define MOCKED_POOL_UUID_STR "12345678-1234-1234-1234-123456789012"
23+
1624
static int
1725
fake_print(const char *fmt, ...)
1826
{
@@ -25,27 +33,83 @@ fake_print(const char *fmt, ...)
2533
* -----------------------------------------------
2634
*/
2735

28-
#define assert_invalid_f_path(path, parts) assert_invalid(vos_path_parse(path, &parts))
29-
#define assert_f_path(path, parts) assert_success(vos_path_parse(path, &parts))
30-
3136
static void
32-
vos_file_parts_tests(void **state)
37+
vos_file_parse_test_errors(void **state)
3338
{
39+
uuid_t pool_uuid;
3440
struct vos_file_parts parts = {0};
35-
uuid_t expected_uuid;
41+
int rc;
3642

37-
uuid_parse("12345678-1234-1234-1234-123456789012", expected_uuid);
43+
rc = uuid_parse(MOCKED_POOL_UUID_STR, pool_uuid);
44+
assert_rc_equal(rc, 0);
3845

39-
assert_invalid_f_path("", parts);
40-
assert_invalid_f_path("/mnt/daos", parts);
41-
assert_invalid_f_path("/mnt/daos/12345678-1234-1234-1234-123456789012", parts);
46+
/* Test invalid vos paths not respecting regex */
47+
rc = vos_path_parse("", &parts);
48+
assert_rc_equal(rc, -DER_INVAL);
4249

43-
assert_f_path("/mnt/daos/12345678-1234-1234-1234-123456789012/vos-1", parts);
50+
rc = vos_path_parse("/mnt/daos", &parts);
51+
assert_rc_equal(rc, -DER_INVAL);
52+
53+
rc = vos_path_parse("/mnt/daos/" MOCKED_POOL_UUID_STR, &parts);
54+
assert_rc_equal(rc, -DER_INVAL);
55+
56+
rc = vos_path_parse("//mnt/daos/" MOCKED_POOL_UUID_STR "/vos-1", &parts);
57+
assert_rc_equal(rc, -DER_INVAL);
58+
59+
rc = vos_path_parse("/mnt/daos/g2345678-1234-1234-1234-123456789012/vos-1", &parts);
60+
assert_rc_equal(rc, -DER_INVAL);
4461

62+
/* Test invalid vos paths with too long db path */
63+
rc = vos_path_parse(
64+
"/long/path/!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!/" MOCKED_POOL_UUID_STR
65+
"/vos-1",
66+
&parts);
67+
assert_rc_equal(rc, -DER_INVAL);
68+
69+
/* Test invalid vos paths with too long vos file name */
70+
rc = vos_path_parse("/mnt/daos/" MOCKED_POOL_UUID_STR "/vos-999999999999", &parts);
71+
assert_rc_equal(rc, -DER_INVAL);
72+
73+
/* Test invalid vos paths with invalid target idx */
74+
rc = vos_path_parse("/mnt/daos/" MOCKED_POOL_UUID_STR "/vos-99999999999", &parts);
75+
assert_rc_equal(rc, -DER_INVAL);
76+
}
77+
78+
static void
79+
vos_file_parse_test_success(void **state)
80+
{
81+
uuid_t expected_uuid;
82+
struct vos_file_parts parts = {0};
83+
int rc;
84+
85+
rc = uuid_parse(MOCKED_POOL_UUID_STR, expected_uuid);
86+
assert_rc_equal(rc, 0);
87+
88+
/* Test with absolute path */
89+
rc = vos_path_parse("/mnt/daos/" MOCKED_POOL_UUID_STR "/vos-0", &parts);
90+
assert_rc_equal(rc, 0);
4591
assert_string_equal("/mnt/daos", parts.vf_db_path);
4692
assert_uuid_equal(expected_uuid, parts.vf_pool_uuid);
47-
assert_string_equal("vos-1", parts.vf_vos_file);
48-
assert_int_equal(1, parts.vf_target_idx);
93+
assert_string_equal("vos-0", parts.vf_vos_file_name);
94+
assert_int_equal(0, parts.vf_target_idx);
95+
96+
/* Test with relative path */
97+
memset(&parts, 0, sizeof(parts));
98+
rc = vos_path_parse("mnt/daos/" MOCKED_POOL_UUID_STR "/vos-42", &parts);
99+
assert_rc_equal(rc, 0);
100+
assert_string_equal("mnt/daos", parts.vf_db_path);
101+
assert_uuid_equal(expected_uuid, parts.vf_pool_uuid);
102+
assert_string_equal("vos-42", parts.vf_vos_file_name);
103+
assert_int_equal(42, parts.vf_target_idx);
104+
105+
/* Test with null db path */
106+
memset(&parts, 1, sizeof(parts));
107+
rc = vos_path_parse(MOCKED_POOL_UUID_STR "/vos-666", &parts);
108+
assert_rc_equal(rc, 0);
109+
assert_string_equal("", parts.vf_db_path);
110+
assert_uuid_equal(expected_uuid, parts.vf_pool_uuid);
111+
assert_string_equal("vos-666", parts.vf_vos_file_name);
112+
assert_int_equal(666, parts.vf_target_idx);
49113
}
50114

51115
#define assert_vtp_eq(a, b) \
@@ -293,8 +357,9 @@ int
293357
ddb_parse_tests_run()
294358
{
295359
static const struct CMUnitTest tests[] = {
296-
TEST(vos_file_parts_tests), TEST(parse_dtx_id_tests), TEST(keys_are_parsed_correctly),
297-
TEST(pool_flags_tests), TEST(date2cmt_time_tests),
360+
TEST(vos_file_parse_test_errors), TEST(vos_file_parse_test_success),
361+
TEST(parse_dtx_id_tests), TEST(keys_are_parsed_correctly),
362+
TEST(pool_flags_tests), TEST(date2cmt_time_tests),
298363
};
299364
return cmocka_run_group_tests_name("DDB helper parsing function tests", tests,
300365
NULL, NULL);

0 commit comments

Comments
 (0)