From 7f5142aa79bea66d9f382080c95c415ccc500f75 Mon Sep 17 00:00:00 2001 From: Alex Ainscow Date: Mon, 24 Nov 2025 16:45:44 +0000 Subject: [PATCH] librados/cls: Refactor interface to provide compile time policing of read-only execs Signed-off-by: Alex Ainscow --- src/include/CMakeLists.txt | 2 + src/include/rados/cls_flags.h | 5 ++ src/include/rados/cls_traits.h | 50 ++++++++++++++++++++ src/include/rados/librados.hpp | 66 +++++++++++++++++++++++++- src/include/rados/objclass.h | 27 +++++++++-- src/librados/IoCtxImpl.cc | 85 +++++++++++++++++++++++++++++++++- src/librados/IoCtxImpl.h | 6 +++ src/librados/librados_cxx.cc | 66 ++++++++++++++++++++++---- src/objclass/class_api.cc | 11 ++++- 9 files changed, 301 insertions(+), 17 deletions(-) create mode 100644 src/include/rados/cls_flags.h create mode 100644 src/include/rados/cls_traits.h diff --git a/src/include/CMakeLists.txt b/src/include/CMakeLists.txt index cb9c2fea8f8..b2007538871 100644 --- a/src/include/CMakeLists.txt +++ b/src/include/CMakeLists.txt @@ -14,6 +14,8 @@ install(FILES page.h crc32c.h rados/objclass.h + rados/cls_flags.h + rados/cls_traits.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/rados) if(WITH_LIBRADOSSTRIPER) install(FILES diff --git a/src/include/rados/cls_flags.h b/src/include/rados/cls_flags.h new file mode 100644 index 00000000000..55347e1a449 --- /dev/null +++ b/src/include/rados/cls_flags.h @@ -0,0 +1,5 @@ +#pragma once + +#define CLS_METHOD_RD 0x1 /// method executes read operations +#define CLS_METHOD_WR 0x2 /// method executes write operations +#define CLS_METHOD_PROMOTE 0x8 /// method cannot be proxied to base tier \ No newline at end of file diff --git a/src/include/rados/cls_traits.h b/src/include/rados/cls_traits.h new file mode 100644 index 00000000000..a6450127691 --- /dev/null +++ b/src/include/rados/cls_traits.h @@ -0,0 +1,50 @@ +#pragma once +#include +#include "cls_flags.h" + +// Convenient point for this macro, as it will be removed once all the deprecated +// exec interfaces are no longer needed. +#define IGNORE_DEPRECATED \ +_Pragma("GCC diagnostic push") \ +_Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\" ") \ +_Pragma("clang diagnostic push") \ +_Pragma("clang diagnostic ignored \"-Wdeprecated-declarations\"") + +#define END_IGNORE_DEPRECATED \ +_Pragma("clang pop") \ +_Pragma("GCC pop") + +template +struct MethodTag {}; + +// Tags representing flags. +using RdTag = MethodTag; +using WrTag = MethodTag; +using PromoteTag = MethodTag; + +// Combinations of flags. +using RdWrTag = MethodTag; +using RdPromoteTag = MethodTag; +using WrPromoteTag = MethodTag; +using RdWrPromoteTag = MethodTag; + +template +struct ClsMethod { + const char* cls; + const char* name; + + constexpr ClsMethod(const char* n) : cls(ClassDef::name), name(n) {} +}; + +// Traits to map Tags to properties +template struct FlagTraits; + +template +struct FlagTraits> { + + static constexpr int value = Flags; + static constexpr bool is_readonly = (Flags & CLS_METHOD_WR) == 0; +}; + +template +constexpr bool is_safe_for_ro_v = FlagTraits::is_readonly; \ No newline at end of file diff --git a/src/include/rados/librados.hpp b/src/include/rados/librados.hpp index 96ed711def0..bca249ea17a 100644 --- a/src/include/rados/librados.hpp +++ b/src/include/rados/librados.hpp @@ -14,6 +14,8 @@ #include "librados_fwd.hpp" #include "rados_types.hpp" +#include "include/rados/cls_traits.h" + namespace libradosstriper { class RadosStriper; @@ -340,9 +342,28 @@ inline namespace v14_2_0 { void cmpext(uint64_t off, const bufferlist& cmp_bl, int *prval); void cmpxattr(const char *name, uint8_t op, const bufferlist& val); void cmpxattr(const char *name, uint8_t op, uint64_t v); + + [[deprecated("in favor of compile-time safe ops")]] void exec(const char *cls, const char *method, bufferlist& inbl); + [[deprecated("in favor of compile-time safe ops")]] void exec(const char *cls, const char *method, bufferlist& inbl, bufferlist *obl, int *prval); + [[deprecated("in favor of compile-time safe ops")]] void exec(const char *cls, const char *method, bufferlist& inbl, ObjectOperationCompletion *completion); + protected: + void exec_impl(const char *cls, const char *method, bufferlist& inbl); + void exec_impl(const char *cls, const char *method, bufferlist& inbl, bufferlist *obl, int *prval); + void exec_impl(const char *cls, const char *method, bufferlist& inbl, ObjectOperationCompletion *completion); + public: + + // By default only allow READ operations. ObjectWriteOperation overrides this + // to allow writes. + template + void exec(const ClsMethod& method, Args&&... args) { + static_assert(FlagTraits::is_readonly, + "Attempt to call a non-readonly class method as part of read. "); + exec_impl(method.cls, method.name, std::forward(args)...); + } + /** * Guard operation with a check that object version == ver * @@ -530,6 +551,14 @@ inline namespace v14_2_0 { void unset_manifest(); friend class IoCtx; + + using ObjectOperation::exec; // For deprecated rw operations. + + template + void exec(const ClsMethod& method, Args&&... args) { + // Read or write operations are permitted, so allow this. + exec_impl(method.cls, method.name, std::forward(args)...); + } }; /* @@ -888,8 +917,24 @@ inline namespace v14_2_0 { int rmxattr(const std::string& oid, const char *name); int stat(const std::string& oid, uint64_t *psize, time_t *pmtime); int stat2(const std::string& oid, uint64_t *psize, struct timespec *pts); + private: + // IoCtx needs a distinction between ro and rw to pick the correct flags + // for the operate call. + int exec_ro(const std::string& oid, const char *cls, const char *method, + bufferlist& inbl, bufferlist& outbl); + int exec_rw(const std::string& oid, const char *cls, const char *method, + bufferlist& inbl, bufferlist& outbl); + public: + template + int exec(const std::string& oid, const ClsMethod& method, bufferlist& inbl, bufferlist& outbl) { + if constexpr (FlagTraits::is_readonly) { + return exec_ro(oid, method.cls, method.name, inbl, outbl); + } + return exec_rw(oid, method.cls, method.name, inbl, outbl); + } + [[deprecated("in favor of compile-time safe ops")]] int exec(const std::string& oid, const char *cls, const char *method, - bufferlist& inbl, bufferlist& outbl); + bufferlist& inbl, bufferlist& outbl); /** * modify object tmap based on encoded update sequence * @@ -1178,8 +1223,25 @@ inline namespace v14_2_0 { */ int aio_cancel(AioCompletion *c); + + private: + int aio_exec_ro(const std::string& oid, AioCompletion *c, const char *cls, const char *method, + bufferlist& inbl, bufferlist *outbl); + int aio_exec_rw(const std::string& oid, AioCompletion *c, const char *cls, const char *method, + bufferlist& inbl, bufferlist *outbl); + + public: + template + int aio_exec(const std::string& oid, AioCompletion *c, + const ClsMethod& method, bufferlist& inbl, bufferlist *outbl) { + if constexpr (FlagTraits::is_readonly) { + return aio_exec_ro(oid, c, method.cls, method.name, inbl, outbl); + } + return aio_exec_rw(oid, c, method.cls, method.name, inbl, outbl); + } + [[deprecated("in favor of compile-time safe ops")]] int aio_exec(const std::string& oid, AioCompletion *c, const char *cls, const char *method, - bufferlist& inbl, bufferlist *outbl); + bufferlist& inbl, bufferlist *outbl); /* * asynchronous version of unlock diff --git a/src/include/rados/objclass.h b/src/include/rados/objclass.h index 1c188cc4bc8..0d35b97b9aa 100644 --- a/src/include/rados/objclass.h +++ b/src/include/rados/objclass.h @@ -7,6 +7,7 @@ #ifdef __cplusplus #include "buffer.h" +#include "include/rados/cls_traits.h" extern "C" { #endif @@ -25,10 +26,6 @@ const char *__cls_name = #name; #define CLS_INIT(name) \ CEPH_CLS_API void __cls_init() -#define CLS_METHOD_RD 0x1 /// method executes read operations -#define CLS_METHOD_WR 0x2 /// method executes write operations -#define CLS_METHOD_PROMOTE 0x8 /// method cannot be proxied to base tier - #define CLS_LOG(level, fmt, ...) \ cls_log(level, " %s:%d: " fmt, __FILE__, __LINE__, ##__VA_ARGS__) #define CLS_ERR(fmt, ...) CLS_LOG(0, fmt, ##__VA_ARGS__) @@ -85,6 +82,28 @@ typedef int (*cls_method_cxx_call_t)(cls_method_context_t ctx, * @param class_call * @param handle */ +namespace detail { +extern int cls_register_cxx_method_impl(cls_handle_t hclass, const char *method, int flags, + cls_method_cxx_call_t class_call, cls_method_handle_t *handle); +} + +template +struct ClassRegistrar { + cls_handle_t h_class; + + ClassRegistrar(cls_handle_t h) : h_class(h) {} + + template + void register_cxx_method(const ClsMethod& method_def, + cls_method_cxx_call_t class_call, + cls_method_handle_t* handle) { + + int flags = FlagTraits::value; + detail::cls_register_cxx_method_impl(h_class, method_def.name, flags, class_call, handle); + } +}; + +[[deprecated("In favor of ClassRegistrar")]] extern int cls_register_cxx_method(cls_handle_t hclass, const char *method, int flags, cls_method_cxx_call_t class_call, cls_method_handle_t *handle); diff --git a/src/librados/IoCtxImpl.cc b/src/librados/IoCtxImpl.cc index 410f18820ac..dcdcffcdbfa 100644 --- a/src/librados/IoCtxImpl.cc +++ b/src/librados/IoCtxImpl.cc @@ -1317,6 +1317,21 @@ int librados::IoCtxImpl::tmap_update(const object_t& oid, bufferlist& cmdbl) int librados::IoCtxImpl::exec(const object_t& oid, const char *cls, const char *method, bufferlist& inbl, bufferlist& outbl) +{ + ::ObjectOperation rd; + prepare_assert_ops(&rd); + rd.call(cls, method, inbl); + // If used through the non-deprecated exec interface, this will always be + // a write, but class execs can be both read AND write, so we call the read + // method, but tell it is a write too. The RWORDERED flag is to prevent the + // balanced read flag from being set. + return operate_read(oid, &rd, &outbl, CEPH_OSD_FLAG_WRITE|CEPH_OSD_FLAG_RWORDERED, + objclass_flags_mask & ~(CEPH_OSD_FLAG_BALANCE_READS|CEPH_OSD_FLAG_LOCALIZE_READS)); +} + +int librados::IoCtxImpl::exec_ro(const object_t& oid, + const char *cls, const char *method, + bufferlist& inbl, bufferlist& outbl) { ::ObjectOperation rd; prepare_assert_ops(&rd); @@ -1340,8 +1355,16 @@ int librados::IoCtxImpl::aio_exec(const object_t& oid, AioCompletionImpl *c, ::ObjectOperation rd; prepare_assert_ops(&rd); rd.call(cls, method, inbl); + // The prepare_read_op interface is used to allow for any data returned by the + // exec to be populated in outbl. If using the non-deprecated interface, this + // is also a write, so the WRITE and RWORDERED flags are added. + // Belt-and-brances, make sure that the BALANCE_READ and LOCALIZE_READ flags + // are never set. Objecter::Op *o = objecter->prepare_read_op( - oid, oloc, rd, snap_seq, outbl, extra_op_flags, objclass_flags_mask, oncomplete, &c->objver); + oid, oloc, rd, snap_seq, outbl, + extra_op_flags|CEPH_OSD_COPY_FROM_FLAG_RWORDERED|CEPH_OSD_FLAG_WRITE, + objclass_flags_mask & ~(CEPH_OSD_FLAG_BALANCE_READS|CEPH_OSD_FLAG_LOCALIZE_READS), + oncomplete, &c->objver); objecter->op_submit(o, &c->tid); return 0; } @@ -1353,6 +1376,63 @@ int librados::IoCtxImpl::aio_exec(const object_t& oid, AioCompletionImpl *c, FUNCTRACE(client->cct); Context *oncomplete = new C_aio_Complete(c); +#if defined(WITH_EVENTTRACE) + ((C_aio_Complete *) oncomplete)->oid = oid; +#endif + c->is_read = true; + c->io = this; + c->bl.clear(); + c->bl.push_back(buffer::create_static(out_len, buf)); + c->blp = &c->bl; + c->out_buf = buf; + + ::ObjectOperation rd; + prepare_assert_ops(&rd); + rd.call(cls, method, inbl); + // The prepare_read_op interface is used to allow for any data returned by the + // exec to be populated in outbl. If using the non-deprecated interface, this + // is also a write, so the WRITE and RWORDERED flags are added. + // Belt-and-brances, make sure that the BALANCE_READ and LOCALIZE_READ flags + // are never set. + Objecter::Op *o = objecter->prepare_read_op( + oid, oloc, rd, snap_seq, &c->bl, + extra_op_flags|CEPH_OSD_COPY_FROM_FLAG_RWORDERED|CEPH_OSD_FLAG_WRITE, + objclass_flags_mask & ~(CEPH_OSD_FLAG_BALANCE_READS|CEPH_OSD_FLAG_LOCALIZE_READS), + oncomplete, &c->objver); + objecter->op_submit(o, &c->tid); + return 0; +} + +int librados::IoCtxImpl::aio_exec_ro(const object_t& oid, AioCompletionImpl *c, + const char *cls, const char *method, + bufferlist& inbl, bufferlist *outbl) +{ + FUNCTRACE(client->cct); + Context *oncomplete = new C_aio_Complete(c); + +#if defined(WITH_EVENTTRACE) + ((C_aio_Complete *) oncomplete)->oid = oid; +#endif + c->is_read = true; + c->io = this; + + ::ObjectOperation rd; + prepare_assert_ops(&rd); + rd.call(cls, method, inbl); + Objecter::Op *o = objecter->prepare_read_op( + oid, oloc, rd, snap_seq, outbl, extra_op_flags, objclass_flags_mask, + oncomplete, &c->objver); + objecter->op_submit(o, &c->tid); + return 0; +} + +int librados::IoCtxImpl::aio_exec_ro(const object_t& oid, AioCompletionImpl *c, + const char *cls, const char *method, + bufferlist& inbl, char *buf, size_t out_len) +{ + FUNCTRACE(client->cct); + Context *oncomplete = new C_aio_Complete(c); + #if defined(WITH_EVENTTRACE) ((C_aio_Complete *) oncomplete)->oid = oid; #endif @@ -1367,7 +1447,8 @@ int librados::IoCtxImpl::aio_exec(const object_t& oid, AioCompletionImpl *c, prepare_assert_ops(&rd); rd.call(cls, method, inbl); Objecter::Op *o = objecter->prepare_read_op( - oid, oloc, rd, snap_seq, &c->bl, extra_op_flags, objclass_flags_mask, oncomplete, &c->objver); + oid, oloc, rd, snap_seq, &c->bl, extra_op_flags, objclass_flags_mask, + oncomplete, &c->objver); objecter->op_submit(o, &c->tid); return 0; } diff --git a/src/librados/IoCtxImpl.h b/src/librados/IoCtxImpl.h index 183dab976fc..645a29f8228 100644 --- a/src/librados/IoCtxImpl.h +++ b/src/librados/IoCtxImpl.h @@ -150,6 +150,7 @@ struct librados::IoCtxImpl { int tmap_update(const object_t& oid, bufferlist& cmdbl); int exec(const object_t& oid, const char *cls, const char *method, bufferlist& inbl, bufferlist& outbl); + int exec_ro(const object_t& oid, const char *cls, const char *method, bufferlist& inbl, bufferlist& outbl); int getxattr(const object_t& oid, const char *name, bufferlist& bl); int setxattr(const object_t& oid, const char *name, bufferlist& bl); @@ -217,6 +218,11 @@ struct librados::IoCtxImpl { const char *method, bufferlist& inbl, bufferlist *outbl); int aio_exec(const object_t& oid, AioCompletionImpl *c, const char *cls, const char *method, bufferlist& inbl, char *buf, size_t out_len); + int aio_exec_ro(const object_t& oid, AioCompletionImpl *c, + const char *cls, const char *method, + bufferlist& inbl, bufferlist *outbl); + int aio_exec_ro(const object_t& oid, AioCompletionImpl *c, const char *cls, + const char *method, bufferlist& inbl, char *buf, size_t out_len); int aio_stat(const object_t& oid, AioCompletionImpl *c, uint64_t *psize, time_t *pmtime); int aio_stat2(const object_t& oid, AioCompletionImpl *c, uint64_t *psize, struct timespec *pts); int aio_getxattr(const object_t& oid, AioCompletionImpl *c, diff --git a/src/librados/librados_cxx.cc b/src/librados/librados_cxx.cc index 223e8d4606c..cb890f180cb 100644 --- a/src/librados/librados_cxx.cc +++ b/src/librados/librados_cxx.cc @@ -151,15 +151,15 @@ void librados::ObjectOperation::assert_exists() o->stat(nullptr, nullptr, nullptr); } -void librados::ObjectOperation::exec(const char *cls, const char *method, - bufferlist& inbl) +void librados::ObjectOperation::exec_impl(const char *cls, const char *method, + bufferlist& inbl) { ceph_assert(impl); ::ObjectOperation *o = &impl->o; o->call(cls, method, inbl); } -void librados::ObjectOperation::exec(const char *cls, const char *method, bufferlist& inbl, bufferlist *outbl, int *prval) +void librados::ObjectOperation::exec_impl(const char *cls, const char *method, bufferlist& inbl, bufferlist *outbl, int *prval) { ceph_assert(impl); ::ObjectOperation *o = &impl->o; @@ -181,7 +181,8 @@ public: } }; -void librados::ObjectOperation::exec(const char *cls, const char *method, bufferlist& inbl, librados::ObjectOperationCompletion *completion) +void librados::ObjectOperation::exec_impl(const char *cls, const char *method, + bufferlist& inbl, librados::ObjectOperationCompletion *completion) { ceph_assert(impl); ::ObjectOperation *o = &impl->o; @@ -191,6 +192,22 @@ void librados::ObjectOperation::exec(const char *cls, const char *method, buffer o->call(cls, method, inbl, ctx->outbl(), ctx, NULL); } +// deprecated +void librados::ObjectOperation::exec(const char *cls, const char *method, + bufferlist& inbl, librados::ObjectOperationCompletion *completion) { + exec_impl(cls, method, inbl, completion); +} +//deprecated +void librados::ObjectOperation::exec(const char *cls, const char *method, + bufferlist& inbl, bufferlist *outbl, int *prval) { + exec_impl(cls, method, inbl, outbl, prval); +} +//deprecated +void librados::ObjectOperation::exec(const char *cls, const char *method, + bufferlist& inbl) { + exec_impl(cls, method, inbl); +} + void librados::ObjectReadOperation::stat(uint64_t *psize, time_t *pmtime, int *prval) { ceph_assert(impl); @@ -1363,13 +1380,27 @@ int librados::IoCtx::stat2(const std::string& oid, uint64_t *psize, struct times return io_ctx_impl->stat2(obj, psize, pts); } -int librados::IoCtx::exec(const std::string& oid, const char *cls, const char *method, +int librados::IoCtx::exec_rw(const std::string& oid, const char *cls, const char *method, bufferlist& inbl, bufferlist& outbl) { object_t obj(oid); return io_ctx_impl->exec(obj, cls, method, inbl, outbl); } +int librados::IoCtx::exec_ro(const std::string& oid, const char *cls, const char *method, + bufferlist& inbl, bufferlist& outbl) +{ + object_t obj(oid); + return io_ctx_impl->exec_ro(obj, cls, method, inbl, outbl); +} + +// Deprecated +int librados::IoCtx::exec(const std::string& oid, const char *cls, const char *method, + bufferlist& inbl, bufferlist& outbl) +{ + return exec_rw(oid, cls, method, inbl, outbl); +} + int librados::IoCtx::tmap_update(const std::string& oid, bufferlist& cmdbl) { object_t obj(oid); @@ -1953,10 +1984,29 @@ int librados::IoCtx::aio_read(const std::string& oid, librados::AioCompletion *c return io_ctx_impl->aio_read(oid, c->pc, pbl, len, off, snapid); } +int librados::IoCtx::aio_exec_ro(const std::string& oid, + librados::AioCompletion *c, const char *cls, + const char *method, bufferlist& inbl, + bufferlist *outbl) +{ + object_t obj(oid); + return io_ctx_impl->aio_exec_ro(obj, c->pc, cls, method, inbl, outbl); +} + +int librados::IoCtx::aio_exec_rw(const std::string& oid, + librados::AioCompletion *c, const char *cls, + const char *method, bufferlist& inbl, + bufferlist *outbl) +{ + object_t obj(oid); + return io_ctx_impl->aio_exec(obj, c->pc, cls, method, inbl, outbl); +} + +// deprecated. int librados::IoCtx::aio_exec(const std::string& oid, - librados::AioCompletion *c, const char *cls, - const char *method, bufferlist& inbl, - bufferlist *outbl) + librados::AioCompletion *c, const char *cls, + const char *method, bufferlist& inbl, + bufferlist *outbl) { object_t obj(oid); return io_ctx_impl->aio_exec(obj, c->pc, cls, method, inbl, outbl); diff --git a/src/objclass/class_api.cc b/src/objclass/class_api.cc index b84b05afc2d..f56d8ef6c38 100644 --- a/src/objclass/class_api.cc +++ b/src/objclass/class_api.cc @@ -55,7 +55,7 @@ int cls_register_method(cls_handle_t hclass, const char *method, return (hmethod != NULL); } -int cls_register_cxx_method(cls_handle_t hclass, const char *method, +int detail::cls_register_cxx_method_impl(cls_handle_t hclass, const char *method, int flags, cls_method_cxx_call_t class_call, cls_method_handle_t *handle) { @@ -66,6 +66,15 @@ int cls_register_cxx_method(cls_handle_t hclass, const char *method, return (hmethod != NULL); } +// deprecated +int cls_register_cxx_method(cls_handle_t hclass, const char *method, + int flags, + cls_method_cxx_call_t class_call, cls_method_handle_t *handle) +{ + return detail::cls_register_cxx_method_impl(hclass, method, flags, class_call, handle); +} + + int cls_unregister_method(cls_method_handle_t handle) { ClassHandler::ClassMethod *method = (ClassHandler::ClassMethod *)handle; -- 2.47.3