Skip to content
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.

Sign up for GitHub

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

Jump to bottom

rgw: Fix use after free in IAM policy parser #16823

Merged
merged 3 commits into from Aug 7, 2017
Merged

rgw: Fix use after free in IAM policy parser #16823

merged 3 commits into from Aug 7, 2017

Conversation

adamemerson
Copy link
Contributor

No description provided.

@wjwithagen
Copy link
Contributor

@adamemerson
Nope, Still same crash...

Thread 1 received signal SIGSEGV, Segmentation fault.
0x000000000062441d in std::__1::__tree_is_left_child<std::__1::__tree_node_base<void*>*> (
    __x=0x813158138) at /usr/include/c++/v1/__tree:78
78          return __x == __x->__parent_->__left_;

(gdb) bt
#0  0x000000000062441d in std::__1::__tree_is_left_child<std::__1::__tree_node_base<void*>*> (
    __x=0x813158138) at /usr/include/c++/v1/__tree:78
#1  std::__1::__tree_next_iter<std::__1::__tree_end_node<std::__1::__tree_node_base<void*>*>*, std::__1::__tree_node_base<void*>*> (__x=0x813158138) at /usr/include/c++/v1/__tree:181
#2  std::__1::__tree_const_iterator<rgw::IAM::TokenID, std::__1::__tree_node<rgw::IAM::TokenID, void*>*, long>::operator++ (this=0x7fffffffb288) at /usr/include/c++/v1/__tree:915
#3  rgw::IAM::PolicyParser::reset (this=0x7fffffffbaf8, v=...)
    at /home/jenkins/workspace/ceph-master/src/rgw/rgw_iam_policy.cc:596
#4  0x00000000006143d9 in rgw::IAM::ParseState::reset (this=0x813158138)
    at /home/jenkins/workspace/ceph-master/src/rgw/rgw_iam_policy.cc:872
#5  0x0000000000613f30 in rgw::IAM::ParseState::obj_end (this=0x813158138)
    at /home/jenkins/workspace/ceph-master/src/rgw/rgw_iam_policy.cc:670
#6  0x000000000066222c in rgw::IAM::PolicyParser::EndObject (this=0x7fffffffbaf8, memberCount=4)
    at /home/jenkins/workspace/ceph-master/src/rgw/rgw_iam_policy.cc:619
#7  0x000000000065f2e1 in rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::ParseObject<96u, rapidjson::GenericStringStream<rapidjson::UTF8<char> >, rgw::IAM::PolicyParser> (this=0x7fffffffbaa8, is=..., handler=...)
    at /home/jenkins/workspace/ceph-master/src/rgw/../rapidjson/include/rapidjson/reader.h:637
#8  0x000000000065e200 in rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::ParseValue<96u, rapidjson::GenericStringStream<rapidjson::UTF8<char> >, rgw::IAM::PolicyParser> (this=0x7fffffffbaa8, is=..., handler=...)
    at /home/jenkins/workspace/ceph-master/src/rgw/../rapidjson/include/rapidjson/reader.h:1398
#9  0x000000000065f63c in rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::ParseArray<96u, rapidjson::GenericStringStream<rapidjson::UTF8<char> >, rgw::IAM::PolicyParser> (this=0x7fffffffbaa8, is=..., handler=...)
    at /home/jenkins/workspace/ceph-master/src/rgw/../rapidjson/include/rapidjson/reader.h:674
#10 0x000000000065e216 in rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::ParseValue<96u, rapidjson::GenericStringStream<rapidjson::UTF8<char> >, rgw::IAM::PolicyParser> (this=0x7fffffffbaa8, is=..., handler=...)
    at /home/jenkins/workspace/ceph-master/src/rgw/../rapidjson/include/rapidjson/reader.h:1399
#11 0x000000000065f1f4 in rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::ParseObject<96u, rapidjson::GenericStringStream<rapidjson::UTF8<char> >, rgw::IAM::PolicyParser> (this=0x7fffffffbaa8, is=..., handler=...)
    at /home/jenkins/workspace/ceph-master/src/rgw/../rapidjson/include/rapidjson/reader.h:621
#12 0x000000000065e200 in rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::ParseValue<96u, rapidjson::GenericStringStream<rapidjson::UTF8<char> >, rgw::IAM::PolicyParser> (this=0x7fffffffbaa8, is=..., handler=...)
    at /home/jenkins/workspace/ceph-master/src/rgw/../rapidjson/include/rapidjson/reader.h:1398
#13 0x000000000062abc2 in rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::Parse<96u, rapidjson::GenericStringStream<rapidjson::UTF8<char> >, rgw::IAM::PolicyParser> (this=0x7fffffffbaa8, is=..., handler=...)
    at /home/jenkins/workspace/ceph-master/src/rgw/../rapidjson/include/rapidjson/reader.h:501
#14 0x000000000061fb32 in rgw::IAM::Policy::Policy (this=0x7fffffffd270, cct=0x812ee4780,
    tenant=..., _text=...) at /home/jenkins/workspace/ceph-master/src/rgw/rgw_iam_policy.cc:1530
#15 0x00000000004a48b8 in PolicyTest_Parse3_Test::TestBody (this=0x812e3d040)
    at /home/jenkins/workspace/ceph-master/src/test/rgw/test_rgw_iam_policy.cc:264
