Skip to content

Commit

Permalink
modules/kvs: Replace EPERM w/ ENOTRECOVERABLE
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@chu11
chu11 committed Oct 17, 2017
1 parent eebb5e7 commit d3fe8e2
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 33 deletions.
6 changes: 3 additions & 3 deletions src/modules/kvs/commit.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
28 changes: 14 additions & 14 deletions src/modules/kvs/lookup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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)) {
Expand All @@ -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 {
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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))) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))) {
Expand All @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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) */
Expand All @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/modules/kvs/test/commit.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
30 changes: 16 additions & 14 deletions src/modules/kvs/test/lookup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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.
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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);
}
Expand Down

0 comments on commit d3fe8e2

Please sign in to comment.