From d3fe8e29a06e81482c2a60e5d932bcfbf7c1cb01 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 17 Oct 2017 16:53:51 -0700 Subject: [PATCH] modules/kvs: Replace EPERM w/ ENOTRECOVERABLE Previously EPERM was generally used as the errno to signify that the state without the KVS was inconsistent/illogical (e.g. a dirref points to a val object). To remove confusion that a potential security issue is in play, change the errno from EPERM to ENOTRECOVERABLE. Update unit tests appropriately. Fixes #1228 --- src/modules/kvs/commit.c | 6 +++--- src/modules/kvs/lookup.c | 28 ++++++++++++++-------------- src/modules/kvs/test/commit.c | 4 ++-- src/modules/kvs/test/lookup.c | 30 ++++++++++++++++-------------- 4 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/modules/kvs/commit.c b/src/modules/kvs/commit.c index d3ddaaaad83d..24658ae581a8 100644 --- a/src/modules/kvs/commit.c +++ b/src/modules/kvs/commit.c @@ -388,7 +388,7 @@ static int commit_link_dirent (commit_t *c, int current_epoch, *next++ = '\0'; if (!treeobj_is_dir (dir)) { - saved_errno = EPERM; + saved_errno = ENOTRECOVERABLE; goto done; } @@ -419,7 +419,7 @@ static int commit_link_dirent (commit_t *c, int current_epoch, if (refcount != 1) { flux_log (c->cm->h, LOG_ERR, "invalid dirref count: %d", refcount); - saved_errno = EPERM; + saved_errno = ENOTRECOVERABLE; goto done; } @@ -669,7 +669,7 @@ commit_process_t commit_process (commit_t *c, break; default: flux_log (c->cm->h, LOG_ERR, "invalid commit state: %d", c->state); - c->errnum = EPERM; + c->errnum = ENOTRECOVERABLE; return COMMIT_PROCESS_ERROR; } diff --git a/src/modules/kvs/lookup.c b/src/modules/kvs/lookup.c index cb1df58f79af..472def5e5c56 100644 --- a/src/modules/kvs/lookup.c +++ b/src/modules/kvs/lookup.c @@ -242,7 +242,7 @@ static bool walk (lookup_t *lh) if (refcount != 1) { flux_log (lh->h, LOG_ERR, "invalid dirref count: %d", refcount); - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto error; } @@ -264,7 +264,7 @@ static bool walk (lookup_t *lh) if (wl->depth == 0 && wl->dirent == lh->root_dirent) lh->errnum = EINVAL; else - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto error; } if (!treeobj_is_dir (dir)) { @@ -274,7 +274,7 @@ static bool walk (lookup_t *lh) if (wl->depth == 0 && wl->dirent == lh->root_dirent) lh->errnum = EINVAL; else - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto error; } } else { @@ -292,7 +292,7 @@ static bool walk (lookup_t *lh) "lh->path=%s pathcomp=%s: wl->dirent=%s ", __FUNCTION__, lh->path, pathcomp, s); free (s); - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto error; } } @@ -532,7 +532,7 @@ int lookup_iter_missing_refs (lookup_t *lh, lookup_ref_f cb, void *data) int refcount, i; if (!treeobj_is_valref (lh->valref_missing_refs)) { - errno = EPERM; + errno = ENOTRECOVERABLE; return -1; } @@ -654,7 +654,7 @@ static int get_single_blobref_valref_value (lookup_t *lh, bool *stall) } if (cache_entry_get_raw (hp, &valdata, &len) < 0) { flux_log (lh->h, LOG_ERR, "valref points to non-raw data"); - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; return -1; } if (!(lh->val = treeobj_create_val (valdata, len))) { @@ -688,7 +688,7 @@ static int get_multi_blobref_valref_length (lookup_t *lh, int refcount, if (cache_entry_get_raw (hp, NULL, &len) < 0) { flux_log (lh->h, LOG_ERR, "valref points to non-raw data"); - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; return -1; } @@ -819,7 +819,7 @@ bool lookup (lookup_t *lh) } if (!treeobj_is_dir (valtmp)) { /* root_ref points to not dir */ - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto done; } lh->val = json_incref (valtmp); @@ -863,7 +863,7 @@ bool lookup (lookup_t *lh) if (refcount != 1) { flux_log (lh->h, LOG_ERR, "invalid dirref count: %d", refcount); - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto done; } if (!(reftmp = treeobj_get_blobref (lh->wdirent, 0))) { @@ -877,12 +877,12 @@ bool lookup (lookup_t *lh) } if (!(valtmp = cache_entry_get_json (hp))) { flux_log (lh->h, LOG_ERR, "dirref points to non-json"); - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto done; } if (!treeobj_is_dir (valtmp)) { /* dirref points to not dir */ - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto done; } lh->val = json_incref (valtmp); @@ -904,7 +904,7 @@ bool lookup (lookup_t *lh) if (!refcount) { flux_log (lh->h, LOG_ERR, "invalid valref count: %d", refcount); - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto done; } if (refcount == 1) { @@ -957,7 +957,7 @@ bool lookup (lookup_t *lh) flux_log (lh->h, LOG_ERR, "%s: corrupt dirent: %s", __FUNCTION__, s); free (s); - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto done; } /* val now contains the requested object (copied) */ @@ -967,7 +967,7 @@ bool lookup (lookup_t *lh) default: flux_log (lh->h, LOG_ERR, "%s: invalid state %d", __FUNCTION__, lh->state); - lh->errnum = EPERM; + lh->errnum = ENOTRECOVERABLE; goto done; } diff --git a/src/modules/kvs/test/commit.c b/src/modules/kvs/test/commit.c index b0396edd06ea..1d2d1611abf7 100644 --- a/src/modules/kvs/test/commit.c +++ b/src/modules/kvs/test/commit.c @@ -1426,8 +1426,8 @@ void commit_process_bad_dirrefs (void) { ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_ERROR, "commit_process returns COMMIT_PROCESS_ERROR again"); - ok (commit_get_errnum (c) == EPERM, - "commit_get_errnum return EPERM"); + ok (commit_get_errnum (c) == ENOTRECOVERABLE, + "commit_get_errnum return ENOTRECOVERABLE"); commit_mgr_destroy (cm); cache_destroy (cache); diff --git a/src/modules/kvs/test/lookup.c b/src/modules/kvs/test/lookup.c index 0bedac915497..de0a56052183 100644 --- a/src/modules/kvs/test/lookup.c +++ b/src/modules/kvs/test/lookup.c @@ -743,7 +743,7 @@ void lookup_errors (void) { "lookup_create on valref in path"); check (lh, 0, NULL, "lookup valref in path"); - /* Lookup path w/ dir in middle, should get EPERM */ + /* Lookup path w/ dir in middle, should get ENOTRECOVERABLE */ ok ((lh = lookup_create (cache, 1, root_ref, @@ -752,7 +752,7 @@ void lookup_errors (void) { NULL, 0)) != NULL, "lookup_create on dir in path"); - check (lh, EPERM, NULL, "lookup dir in path"); + check (lh, ENOTRECOVERABLE, NULL, "lookup dir in path"); /* Lookup path w/ infinite link loop, should get ELOOP */ ok ((lh = lookup_create (cache, @@ -864,7 +864,7 @@ void lookup_errors (void) { "lookup_create on symlink"); check (lh, ENOTDIR, NULL, "lookup symlink, expecting dir"); - /* Lookup a dirref that doesn't point to a dir, should get EPERM. */ + /* Lookup a dirref that doesn't point to a dir, should get ENOTRECOVERABLE. */ ok ((lh = lookup_create (cache, 1, root_ref, @@ -873,9 +873,10 @@ void lookup_errors (void) { NULL, FLUX_KVS_READDIR)) != NULL, "lookup_create on dirref_bad"); - check (lh, EPERM, NULL, "lookup dirref_bad"); + check (lh, ENOTRECOVERABLE, NULL, "lookup dirref_bad"); - /* Lookup a dirref that doesn't point to a dir, in middle of path, should get EPERM. */ + /* Lookup a dirref that doesn't point to a dir, in middle of path, + * should get ENOTRECOVERABLE. */ ok ((lh = lookup_create (cache, 1, root_ref, @@ -884,9 +885,10 @@ void lookup_errors (void) { NULL, FLUX_KVS_READDIR)) != NULL, "lookup_create on dirref_bad, in middle of path"); - check (lh, EPERM, NULL, "lookup dirref_bad, in middle of path"); + check (lh, ENOTRECOVERABLE, NULL, "lookup dirref_bad, in middle of path"); - /* Lookup a valref that doesn't point to raw data, should get EPERM */ + /* Lookup a valref that doesn't point to raw data, should get + * ENOTRECOVERABLE */ ok ((lh = lookup_create (cache, 1, root_ref, @@ -895,10 +897,10 @@ void lookup_errors (void) { NULL, 0)) != NULL, "lookup_create on valref_bad"); - check (lh, EPERM, NULL, "lookup valref_bad"); + check (lh, ENOTRECOVERABLE, NULL, "lookup valref_bad"); /* Lookup a valref multiple blobref that doesn't point to raw - * data, should get EPERM */ + * data, should get ENOTRECOVERABLE */ ok ((lh = lookup_create (cache, 1, root_ref, @@ -907,7 +909,7 @@ void lookup_errors (void) { NULL, 0)) != NULL, "lookup_create on valref_multi_bad"); - check (lh, EPERM, NULL, "lookup valref_multi_bad"); + check (lh, ENOTRECOVERABLE, NULL, "lookup valref_multi_bad"); /* Lookup a valref multiple blobref that points to buffers that will * over int, should get EOVERFLOW. @@ -933,7 +935,7 @@ void lookup_errors (void) { "lookup_create on bad root_ref"); check (lh, EINVAL, NULL, "lookup bad root_ref"); - /* Lookup dirref with multiple blobrefs, should get EPERM */ + /* Lookup dirref with multiple blobrefs, should get ENOTRECOVERABLE */ ok ((lh = lookup_create (cache, 1, root_ref, @@ -942,10 +944,10 @@ void lookup_errors (void) { NULL, FLUX_KVS_READDIR)) != NULL, "lookup_create on dirref_multi"); - check (lh, EPERM, NULL, "lookup dirref_multi"); + check (lh, ENOTRECOVERABLE, NULL, "lookup dirref_multi"); /* Lookup path w/ dirref w/ multiple blobrefs in middle, should - * get EPERM */ + * get ENOTRECOVERABLE */ ok ((lh = lookup_create (cache, 1, root_ref, @@ -954,7 +956,7 @@ void lookup_errors (void) { NULL, 0)) != NULL, "lookup_create on dirref_multi, part of path"); - check (lh, EPERM, NULL, "lookup dirref_multi, part of path"); + check (lh, ENOTRECOVERABLE, NULL, "lookup dirref_multi, part of path"); cache_destroy (cache); }