#16 0x000000000056510e in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ()
    at /usr/include/c++/v1/__string:226
#17 0x000000000054544b in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) () at /usr/include/c++/v1/__string:226
#18 0x0000000000500f26 in testing::Test::Run() () at /usr/include/c++/v1/__string:226
#19 0x000000000050366d in testing::TestInfo::Run() () at /usr/include/c++/v1/__string:226
#20 0x000000000050492c in testing::TestCase::Run() () at /usr/include/c++/v1/__string:226
#21 0x000000000051b7ec in testing::internal::UnitTestImpl::RunAllTests() ()
    at /usr/include/c++/v1/__string:226
#22 0x0000000000568e4e in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) () at /usr/include/c++/v1/__string:226
#23 0x000000000054848b in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) () at /usr/include/c++/v1/__string:226
#24 0x000000000051b3eb in testing::UnitTest::Run() () at /usr/include/c++/v1/__string:226
#25 0x000000000056f611 in RUN_ALL_TESTS ()
    at /home/jenkins/workspace/ceph-master/src/googletest/googletest/include/gtest/gtest.h:2233
#26 0x000000000056f5f5 in main (argc=1, argv=0x7fffffffea50)
    at /home/jenkins/workspace/ceph-master/src/googletest/googlemock/src/gmock_main.cc:53

(gdb) p * __x
$2 = {<std::__1::__tree_end_node<std::__1::__tree_node_base<void*>*>> = {__left_ = 0x7fffffffbaf8},
  __right_ = 0x8d2920 <rgw::IAM::keyword_hash::lookup(char const*, unsigned int)::wordlist+288>,
  __parent_ = 0x1, __is_black_ = 248}

So the fact that parent = 0x1 is causing the segmentation trap.

And the reset code seems to be:

  void reset(std::set<TokenID> v) {
    for (auto in : v) {
      seen &= ~dex(in);
      v.erase(in);
    }
  }

@mattbenjamin
Copy link
Contributor

mattbenjamin commented Aug 7, 2017 •
edited

@adamemerson cf this: http://qa-proxy.ceph.com/teuthology/yuriw-2017-08-06_21:30:50-rgw-wip-yuri-master_2017_8_7-distro-basic-smithi/1490915/remote/smithi137/log/valgrind/ceph.client.0.log.gz

this looks similar: http://qa-proxy.ceph.com/teuthology/yuriw-2017-08-06_21:30:50-rgw-wip-yuri-master_2017_8_7-distro-basic-smithi/1490920/remote/smithi136/log/valgrind/ceph.client.0.log.gz

this looks similar: http://qa-proxy.ceph.com/teuthology/yuriw-2017-08-06_21:30:50-rgw-wip-yuri-master_2017_8_7-distro-basic-smithi/1490928/remote/smithi011/log/valgrind/ceph.client.0.log.gz

this looks similar: http://qa-proxy.ceph.com/teuthology/yuriw-2017-08-06_21:30:50-rgw-wip-yuri-master_2017_8_7-distro-basic-smithi/1490934/remote/smithi144/log/valgrind/ceph.client.0.log.gz

@adamemerson
Copy link
Contributor Author

@wjwithagen Do you have d02db19 in the branch you're testing? It was recently committed to master.

Also!

Can you send me a valgrind trace? Something with --track-origins=yes and --leak-check=no would help.

Thank you.

@adamemerson
rgw: Fix use after free in IAM policy parser
8377ba6 
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson
rgw: Fix another use after free
97d026d 
This one was caused by iterator invalidation in set operations. In
this case just replace the set entirely with a bitfield.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson
rgw: Fix the last policy use-after-free
5353d95 
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson
Copy link
Contributor Author

@wjwithagen @mattbenjamin All right! I think this latest push should fix all the problems.

@adamemerson adamemerson added this to the luminous milestone Aug 7, 2017
mattbenjamin
mattbenjamin reviewed Aug 7, 2017
View reviewed changes
src/rgw/rgw_iam_policy.cc
@@ -507,7 +507,7 @@ struct PolicyParser : public BaseReaderHandler<UTF8<>, PolicyParser> {
CephContext* cct;
const string& tenant;
Policy& policy;
std::set<TokenID> v;
uint32_t v = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32 tokens is not many

wjwithagen
wjwithagen approved these changes Aug 7, 2017
View reviewed changes
Copy link
Contributor

@wjwithagen wjwithagen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamemerson
With the latest changes the test completes on FreeBSD.

@mattbenjamin
Copy link
Contributor

http://pulpito.ceph.com/mbenjamin-2017-08-07_19:00:52-rgw-wip-rgw-policy-uaf---basic-smithi/

@mattbenjamin mattbenjamin merged commit 94883e0 into ceph:master Aug 7, 2017
@adamemerson adamemerson deleted the wip-use-after-free branch October 29, 2017 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattbenjamin mattbenjamin mattbenjamin left review comments

@wjwithagen wjwithagen wjwithagen approved these changes

Assignees

@adamemerson adamemerson

Labels
bug-fix rgw
Projects
None yet
Milestone
luminous
3 participants
@adamemerson @wjwithagen @mattbenjamin