Skip to content
This repository has been archived by the owner on Dec 16, 2020. It is now read-only.

Use of absl::string_view in WasmState to store data leads to a dangling pointer and crash of Envoy #561

Closed
yskopets opened this issue Jul 2, 2020 · 2 comments

Comments

@yskopets
Copy link
Member

yskopets commented Jul 2, 2020

Summary

  • At the moment, WasmState class uses absl::string_view to store some of its data (namely, schema_)
    // A simple wrapper around generic values
    class WasmState : public StreamInfo::FilterState::Object {
    public:
      explicit WasmState(const WasmStatePrototype& proto)
          : readonly_(proto.readonly_), type_(proto.type_), schema_(proto.schema_) {}
    
    ...
    
    private:
      absl::string_view schema_;
    };
  • It's probably done in assumption that WasmState will always outlast the WasmStatePrototype it's created from
  • However, it's not guaranteed
  • The issue is reproducible on Istio 1.6.1 where use of envoy.wasm.metadata_exchange filter causes removal of WasmStatePrototype, consequent dangling pointer in WasmState and, finally, crash of Envoy

How to reproduce

The issue is consistently reproducible under the following scenario

  • Use Istio 1.6.1
  • Deploy Bookinfo application
  • HTTP listeners in Envoy sidecars will be configured with envoy.wasm.metadata_exchange and envoy.wasm.stats filters, namely:
               {
                "name": "istio.metadata_exchange",
                "typed_config": {
                 "@type": "type.googleapis.com/udpa.type.v1.TypedStruct",
                 "type_url": "type.googleapis.com/envoy.extensions.filters.http.wasm.v3.Wasm",
                 "value": {
                  "config": {
                   "vm_config": {
                    "runtime": "envoy.wasm.runtime.null",
                    "code": {
                     "local": {
                      "inline_string": "envoy.wasm.metadata_exchange"
                     }
                    }
                   },
                   "configuration": "{}\n"
                  }
                 }
                }
               },
    and
               {
                "name": "istio.stats",
                "typed_config": {
                 "@type": "type.googleapis.com/udpa.type.v1.TypedStruct",
                 "type_url": "type.googleapis.com/envoy.extensions.filters.http.wasm.v3.Wasm",
                 "value": {
                  "config": {
                   "root_id": "stats_outbound",
                   "vm_config": {
                    "vm_id": "stats_outbound",
                    "runtime": "envoy.wasm.runtime.null",
                    "code": {
                     "local": {
                      "inline_string": "envoy.wasm.stats"
                     }
                    }
                   },
                   "configuration": "{\n  \"debug\": \"false\",\n  \"stat_prefix\": \"istio\",\n  \"metrics\": [\n    {\n      \"dimensions\": {\n        \"source_cluster\": \"node.metadata['CLUSTER_ID']\",\n        \"destination_cluster\": \"upstream_peer.cluster_id\"\n      }\n    }\n  ]\n}\n"
                  }
                 }
                }
               },
  • Start making requests to the Product Page
  • Update Istio configuration to cause update of a Listener inside the sidecar alongside the Product Page app
  • Envoy sidecar alongside the Product Page app will crash with
    #0: Envoy::SignalAction::sigHandler() [0x5585c6005e6c]
    #1: __restore_rt [0x7f9e2f3dd890]
    #2: flatbuffers::Table::GetVTable() [0x5585c24d865c]
    #3: flatbuffers::Table::GetOptionalFieldOffset() [0x5585c24d85b9]
    #4: flatbuffers::Table::GetPointer<>() [0x5585c3cca334]
    #5: flatbuffers::Table::GetPointer<>() [0x5585c3cca2fd]
    #6: reflection::Schema::root_table() [0x5585c3cc945d]
    #7: google::api::expr::runtime::CreateFlatBuffersBackedObject() [0x5585c3cc5f1b]
    #8: Envoy::Extensions::Common::Wasm::WasmState::exprValue() [0x5585c3cc1952]
    #9: Envoy::Extensions::Common::Wasm::Context::findValue() [0x5585c3c19c0b]
    #10: Envoy::Extensions::Common::Wasm::Context::FindValue() [0x5585c3c31c67]
    #11: Envoy::Extensions::Common::Wasm::Context::FindValue() [0x5585c3c31e76]
    #12: google::api::expr::runtime::(anonymous namespace)::IdentStep::DoEvaluate() [0x5585c4a621c6]
    #13: google::api::expr::runtime::(anonymous namespace)::IdentStep::Evaluate() [0x5585c4a620a4]
    #14: google::api::expr::runtime::CelExpressionFlatImpl::Trace() [0x5585c4a693ca]
    #15: google::api::expr::runtime::CelExpressionFlatImpl::Evaluate() [0x5585c4a69175]
    #16: google::api::expr::runtime::CelExpressionFlatImpl::Evaluate() [0x5585c4a6a529]
    #17: Envoy::Extensions::Common::Wasm::EvaluateExpressionFactory::create()::{lambda()#1}::operator()() [0x5585c3c841d6]
    #18: std::__1::__invoke<>() [0x5585c3c84059] 
    #19: std::__1::__invoke_void_return_wrapper<>::__call<>() [0x5585c3c83f8c]
    #20: std::__1::__function::__alloc_func<>::operator()() [0x5585c3c83f1c]
    #21: std::__1::__function::__func<>::operator()() [0x5585c3c8303d]
    #22: std::__1::__function::__value_func<>::operator()() [0x5585c3c72792]
    #23: std::__1::function<>::operator()() [0x5585c3c7181e]
    #24: Envoy::Extensions::Common::Wasm::Exports::call_foreign_function() [0x5585c3c66f5a]
    #25: Envoy::Extensions::Common::Wasm::Null::Plugin::proxy_call_foreign_function() [0x5585c24a69a2]
    #26: Envoy::Extensions::Common::Wasm::Null::Plugin::exprEvaluate() [0x5585c26b5497]
    #27: Envoy::Extensions::Common::Wasm::Null::Plugin::evaluateExpression<>() [0x5585c26b364b]
    #28: Envoy::Extensions::Common::Wasm::Null::Plugin::Stats::PluginRootContext::report() [0x5585c2693b58]
    #29: Envoy::Extensions::Common::Wasm::Null::Plugin::Stats::PluginContext::onLog() [0x5585c26b8161]
    #30: Envoy::Extensions::Common::Wasm::Null::NullPlugin::onLog() [0x5585c27fc799]
    #31: Envoy::Extensions::Common::Wasm::Null::NullPlugin::getFunction()::$_2::operator()() [0x5585c2800a39]
    #32: std::__1::__invoke<>() [0x5585c28009e2]
    #33: std::__1::__invoke_void_return_wrapper<>::__call<>() [0x5585c2800967]
    #34: std::__1::__function::__alloc_func<>::operator()() [0x5585c2800917]
    #35: std::__1::__function::__func<>::operator()() [0x5585c27ffa48]
    #36: std::__1::__function::__value_func<>::operator()() [0x5585c3c5c41a]
    #37: std::__1::function<>::operator()() [0x5585c3c30cb5]
    #38: Envoy::Extensions::Common::Wasm::Context::onLog() [0x5585c3c251cc]
    #39: Envoy::Extensions::Common::Wasm::Context::log() [0x5585c3c24f8c]
    #40: Envoy::Http::ConnectionManagerImpl::ActiveStream::~ActiveStream() [0x5585c5a762b8]
    #41: Envoy::Http::ConnectionManagerImpl::ActiveStream::~ActiveStream() [0x5585c5a76b33]
    #42: Envoy::Http::ConnectionManagerImpl::ActiveStream::~ActiveStream() [0x5585c5a76c2c]
    #43: std::__1::default_delete<>::operator()() [0x5585c2bdf34f]
    #44: std::__1::unique_ptr<>::reset() [0x5585c2bdf2cf]
    #45: Envoy::Event::DispatcherImpl::clearDeferredDeleteList() [0x5585c53ca9fe]
    #46: Envoy::Event::DispatcherImpl::DispatcherImpl()::$_0::operator()() [0x5585c53cd50c]
    #47: std::__1::__invoke<>() [0x5585c53cd4cd]
    #48: std::__1::__invoke_void_return_wrapper<>::__call<>() [0x5585c53cd47d]
    #49: std::__1::__function::__alloc_func<>::operator()() [0x5585c53cd44d]
    #50: std::__1::__function::__func<>::operator()() [0x5585c53cc59e]
    #51: std::__1::__function::__value_func<>::operator()() [0x5585c25c9b85]
    #52: std::__1::function<>::operator()() [0x5585c25c9b45]
    #53: Envoy::Event::TimerImpl::TimerImpl()::$_0::operator()() [0x5585c540b0dd]
    #54: Envoy::Event::TimerImpl::TimerImpl()::$_0::__invoke() [0x5585c540b086]
    #55: event_process_active_single_queue [0x5585c5ef05d1]
    #56: event_process_active [0x5585c5eeacca]
    #57: event_base_loop [0x5585c5ee9b9c]
    #58: Envoy::Event::LibeventScheduler::run() [0x5585c5409ca8]
    #59: Envoy::Event::DispatcherImpl::run() [0x5585c53cbe7a]
    #60: Envoy::Server::WorkerImpl::threadRoutine() [0x5585c539eca1]
    #61: Envoy::Server::WorkerImpl::start()::$_4::operator()() [0x5585c53a614c]
    #62: std::__1::__invoke<>() [0x5585c53a610d]
    #63: std::__1::__invoke_void_return_wrapper<>::__call<>() [0x5585c53a60bd]
    

Cause

  • the issue is caused by the dangling pointer in WasmState object
  • WasmStatePrototype and WasmState objects get created in envoy.wasm.metadata_exchange filter
  • WasmState object is being used in envoy.wasm.stats filter
  • envoy.wasm.stats filter gets called on a different cycle of the Envoy Dispatcher loop, which leaves a room for WasmState object to become invalid
  • WasmState object become invalid when WasmStatePrototype it was created from gets removed
  • apparently, envoy.wasm.metadata_exchange filter recreates the WasmStatePrototype object on every call to onConfigure() callback, which happens on add/update to any HTTP Listener in the sidecar
  • changes done as part of onConfigure() in envoy.wasm.metadata_exchange filter affect all in-inflight requests on every HTTP listener, which increases the chances that there is a WasmState object that will become invalid
@yskopets
Copy link
Member Author

yskopets commented Jul 2, 2020

While troubleshooting the issue, I've prepared a patch where WasmState is re-worked to hold a std::shared_ptr<const WasmStatePrototype> instead of absl::string_view.

Let me know if you think it's a proper solution and I should open a PR.

@bianpengyuan
Copy link
Contributor

It seems to be the same issue fixed by #547? It should be included in the next patch release (1.6.5).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants