From a0960562637148de0bea886e34e764e2a5f38c3f Mon Sep 17 00:00:00 2001 From: Sandboxed API Team Date: Wed, 1 Dec 2021 09:54:09 -0800 Subject: [PATCH] Automated rollback of commit b72078f692b6e3235db3b83f8df7436fa8a6127c. PiperOrigin-RevId: 413442229 Change-Id: I48d03ce200160da1c86faec29b2ca51fb1ead834 --- sandboxed_api/BUILD.bazel | 2 - sandboxed_api/CMakeLists.txt | 2 - sandboxed_api/var_abstract.cc | 33 ++++++++++++ sandboxed_api/var_abstract.h | 40 ++++++++++++++- sandboxed_api/var_array.h | 11 ++-- sandboxed_api/var_int.h | 17 +++--- sandboxed_api/var_lenval.h | 12 ++--- sandboxed_api/var_pointable.cc | 22 -------- sandboxed_api/var_pointable.h | 94 ---------------------------------- sandboxed_api/var_proto.h | 11 ++-- sandboxed_api/var_ptr.h | 8 +-- sandboxed_api/var_reg.h | 6 +-- sandboxed_api/var_struct.h | 12 ++--- sandboxed_api/var_void.h | 10 +--- sandboxed_api/vars.h | 1 - 15 files changed, 105 insertions(+), 176 deletions(-) delete mode 100644 sandboxed_api/var_pointable.cc delete mode 100644 sandboxed_api/var_pointable.h diff --git a/sandboxed_api/BUILD.bazel b/sandboxed_api/BUILD.bazel index 8dc47f2..e02db84 100644 --- a/sandboxed_api/BUILD.bazel +++ b/sandboxed_api/BUILD.bazel @@ -131,7 +131,6 @@ cc_library( "var_abstract.cc", "var_int.cc", "var_lenval.cc", - "var_pointable.cc", ], hdrs = [ "proto_helper.h", @@ -140,7 +139,6 @@ cc_library( "var_array.h", "var_int.h", "var_lenval.h", - "var_pointable.h", "var_proto.h", "var_ptr.h", "var_reg.h", diff --git a/sandboxed_api/CMakeLists.txt b/sandboxed_api/CMakeLists.txt index ab94d65..7aab157 100644 --- a/sandboxed_api/CMakeLists.txt +++ b/sandboxed_api/CMakeLists.txt @@ -139,8 +139,6 @@ add_library(sapi_vars ${SAPI_LIB_TYPE} var_int.h var_lenval.cc var_lenval.h - var_pointable.cc - var_pointable.h var_proto.h var_ptr.h var_reg.h diff --git a/sandboxed_api/var_abstract.cc b/sandboxed_api/var_abstract.cc index 812691c..e60a930 100644 --- a/sandboxed_api/var_abstract.cc +++ b/sandboxed_api/var_abstract.cc @@ -18,11 +18,14 @@ #include +#include + #include #include "absl/strings/str_cat.h" #include "sandboxed_api/rpcchannel.h" #include "sandboxed_api/sandbox2/comms.h" #include "sandboxed_api/util/status_macros.h" +#include "sandboxed_api/var_ptr.h" namespace sapi::v { @@ -32,6 +35,36 @@ Var::~Var() { } } +void Var::PtrDeleter::operator()(Ptr* p) { delete p; } + +Ptr* Var::PtrNone() { + if (!ptr_none_) { + ptr_none_.reset(new Ptr(this, kSyncNone)); + } + return ptr_none_.get(); +} + +Ptr* Var::PtrBoth() { + if (!ptr_both_) { + ptr_both_.reset(new Ptr(this, kSyncBoth)); + } + return ptr_both_.get(); +} + +Ptr* Var::PtrBefore() { + if (!ptr_before_) { + ptr_before_.reset(new Ptr(this, kSyncBefore)); + } + return ptr_before_.get(); +} + +Ptr* Var::PtrAfter() { + if (!ptr_after_) { + ptr_after_.reset(new Ptr(this, kSyncAfter)); + } + return ptr_after_.get(); +} + absl::Status Var::Allocate(RPCChannel* rpc_channel, bool automatic_free) { void* addr; SAPI_RETURN_IF_ERROR(rpc_channel->Allocate(GetSize(), &addr)); diff --git a/sandboxed_api/var_abstract.h b/sandboxed_api/var_abstract.h index 05dcf34..8142fcc 100644 --- a/sandboxed_api/var_abstract.h +++ b/sandboxed_api/var_abstract.h @@ -19,6 +19,7 @@ #include #include +#include "absl/base/attributes.h" #include "absl/base/macros.h" #include "absl/status/status.h" #include "sandboxed_api/var_type.h" @@ -36,8 +37,29 @@ namespace sapi::v { class Ptr; +class ABSL_DEPRECATED( + "Use the Var::PtrXXX() family of methods instead") Pointable { + public: + enum SyncType { + // Do not synchronize the underlying object after/before calls. + kSyncNone = 0x0, + // Synchronize the underlying object (send the data to the sandboxee) + // before the call takes place. + kSyncBefore = 0x1, + // Synchronize the underlying object (retrieve data from the sandboxee) + // after the call has finished. + kSyncAfter = 0x2, + // Synchronize the underlying object with the remote object, by sending the + // data to the sandboxee before the call, and retrieving it from the + // sandboxee after the call has finished. + kSyncBoth = kSyncBefore | kSyncAfter, + }; + + virtual ~Pointable() = default; +}; + // An abstract class representing variables. -class Var { +class Var : public Pointable { public: Var(const Var&) = delete; Var& operator=(const Var&) = delete; @@ -68,6 +90,12 @@ class Var { protected: Var() = default; + // Functions to get pointers with certain type of synchronization schemes. + Ptr* PtrNone(); + Ptr* PtrBoth(); + Ptr* PtrBefore(); + Ptr* PtrAfter(); + // Set pointer to local storage class. void SetLocal(void* local) { local_ = local; } @@ -96,9 +124,19 @@ class Var { pid_t pid); private: + // Needed so that we can use unique_ptr with incomplete type. + struct PtrDeleter { + void operator()(Ptr* p); + }; + // Invokes Allocate()/Free()/Transfer*Sandboxee(). friend class ::sapi::Sandbox; + std::unique_ptr ptr_none_; + std::unique_ptr ptr_both_; + std::unique_ptr ptr_before_; + std::unique_ptr ptr_after_; + // Pointer to local storage of the variable. void* local_ = nullptr; // Pointer to remote storage of the variable. diff --git a/sandboxed_api/var_array.h b/sandboxed_api/var_array.h index 8438343..498dd43 100644 --- a/sandboxed_api/var_array.h +++ b/sandboxed_api/var_array.h @@ -27,15 +27,18 @@ #include "sandboxed_api/rpcchannel.h" #include "sandboxed_api/util/status_macros.h" #include "sandboxed_api/var_abstract.h" -#include "sandboxed_api/var_pointable.h" #include "sandboxed_api/var_ptr.h" namespace sapi::v { // Class representing an array. template -class Array : public Var, public Pointable { +class Array : public Var { public: + using Var::PtrAfter; + using Var::PtrBefore; + using Var::PtrBoth; + // The array is not owned by this object. Array(T* arr, size_t nelem) : arr_(arr), @@ -96,10 +99,6 @@ class Array : public Var, public Pointable { private: friend class LenVal; - Ptr* CreatePtr(Pointable::SyncType type) override { - return new Ptr(this, type); - } - // Resizes the internal storage. absl::Status EnsureOwnedLocalBuffer(size_t size) { if (size % sizeof(T)) { diff --git a/sandboxed_api/var_int.h b/sandboxed_api/var_int.h index 935f533..a5ba587 100644 --- a/sandboxed_api/var_int.h +++ b/sandboxed_api/var_int.h @@ -18,24 +18,21 @@ #include #include "sandboxed_api/sandbox2/comms.h" -#include "sandboxed_api/var_pointable.h" #include "sandboxed_api/var_ptr.h" #include "sandboxed_api/var_reg.h" namespace sapi::v { -// Intermediate class for register sized variables -// so we don't have to implement ptr() everywhere. +// Intermediate class for register sized variables so we don't have to implement +// ptr() everywhere. template -class IntBase : public Reg, public Pointable { +class IntBase : public Reg { public: - IntBase() : IntBase(static_cast(0)) {} - explicit IntBase(T value) { this->SetValue(value); } + using Var::PtrAfter; + using Var::PtrBefore; + using Var::PtrBoth; - private: - Ptr* CreatePtr(Pointable::SyncType type) override { - return new Ptr(this, type); - } + explicit IntBase(T value = {}) { this->SetValue(value); } }; using Bool = IntBase; diff --git a/sandboxed_api/var_lenval.h b/sandboxed_api/var_lenval.h index 8b2a207..3047796 100644 --- a/sandboxed_api/var_lenval.h +++ b/sandboxed_api/var_lenval.h @@ -23,7 +23,6 @@ #include "sandboxed_api/lenval_core.h" #include "sandboxed_api/var_abstract.h" #include "sandboxed_api/var_array.h" -#include "sandboxed_api/var_pointable.h" #include "sandboxed_api/var_ptr.h" #include "sandboxed_api/var_struct.h" @@ -36,8 +35,12 @@ class Proto; // sandboxee which allows the bidirectional synchronization data structures with // changing lengths (e.g. protobuf structures). You probably want to directly // use protobufs as they are easier to handle. -class LenVal : public Var, public Pointable { +class LenVal : public Var { public: + using Var::PtrAfter; + using Var::PtrBefore; + using Var::PtrBoth; + explicit LenVal(const char* data, uint64_t size) : array_(const_cast(reinterpret_cast(data)), size), @@ -54,10 +57,6 @@ class LenVal : public Var, public Pointable { std::string GetTypeString() const final { return "LengthValue"; } std::string ToString() const final { return "LenVal"; } - Ptr* CreatePtr(Pointable::SyncType type) override { - return new Ptr(this, type); - } - absl::Status ResizeData(RPCChannel* rpc_channel, size_t size); size_t GetDataSize() const { return struct_.data().size; } uint8_t* GetData() const { return array_.GetData(); } @@ -65,6 +64,7 @@ class LenVal : public Var, public Pointable { protected: size_t GetSize() const final { return 0; } + absl::Status Allocate(RPCChannel* rpc_channel, bool automatic_free) override; absl::Status Free(RPCChannel* rpc_channel) override; absl::Status TransferToSandboxee(RPCChannel* rpc_channel, pid_t pid) override; diff --git a/sandboxed_api/var_pointable.cc b/sandboxed_api/var_pointable.cc deleted file mode 100644 index 06df9ac..0000000 --- a/sandboxed_api/var_pointable.cc +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2019 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "sandboxed_api/var_pointable.h" -#include "sandboxed_api/var_ptr.h" - -namespace sapi::v { - -void Pointable::PtrDeleter::operator()(Ptr *p) { delete p; } - -} // namespace sapi::v diff --git a/sandboxed_api/var_pointable.h b/sandboxed_api/var_pointable.h deleted file mode 100644 index 6c74c5b..0000000 --- a/sandboxed_api/var_pointable.h +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright 2019 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef SANDBOXED_API_VAR_POINTABLE_H_ -#define SANDBOXED_API_VAR_POINTABLE_H_ - -#include - -#include "sandboxed_api/var_reg.h" - -namespace sapi::v { - -class Ptr; - -// Class that implements pointer support for different objects. -class Pointable { - public: - enum SyncType { - // Do not synchronize the underlying object after/before calls. - kSyncNone = 0x0, - // Synchronize the underlying object (send the data to the sandboxee) - // before the call takes place. - kSyncBefore = 0x1, - // Synchronize the underlying object (retrieve data from the sandboxee) - // after the call has finished. - kSyncAfter = 0x2, - // Synchronize the underlying object with the remote object, by sending the - // data to the sandboxee before the call, and retrieving it from the - // sandboxee after the call has finished. - kSyncBoth = kSyncBefore | kSyncAfter, - }; - - virtual ~Pointable() = default; - - // Functions to get pointers with certain type of synchronization scheme. - Ptr* PtrNone() { - if (ptr_none_ == nullptr) { - ptr_none_.reset(CreatePtr(kSyncNone)); - } - - return ptr_none_.get(); - } - - Ptr* PtrBoth() { - if (ptr_both_ == nullptr) { - ptr_both_.reset(CreatePtr(kSyncBoth)); - } - return ptr_both_.get(); - } - - Ptr* PtrBefore() { - if (ptr_before_ == nullptr) { - ptr_before_.reset(CreatePtr(kSyncBefore)); - } - return ptr_before_.get(); - } - - Ptr* PtrAfter() { - if (ptr_after_ == nullptr) { - ptr_after_.reset(CreatePtr(kSyncAfter)); - } - return ptr_after_.get(); - } - - private: - // Needed so that we can use unique_ptr with incomplete type. - struct PtrDeleter { - void operator()(Ptr* p); - }; - - // Necessary to implement creation of Ptr in inheriting class as it is - // incomplete type here. - virtual Ptr* CreatePtr(SyncType sync_type) = 0; - - std::unique_ptr ptr_none_; - std::unique_ptr ptr_both_; - std::unique_ptr ptr_before_; - std::unique_ptr ptr_after_; -}; - -} // namespace sapi::v - -#endif // SANDBOXED_API_VAR_POINTABLE_H_ diff --git a/sandboxed_api/var_proto.h b/sandboxed_api/var_proto.h index 9893f95..fc81713 100644 --- a/sandboxed_api/var_proto.h +++ b/sandboxed_api/var_proto.h @@ -28,17 +28,20 @@ #include "sandboxed_api/proto_helper.h" #include "sandboxed_api/util/status_macros.h" #include "sandboxed_api/var_lenval.h" -#include "sandboxed_api/var_pointable.h" #include "sandboxed_api/var_ptr.h" namespace sapi::v { template -class Proto : public Pointable, public Var { +class Proto : public Var { public: static_assert(std::is_base_of::value, "Template argument must be a proto message"); + using Var::PtrAfter; + using Var::PtrBefore; + using Var::PtrBoth; + ABSL_DEPRECATED("Use Proto<>::FromMessage() instead") explicit Proto(const T& proto) : wrapped_var_(SerializeProto(proto).value()) {} @@ -103,10 +106,6 @@ class Proto : public Pointable, public Var { explicit Proto(std::vector data) : wrapped_var_(std::move(data)) {} - Ptr* CreatePtr(Pointable::SyncType type) override { - return new Ptr(this, type); - } - // The management of reading/writing the data to the sandboxee is handled by // the LenVal class. LenVal wrapped_var_; diff --git a/sandboxed_api/var_ptr.h b/sandboxed_api/var_ptr.h index 5dcbfd6..1694a0f 100644 --- a/sandboxed_api/var_ptr.h +++ b/sandboxed_api/var_ptr.h @@ -19,14 +19,14 @@ #include "absl/base/macros.h" #include "absl/strings/str_format.h" -#include "sandboxed_api/var_pointable.h" +#include "sandboxed_api/var_abstract.h" #include "sandboxed_api/var_reg.h" namespace sapi::v { // Class representing a pointer. Takes both Var* and regular pointers in the // initializers. -class Ptr : public Reg, public Pointable { +class Ptr : public Reg { public: Ptr() = delete; @@ -66,10 +66,6 @@ class Ptr : public Reg, public Pointable { } private: - Ptr* CreatePtr(Pointable::SyncType sync_type) override { - return new Ptr(this, sync_type); - } - // GetDataPtr() interface requires of us to return a pointer to the data // (variable) that can be copied. We cannot get pointer to pointer with // Var::GetRemote(), hence we cache it, and return pointer to it. diff --git a/sandboxed_api/var_reg.h b/sandboxed_api/var_reg.h index 786c814..6ead26c 100644 --- a/sandboxed_api/var_reg.h +++ b/sandboxed_api/var_reg.h @@ -29,9 +29,6 @@ namespace sapi::v { // type specifier in methods. class Callable : public Var { public: - Callable(const Callable&) = delete; - Callable& operator=(const Callable&) = delete; - // Get pointer to the stored data. virtual const void* GetDataPtr() = 0; @@ -82,7 +79,8 @@ class Reg : public Callable { std::string ToString() const override; protected: - T value_; // The stored value. + // The stored value. + T value_; }; template diff --git a/sandboxed_api/var_struct.h b/sandboxed_api/var_struct.h index 358a329..810ddac 100644 --- a/sandboxed_api/var_struct.h +++ b/sandboxed_api/var_struct.h @@ -19,15 +19,18 @@ #include "absl/base/macros.h" #include "absl/strings/str_cat.h" -#include "sandboxed_api/var_pointable.h" #include "sandboxed_api/var_ptr.h" namespace sapi::v { // Class representing a structure. template -class Struct : public Var, public Pointable { +class Struct : public Var { public: + using Var::PtrAfter; + using Var::PtrBefore; + using Var::PtrBoth; + // Forwarding constructor to initalize the struct_ field. template explicit Struct(Args&&... args) : struct_(std::forward(args)...) { @@ -48,11 +51,6 @@ class Struct : public Var, public Pointable { friend class LenVal; T struct_; - - private: - Ptr* CreatePtr(Pointable::SyncType type) override { - return new Ptr(this, type); - } }; } // namespace sapi::v diff --git a/sandboxed_api/var_void.h b/sandboxed_api/var_void.h index 0f325d5..e6c8d49 100644 --- a/sandboxed_api/var_void.h +++ b/sandboxed_api/var_void.h @@ -23,13 +23,10 @@ namespace sapi::v { // Good, old void. -class Void : public Callable, public Pointable { +class Void : public Callable { public: Void() = default; - Void(const Void&) = delete; - Void& operator=(const Void&) = delete; - size_t GetSize() const final { return 0U; } Type GetType() const final { return Type::kVoid; } std::string GetTypeString() const final { return "Void"; } @@ -37,11 +34,6 @@ class Void : public Callable, public Pointable { const void* GetDataPtr() override { return nullptr; } void SetDataFromPtr(const void* ptr, size_t max_sz) override {} - - private: - Ptr* CreatePtr(Pointable::SyncType sync_type) override { - return new Ptr(this, sync_type); - } }; } // namespace sapi::v diff --git a/sandboxed_api/vars.h b/sandboxed_api/vars.h index d11db39..eabc717 100644 --- a/sandboxed_api/vars.h +++ b/sandboxed_api/vars.h @@ -18,7 +18,6 @@ #include "sandboxed_api/var_array.h" #include "sandboxed_api/var_int.h" #include "sandboxed_api/var_lenval.h" -#include "sandboxed_api/var_pointable.h" #include "sandboxed_api/var_proto.h" #include "sandboxed_api/var_ptr.h" #include "sandboxed_api/var_struct.h"