Project

General

Profile

Bug #13715

RadosClient: result code overflowed!

Added by xie xingguo over 8 years ago. Updated over 8 years ago.

Status:
Resolved
Priority:
High
Assignee:
-
Category:
-
Target version:
-
% Done:

0%

Source:
other
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

bool librados::RadosClient::pool_requires_alignment(int64_t pool_id) {
int r = wait_for_osdmap();
if (r < 0) {
return r;
}

See above, the negative answers given by wait_for_osdmap() are directly outputted here,
which are therefore implicitly casted into bool type and may mislead caller
as we really don't know the answer yet.

The pool_required_alignment() method has the same problem:

uint64_t librados::RadosClient::pool_required_alignment(int64_t pool_id) {
int r = wait_for_osdmap();
if (r < 0) {
return r;
}

If wait_for_osdmap() fails, as shown above, pool_required_alignment() can
produce large overflowed results, which might be disastrous if they are
unconditionally trusted by caller.

The possible solutions are as follows:
(1) Make sure that wait_for_map() never fails.If it does, then retry until
it succeeds.
(2) Explicitly cast error codes into context-safe result codes.
For example:
bool librados::RadosClient::pool_requires_alignment(int64_t pool_id) {
int r = wait_for_osdmap();
if (r < 0) {
// lie to caller, we really don't know the answer yet.
return false;
}
(3) Refacter these two methods, let error codes can be safely outputed and
rightfully handled by caller.
For example:
int librados::RadosClient::pool_required_alignment(
int64_t pool_id, uint64_t *alignment) {
int r = wait_for_osdmap();
if (r < 0) {
return r;
}

const OSDMap *osdmap = objecter->get_osdmap_read();
*alignment = osdmap->have_pg_pool(pool_id) ?
osdmap->get_pg_pool(pool_id)->required_alignment() : 0;
objecter->put_osdmap_read();
return 0;
}

What's your suggestion?

Associated revisions

Revision 1f855456 (diff)
Added by xie xingguo over 8 years ago

RadosClient: result code overflowed

pool_requires_alignment and pool_required_alignment may produce overflowed results and need to refactered.
Fixes: #13715
Signed-off-by: xie xingguo <>

Revision 3b146f59 (diff)
Added by xie xingguo over 8 years ago

RadosClient: reimplement the pool alignment methods using the new ones

so we don't less duplicated code and the logic reveals the problem of the old methods in a more visible way.
Fixes: #13715
Signed-off-by: xie xingguo <>

History

#1 Updated by xie xingguo over 8 years ago

BTW, RGWRados::get_required_alignment() shall test ioctx.pool_requires_alignment() first,
otherwise the answer is untrustful and might not be rightfully handled by caller.

#2 Updated by Sage Weil over 8 years ago

  • Status changed from New to Resolved

Also available in: Atom PDF