New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rbd: null check before pool_name use #18790
Conversation
jenkins retest this please |
src/tools/rbd/Utils.cc
Outdated
if (!boost::regex_match (*pool_name, pattern)) { | ||
std::cerr << "rbd: invalid pool name '" << *pool_name << "'" << std::endl; | ||
return -EINVAL; | ||
if (pool_name != nullptr && pool_name->empty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you checking that the pool_name
is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dillaman Thanks for review.
since pool_name is never checked for NULL in function before. Either we should check pool_name to be nonnull initially and return.
But before passing pool_name to regex_match() API we should check its nonnull.
Coverity raised the Issue, Please have a look at Coverity ID:1394846
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That wasn't my question -- I understand and agree w/ the nullptr check (even though in practice it will never be hit). I asked why did you also insert && pool_name->empty()
since if the pool name is empty, what's the point of validating the reg ex?
f0ed9cf
to
7384bb2
Compare
@dillaman Do you want any other change to be done? |
src/tools/rbd/Utils.cc
Outdated
if (!boost::regex_match (*pool_name, pattern)) { | ||
std::cerr << "rbd: invalid pool name '" << *pool_name << "'" << std::endl; | ||
return -EINVAL; | ||
if (pool_name != nullptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: just combine the two if
statements into a single line
7384bb2
to
55c2572
Compare
@dillaman Done Changes. Thanks. |
Fixes the coverity issue: CID 1394846 (ceph#1 of 1): Dereference after null check (FORWARD_NULL) 15. var_deref_model: Passing null pointer pool_name to regex_match, which dereferences it. Signed-off-by: Amit Kumar <amitkuma@redhat.com>
Fixes the coverity issue:
CID 1394846 (#1 of 1): Dereference after null check (FORWARD_NULL)
Signed-off-by: Amit Kumar amitkuma@redhat.com