Skip to content

Commit 8b02e09

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 72a9f0c commit 8b02e09

9 files changed

Lines changed: 352 additions & 67 deletions

File tree

‎src/utils/ddb/ddb_parse.c‎

Lines changed: 106 additions & 35 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

@@ -22,55 +25,123 @@ safe_strcat(char *dst, const char *src, size_t dst_size)
2225
strncat(dst, src, remaining_space);
2326
}
2427

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

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

35-
D_ALLOC(path_copy, path_len);
36-
if (path_copy == NULL)
37-
return -DER_NOMEM;
38-
strcpy(path_copy, path);
39-
40-
tok = strtok(path_copy, "/");
41-
while (tok != NULL && rc != 0) {
42-
rc = uuid_parse(tok, vos_file_parts->vf_pool_uuid);
43-
if (!SUCCESS(rc)) {
44-
safe_strcat(vos_file_parts->vf_db_path, "/", DB_PATH_LEN);
45-
safe_strcat(vos_file_parts->vf_db_path, tok, DB_PATH_LEN);
66+
rc = regcomp(&preg, regex_buf, REG_EXTENDED);
67+
if (rc != 0) {
68+
print_regx_error(rc, &preg, regex_buf);
69+
rc = -DER_INVAL;
70+
goto out;
71+
}
72+
73+
rc = regexec(&preg, path, MATCH_SIZE, match, 0);
74+
if (rc == REG_NOMATCH) {
75+
D_ERROR("Innvalid VOS path: '%s'\n", path);
76+
rc = -DER_INVAL;
77+
goto out_preg;
78+
}
79+
D_ASSERT(SUCCESS(rc));
80+
81+
vos_file_parts->vf_db_path[0] = '\0';
82+
if ((match[DB_PATH_IDX].rm_eo - match[DB_PATH_IDX].rm_so) != 0) {
83+
D_ASSERT(match[DB_PATH_IDX].rm_so == 0);
84+
if (match[DB_PATH_IDX].rm_eo > DB_PATH_SIZE) {
85+
D_ERROR("DB path '%.*s' too long in VOS path '%s': get=%i, max=%i\n",
86+
match[DB_PATH_IDX].rm_eo - 1, &path[match[DB_PATH_IDX].rm_so], path,
87+
match[DB_PATH_IDX].rm_eo - 1, DB_PATH_SIZE - 1);
88+
rc = -DER_INVAL;
89+
goto out_preg;
4690
}
47-
tok = strtok(NULL, "/");
91+
memcpy(vos_file_parts->vf_db_path, path, match[DB_PATH_IDX].rm_eo - 1);
92+
vos_file_parts->vf_db_path[match[DB_PATH_IDX].rm_eo - 1] = '\0';
4893
}
4994

50-
if (rc != 0 || tok == NULL) {
51-
D_ERROR("Incomplete path: %s\n", path);
52-
D_GOTO(done, rc = -DER_INVAL);
95+
D_ASSERT(match[POOL_UUID_IDX].rm_so != (regoff_t)-1);
96+
D_ASSERT(match[POOL_UUID_IDX].rm_eo - match[POOL_UUID_IDX].rm_so == POOL_UUID_LEN);
97+
memcpy(pool_uuid, &path[match[POOL_UUID_IDX].rm_so], POOL_UUID_LEN);
98+
pool_uuid[POOL_UUID_LEN] = '\0';
99+
rc = uuid_parse(pool_uuid, vos_file_parts->vf_pool_uuid);
100+
if (!SUCCESS(rc)) {
101+
D_CRIT("Invalid Pool UUID '%s' in VOS path '%s'\n", pool_uuid, path);
102+
rc = -DER_INVAL;
103+
goto out_preg;
53104
}
54105

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

68-
done:
69-
if (!SUCCESS(rc)) {
70-
/* Reset to if not valid */
137+
rc = -DER_SUCCESS;
138+
139+
out_preg:
140+
regfree(&preg);
141+
out:
142+
/* Reset to zero if not valid */
143+
if (!SUCCESS(rc))
71144
memset(vos_file_parts, 0, sizeof(*vos_file_parts));
72-
}
73-
D_FREE(path_copy);
74145
return rc;
75146
}
76147

‎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 64
18+
enum { DB_PATH_SIZE = 64, 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
@@ -80,7 +80,7 @@ dv_pool_destroy(const char *path)
8080
return rc;
8181
}
8282

83-
if (strncmp(path_parts.vf_vos_file, "rdb", 3) == 0)
83+
if (strncmp(path_parts.vf_vos_file_name, "rdb", 3) == 0)
8484
flags |= VOS_POF_RDB;
8585

8686
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)