Message ID | 20210819071956.3431114-1-paul.elder@ideasonboard.com |
---|---|
State | Accepted |
Commit | 31078711d6c3639073db97322c6f7d98dacbbefe |
Headers | show |
Series |
|
Related | show |
Hi Paul, On 8/19/21 12:49 PM, Paul Elder wrote: > Regarding (de)serialization in isolated IPA calls, we have four layers: > - struct > - byte vector + fd vector > - IPCMessage > - IPC payload > > The proxy handles the upper three layers (with help from the > IPADataSerializer), and passes an IPCMessage to the IPC mechanism > (implemented as an IPCPipe), which sends an IPC payload to its worker > counterpart. > > When a FileDescriptor is involved, previously it was only a > FileDescriptor in the first layer; in the lower three it was an int. To > reduce the risk of potential fd leaks in the future, keep the > FileDescriptor as-is throughout the upper three layers. Only the IPC > mechanism will deal with ints, if it so wishes, when it does the actual > IPC. IPCPipeUnixSocket does deal with ints for sending fds, so the > conversion between IPCMessage and IPCUnixSocket::Payload converts > between FileDescriptor and int. > > Additionally, change the data portion of the serialized form of > FileDescriptor to a 32-bit unsigned integer, for alightnment purposes > and in preparation for conversion to an index into the fd array. > > Also update the deserializer of FrameBuffer::Plane accordingly. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > Tested-by: Umang Jain <umang.jain@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > --- > Changes in v4: > - update deserializer for FrameBuffer::Plane > --- v4 tested again with fd-leak test. No issues observed. > .../libcamera/internal/ipa_data_serializer.h | 40 ++++---- > include/libcamera/internal/ipc_pipe.h | 10 +- > src/libcamera/ipa_data_serializer.cpp | 95 ++++++++++--------- > src/libcamera/ipc_pipe.cpp | 12 ++- > .../ipa_data_serializer_test.cpp | 12 +-- > .../module_ipa_proxy.cpp.tmpl | 2 +- > .../module_ipa_proxy.h.tmpl | 2 +- > .../libcamera_templates/proxy_functions.tmpl | 2 +- > .../libcamera_templates/serializer.tmpl | 26 ++--- > 9 files changed, 108 insertions(+), 93 deletions(-) > > diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h > index 519093bd..4353e07b 100644 > --- a/include/libcamera/internal/ipa_data_serializer.h > +++ b/include/libcamera/internal/ipa_data_serializer.h > @@ -66,7 +66,7 @@ template<typename T> > class IPADataSerializer > { > public: > - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> > + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> > serialize(const T &data, ControlSerializer *cs = nullptr); > > static T deserialize(const std::vector<uint8_t> &data, > @@ -76,12 +76,12 @@ public: > ControlSerializer *cs = nullptr); > > static T deserialize(const std::vector<uint8_t> &data, > - const std::vector<int32_t> &fds, > + const std::vector<FileDescriptor> &fds, > ControlSerializer *cs = nullptr); > static T deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > - std::vector<int32_t>::const_iterator fdsBegin, > - std::vector<int32_t>::const_iterator fdsEnd, > + std::vector<FileDescriptor>::const_iterator fdsBegin, > + std::vector<FileDescriptor>::const_iterator fdsEnd, > ControlSerializer *cs = nullptr); > }; > > @@ -104,11 +104,11 @@ template<typename V> > class IPADataSerializer<std::vector<V>> > { > public: > - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> > + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> > serialize(const std::vector<V> &data, ControlSerializer *cs = nullptr) > { > std::vector<uint8_t> dataVec; > - std::vector<int32_t> fdsVec; > + std::vector<FileDescriptor> fdsVec; > > /* Serialize the length. */ > uint32_t vecLen = data.size(); > @@ -117,7 +117,7 @@ public: > /* Serialize the members. */ > for (auto const &it : data) { > std::vector<uint8_t> dvec; > - std::vector<int32_t> fvec; > + std::vector<FileDescriptor> fvec; > > std::tie(dvec, fvec) = > IPADataSerializer<V>::serialize(it, cs); > @@ -141,11 +141,11 @@ public: > std::vector<uint8_t>::const_iterator dataEnd, > ControlSerializer *cs = nullptr) > { > - std::vector<int32_t> fds; > + std::vector<FileDescriptor> fds; > return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs); > } > > - static std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds, > + static std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<FileDescriptor> &fds, > ControlSerializer *cs = nullptr) > { > return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); > @@ -153,15 +153,15 @@ public: > > static std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > - std::vector<int32_t>::const_iterator fdsBegin, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, > + std::vector<FileDescriptor>::const_iterator fdsBegin, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, > ControlSerializer *cs = nullptr) > { > uint32_t vecLen = readPOD<uint32_t>(dataBegin, 0, dataEnd); > std::vector<V> ret(vecLen); > > std::vector<uint8_t>::const_iterator dataIter = dataBegin + 4; > - std::vector<int32_t>::const_iterator fdIter = fdsBegin; > + std::vector<FileDescriptor>::const_iterator fdIter = fdsBegin; > for (uint32_t i = 0; i < vecLen; i++) { > uint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd); > uint32_t sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd); > @@ -201,11 +201,11 @@ template<typename K, typename V> > class IPADataSerializer<std::map<K, V>> > { > public: > - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> > + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> > serialize(const std::map<K, V> &data, ControlSerializer *cs = nullptr) > { > std::vector<uint8_t> dataVec; > - std::vector<int32_t> fdsVec; > + std::vector<FileDescriptor> fdsVec; > > /* Serialize the length. */ > uint32_t mapLen = data.size(); > @@ -214,7 +214,7 @@ public: > /* Serialize the members. */ > for (auto const &it : data) { > std::vector<uint8_t> dvec; > - std::vector<int32_t> fvec; > + std::vector<FileDescriptor> fvec; > > std::tie(dvec, fvec) = > IPADataSerializer<K>::serialize(it.first, cs); > @@ -247,11 +247,11 @@ public: > std::vector<uint8_t>::const_iterator dataEnd, > ControlSerializer *cs = nullptr) > { > - std::vector<int32_t> fds; > + std::vector<FileDescriptor> fds; > return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs); > } > > - static std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds, > + static std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<FileDescriptor> &fds, > ControlSerializer *cs = nullptr) > { > return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); > @@ -259,8 +259,8 @@ public: > > static std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > - std::vector<int32_t>::const_iterator fdsBegin, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, > + std::vector<FileDescriptor>::const_iterator fdsBegin, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, > ControlSerializer *cs = nullptr) > { > std::map<K, V> ret; > @@ -268,7 +268,7 @@ public: > uint32_t mapLen = readPOD<uint32_t>(dataBegin, 0, dataEnd); > > std::vector<uint8_t>::const_iterator dataIter = dataBegin + 4; > - std::vector<int32_t>::const_iterator fdIter = fdsBegin; > + std::vector<FileDescriptor>::const_iterator fdIter = fdsBegin; > for (uint32_t i = 0; i < mapLen; i++) { > uint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd); > uint32_t sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd); > diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h > index e58de340..52cc8fa3 100644 > --- a/include/libcamera/internal/ipc_pipe.h > +++ b/include/libcamera/internal/ipc_pipe.h > @@ -11,6 +11,8 @@ > > #include <libcamera/base/signal.h> > > +#include <libcamera/file_descriptor.h> > + > #include "libcamera/internal/ipc_unixsocket.h" > > namespace libcamera { > @@ -26,23 +28,23 @@ public: > IPCMessage(); > IPCMessage(uint32_t cmd); > IPCMessage(const Header &header); > - IPCMessage(const IPCUnixSocket::Payload &payload); > + IPCMessage(IPCUnixSocket::Payload &payload); > > IPCUnixSocket::Payload payload() const; > > Header &header() { return header_; } > std::vector<uint8_t> &data() { return data_; } > - std::vector<int32_t> &fds() { return fds_; } > + std::vector<FileDescriptor> &fds() { return fds_; } > > const Header &header() const { return header_; } > const std::vector<uint8_t> &data() const { return data_; } > - const std::vector<int32_t> &fds() const { return fds_; } > + const std::vector<FileDescriptor> &fds() const { return fds_; } > > private: > Header header_; > > std::vector<uint8_t> data_; > - std::vector<int32_t> fds_; > + std::vector<FileDescriptor> fds_; > }; > > class IPCPipe > diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp > index fb941e6b..5b183c70 100644 > --- a/src/libcamera/ipa_data_serializer.cpp > +++ b/src/libcamera/ipa_data_serializer.cpp > @@ -7,6 +7,8 @@ > > #include "libcamera/internal/ipa_data_serializer.h" > > +#include <unistd.h> > + > #include <libcamera/base/log.h> > > /** > @@ -141,7 +143,7 @@ namespace { > /** > * \fn template<typename T> IPADataSerializer<T>::deserialize( > * const std::vector<uint8_t> &data, > - * const std::vector<int32_t> &fds, > + * const std::vector<FileDescriptor> &fds, > * ControlSerializer *cs = nullptr) > * \brief Deserialize byte vector and fd vector into an object > * \tparam T Type of object to deserialize to > @@ -162,8 +164,8 @@ namespace { > * \fn template<typename T> IPADataSerializer::deserialize( > * std::vector<uint8_t>::const_iterator dataBegin, > * std::vector<uint8_t>::const_iterator dataEnd, > - * std::vector<int32_t>::const_iterator fdsBegin, > - * std::vector<int32_t>::const_iterator fdsEnd, > + * std::vector<FileDescriptor>::const_iterator fdsBegin, > + * std::vector<FileDescriptor>::const_iterator fdsEnd, > * ControlSerializer *cs = nullptr) > * \brief Deserialize byte vector and fd vector into an object > * \tparam T Type of object to deserialize to > @@ -187,7 +189,7 @@ namespace { > #define DEFINE_POD_SERIALIZER(type) \ > \ > template<> \ > -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> \ > +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> \ > IPADataSerializer<type>::serialize(const type &data, \ > [[maybe_unused]] ControlSerializer *cs) \ > { \ > @@ -215,7 +217,7 @@ type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \ > \ > template<> \ > type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \ > - [[maybe_unused]] const std::vector<int32_t> &fds, \ > + [[maybe_unused]] const std::vector<FileDescriptor> &fds, \ > ControlSerializer *cs) \ > { \ > return deserialize(data.cbegin(), data.end(), cs); \ > @@ -224,8 +226,8 @@ type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \ > template<> \ > type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \ > std::vector<uint8_t>::const_iterator dataEnd, \ > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, \ > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, \ > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, \ > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, \ > ControlSerializer *cs) \ > { \ > return deserialize(dataBegin, dataEnd, cs); \ > @@ -249,7 +251,7 @@ DEFINE_POD_SERIALIZER(double) > * function parameter serdes). > */ > template<> > -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> > +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> > IPADataSerializer<std::string>::serialize(const std::string &data, > [[maybe_unused]] ControlSerializer *cs) > { > @@ -276,7 +278,7 @@ IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator > template<> > std::string > IPADataSerializer<std::string>::deserialize(const std::vector<uint8_t> &data, > - [[maybe_unused]] const std::vector<int32_t> &fds, > + [[maybe_unused]] const std::vector<FileDescriptor> &fds, > [[maybe_unused]] ControlSerializer *cs) > { > return { data.cbegin(), data.cend() }; > @@ -286,8 +288,8 @@ template<> > std::string > IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, > [[maybe_unused]] ControlSerializer *cs) > { > return { dataBegin, dataEnd }; > @@ -305,7 +307,7 @@ IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator > * be used. The serialized ControlInfoMap will have zero length. > */ > template<> > -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> > +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> > IPADataSerializer<ControlList>::serialize(const ControlList &data, ControlSerializer *cs) > { > if (!cs) > @@ -405,7 +407,7 @@ IPADataSerializer<ControlList>::deserialize(const std::vector<uint8_t> &data, > template<> > ControlList > IPADataSerializer<ControlList>::deserialize(const std::vector<uint8_t> &data, > - [[maybe_unused]] const std::vector<int32_t> &fds, > + [[maybe_unused]] const std::vector<FileDescriptor> &fds, > ControlSerializer *cs) > { > return deserialize(data.cbegin(), data.end(), cs); > @@ -415,8 +417,8 @@ template<> > ControlList > IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, > ControlSerializer *cs) > { > return deserialize(dataBegin, dataEnd, cs); > @@ -429,7 +431,7 @@ IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator > * X bytes - Serialized ControlInfoMap (using ControlSerializer) > */ > template<> > -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> > +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> > IPADataSerializer<ControlInfoMap>::serialize(const ControlInfoMap &map, > ControlSerializer *cs) > { > @@ -491,7 +493,7 @@ IPADataSerializer<ControlInfoMap>::deserialize(const std::vector<uint8_t> &data, > template<> > ControlInfoMap > IPADataSerializer<ControlInfoMap>::deserialize(const std::vector<uint8_t> &data, > - [[maybe_unused]] const std::vector<int32_t> &fds, > + [[maybe_unused]] const std::vector<FileDescriptor> &fds, > ControlSerializer *cs) > { > return deserialize(data.cbegin(), data.end(), cs); > @@ -501,15 +503,15 @@ template<> > ControlInfoMap > IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, > ControlSerializer *cs) > { > return deserialize(dataBegin, dataEnd, cs); > } > > /* > - * FileDescriptors are serialized into a single byte that tells if the > + * FileDescriptors are serialized into four bytes that tells if the > * FileDescriptor is valid or not. If it is valid, then for serialization > * the fd will be written to the fd vector, or for deserialization the > * fd vector const_iterator will be valid. > @@ -517,42 +519,47 @@ IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_itera > * This validity is necessary so that we don't send -1 fd over sendmsg(). It > * also allows us to simply send the entire fd vector into the deserializer > * and it will be recursively consumed as necessary. > - * > - * \todo Consider serializing the FileDescriptor in 4 bytes to ensure > - * 32-bit alignment of all serialized data > */ > template<> > -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> > +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> > IPADataSerializer<FileDescriptor>::serialize(const FileDescriptor &data, > [[maybe_unused]] ControlSerializer *cs) > { > - std::vector<uint8_t> dataVec = { data.isValid() }; > - std::vector<int32_t> fdVec; > + std::vector<uint8_t> dataVec; > + std::vector<FileDescriptor> fdVec; > + > + /* > + * Store as uint32_t to prepare for conversion from validity flag > + * to index, and for alignment. > + */ > + appendPOD<uint32_t>(dataVec, data.isValid()); > + > if (data.isValid()) > - fdVec.push_back(data.fd()); > + fdVec.push_back(data); > + > > return { dataVec, fdVec }; > } > > template<> > -FileDescriptor IPADataSerializer<FileDescriptor>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, > - std::vector<uint8_t>::const_iterator dataEnd, > - std::vector<int32_t>::const_iterator fdsBegin, > - std::vector<int32_t>::const_iterator fdsEnd, > +FileDescriptor IPADataSerializer<FileDescriptor>::deserialize([[maybe_unused]] std::vector<uint8_t>::const_iterator dataBegin, > + [[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd, > + std::vector<FileDescriptor>::const_iterator fdsBegin, > + std::vector<FileDescriptor>::const_iterator fdsEnd, > [[maybe_unused]] ControlSerializer *cs) > { > - ASSERT(std::distance(dataBegin, dataEnd) >= 1); > + ASSERT(std::distance(dataBegin, dataEnd) >= 4); > > - bool valid = !!(*dataBegin); > + uint32_t valid = readPOD<uint32_t>(dataBegin, 0, dataEnd); > > ASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1)); > > - return valid ? FileDescriptor(*fdsBegin) : FileDescriptor(); > + return valid ? *fdsBegin : FileDescriptor(); > } > > template<> > FileDescriptor IPADataSerializer<FileDescriptor>::deserialize(const std::vector<uint8_t> &data, > - const std::vector<int32_t> &fds, > + const std::vector<FileDescriptor> &fds, > [[maybe_unused]] ControlSerializer *cs) > { > return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end()); > @@ -561,19 +568,19 @@ FileDescriptor IPADataSerializer<FileDescriptor>::deserialize(const std::vector< > /* > * FrameBuffer::Plane is serialized as: > * > - * 1 byte - FileDescriptor > + * 4 byte - FileDescriptor > * 4 bytes - uint32_t Length > */ > template<> > -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> > +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> > IPADataSerializer<FrameBuffer::Plane>::serialize(const FrameBuffer::Plane &data, > [[maybe_unused]] ControlSerializer *cs) > { > std::vector<uint8_t> dataVec; > - std::vector<int32_t> fdsVec; > + std::vector<FileDescriptor> fdsVec; > > std::vector<uint8_t> fdBuf; > - std::vector<int32_t> fdFds; > + std::vector<FileDescriptor> fdFds; > std::tie(fdBuf, fdFds) = > IPADataSerializer<FileDescriptor>::serialize(data.fd); > dataVec.insert(dataVec.end(), fdBuf.begin(), fdBuf.end()); > @@ -588,15 +595,15 @@ template<> > FrameBuffer::Plane > IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > - std::vector<int32_t>::const_iterator fdsBegin, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, > + std::vector<FileDescriptor>::const_iterator fdsBegin, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, > [[maybe_unused]] ControlSerializer *cs) > { > FrameBuffer::Plane ret; > > - ret.fd = IPADataSerializer<FileDescriptor>::deserialize(dataBegin, dataBegin + 1, > + ret.fd = IPADataSerializer<FileDescriptor>::deserialize(dataBegin, dataBegin + 4, > fdsBegin, fdsBegin + 1); > - ret.length = readPOD<uint32_t>(dataBegin, 1, dataEnd); > + ret.length = readPOD<uint32_t>(dataBegin, 4, dataEnd); > > return ret; > } > @@ -604,7 +611,7 @@ IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_i > template<> > FrameBuffer::Plane > IPADataSerializer<FrameBuffer::Plane>::deserialize(const std::vector<uint8_t> &data, > - const std::vector<int32_t> &fds, > + const std::vector<FileDescriptor> &fds, > ControlSerializer *cs) > { > return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); > diff --git a/src/libcamera/ipc_pipe.cpp b/src/libcamera/ipc_pipe.cpp > index 28e20e03..84136a82 100644 > --- a/src/libcamera/ipc_pipe.cpp > +++ b/src/libcamera/ipc_pipe.cpp > @@ -77,13 +77,17 @@ IPCMessage::IPCMessage(const Header &header) > * > * This essentially converts an IPCUnixSocket payload into an IPCMessage. > * The header is extracted from the payload into the IPCMessage's header field. > + * > + * If the IPCUnixSocket payload had any valid file descriptors, then they will > + * all be invalidated. > */ > -IPCMessage::IPCMessage(const IPCUnixSocket::Payload &payload) > +IPCMessage::IPCMessage(IPCUnixSocket::Payload &payload) > { > memcpy(&header_, payload.data.data(), sizeof(header_)); > data_ = std::vector<uint8_t>(payload.data.begin() + sizeof(header_), > payload.data.end()); > - fds_ = payload.fds; > + for (int32_t &fd : payload.fds) > + fds_.push_back(FileDescriptor(std::move(fd))); > } > > /** > @@ -104,7 +108,9 @@ IPCUnixSocket::Payload IPCMessage::payload() const > > /* \todo Make this work without copy */ > memcpy(payload.data.data() + sizeof(Header), data_.data(), data_.size()); > - payload.fds = fds_; > + > + for (const FileDescriptor &fd : fds_) > + payload.fds.push_back(fd.fd()); > > return payload; > } > diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp > index bf7e34e2..eca19a66 100644 > --- a/test/serialization/ipa_data_serializer_test.cpp > +++ b/test/serialization/ipa_data_serializer_test.cpp > @@ -53,7 +53,7 @@ template<typename T> > int testPodSerdes(T in) > { > std::vector<uint8_t> buf; > - std::vector<int32_t> fds; > + std::vector<FileDescriptor> fds; > > std::tie(buf, fds) = IPADataSerializer<T>::serialize(in); > T out = IPADataSerializer<T>::deserialize(buf, fds); > @@ -72,7 +72,7 @@ int testVectorSerdes(const std::vector<T> &in, > ControlSerializer *cs = nullptr) > { > std::vector<uint8_t> buf; > - std::vector<int32_t> fds; > + std::vector<FileDescriptor> fds; > > std::tie(buf, fds) = IPADataSerializer<std::vector<T>>::serialize(in, cs); > std::vector<T> out = IPADataSerializer<std::vector<T>>::deserialize(buf, fds, cs); > @@ -92,7 +92,7 @@ int testMapSerdes(const std::map<K, V> &in, > ControlSerializer *cs = nullptr) > { > std::vector<uint8_t> buf; > - std::vector<int32_t> fds; > + std::vector<FileDescriptor> fds; > > std::tie(buf, fds) = IPADataSerializer<std::map<K, V>>::serialize(in, cs); > std::map<K, V> out = IPADataSerializer<std::map<K, V>>::deserialize(buf, fds, cs); > @@ -219,7 +219,7 @@ private: > }; > > std::vector<uint8_t> buf; > - std::vector<int32_t> fds; > + std::vector<FileDescriptor> fds; > > if (testVectorSerdes(vecUint8) != TestPass) > return TestFail; > @@ -291,7 +291,7 @@ private: > { { "a", { 1, 2, 3 } }, { "b", { 4, 5, 6 } }, { "c", { 7, 8, 9 } } }; > > std::vector<uint8_t> buf; > - std::vector<int32_t> fds; > + std::vector<FileDescriptor> fds; > > if (testMapSerdes(mapUintStr) != TestPass) > return TestFail; > @@ -359,7 +359,7 @@ private: > std::string strEmpty = ""; > > std::vector<uint8_t> buf; > - std::vector<int32_t> fds; > + std::vector<FileDescriptor> fds; > > if (testPodSerdes(u32min) != TestPass) > return TestFail; > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > index a4e008c7..aab1fffb 100644 > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > @@ -236,7 +236,7 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) > void {{proxy_name}}::{{method.mojom_name}}IPC( > std::vector<uint8_t>::const_iterator data, > size_t dataSize, > - [[maybe_unused]] const std::vector<int32_t> &fds) > + [[maybe_unused]] const std::vector<FileDescriptor> &fds) > { > {%- for param in method.parameters %} > {{param|name}} {{param.mojom_name}}; > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > index c222f5f2..1979e68f 100644 > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl > @@ -65,7 +65,7 @@ private: > void {{method.mojom_name}}IPC( > std::vector<uint8_t>::const_iterator data, > size_t dataSize, > - const std::vector<int32_t> &fds); > + const std::vector<FileDescriptor> &fds); > {% endfor %} > > /* Helper class to invoke async functions in another thread. */ > diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl > index ea9cc86b..ebcd2aaa 100644 > --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl > +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl > @@ -54,7 +54,7 @@ > {%- for param in params %} > std::vector<uint8_t> {{param.mojom_name}}Buf; > {%- if param|has_fd %} > - std::vector<int32_t> {{param.mojom_name}}Fds; > + std::vector<FileDescriptor> {{param.mojom_name}}Fds; > std::tie({{param.mojom_name}}Buf, {{param.mojom_name}}Fds) = > {%- else %} > std::tie({{param.mojom_name}}Buf, std::ignore) = > diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl > index d8d55807..b8ef8e7b 100644 > --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl > +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl > @@ -40,7 +40,7 @@ > retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end()); > {%- elif field|is_fd %} > std::vector<uint8_t> {{field.mojom_name}}; > - std::vector<int32_t> {{field.mojom_name}}Fds; > + std::vector<FileDescriptor> {{field.mojom_name}}Fds; > std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) = > IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}); > retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end()); > @@ -58,7 +58,7 @@ > {%- elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %} > std::vector<uint8_t> {{field.mojom_name}}; > {%- if field|has_fd %} > - std::vector<int32_t> {{field.mojom_name}}Fds; > + std::vector<FileDescriptor> {{field.mojom_name}}Fds; > std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) = > {%- else %} > std::tie({{field.mojom_name}}, std::ignore) = > @@ -104,9 +104,9 @@ > dataSize -= {{field_size}}; > {%- endif %} > {% elif field|is_fd %} > - {%- set field_size = 1 %} > + {%- set field_size = 4 %} > {{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}} > - ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + 1, n, n + 1, cs); > + ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}, n, n + 1, cs); > {%- if not loop.last %} > m += {{field_size}}; > dataSize -= {{field_size}}; > @@ -177,7 +177,7 @@ > # \a struct. > #} > {%- macro serializer(struct, namespace) %} > - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> > + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> > serialize(const {{struct|name_full}} &data, > {%- if struct|needs_control_serializer %} > ControlSerializer *cs) > @@ -187,7 +187,7 @@ > { > std::vector<uint8_t> retData; > {%- if struct|has_fd %} > - std::vector<int32_t> retFds; > + std::vector<FileDescriptor> retFds; > {%- endif %} > {%- for field in struct.fields %} > {{serializer_field(field, namespace, loop)}} > @@ -210,7 +210,7 @@ > {%- macro deserializer_fd(struct, namespace) %} > static {{struct|name_full}} > deserialize(std::vector<uint8_t> &data, > - std::vector<int32_t> &fds, > + std::vector<FileDescriptor> &fds, > {%- if struct|needs_control_serializer %} > ControlSerializer *cs) > {%- else %} > @@ -224,8 +224,8 @@ > static {{struct|name_full}} > deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > - std::vector<int32_t>::const_iterator fdsBegin, > - std::vector<int32_t>::const_iterator fdsEnd, > + std::vector<FileDescriptor>::const_iterator fdsBegin, > + std::vector<FileDescriptor>::const_iterator fdsEnd, > {%- if struct|needs_control_serializer %} > ControlSerializer *cs) > {%- else %} > @@ -234,7 +234,7 @@ > { > {{struct|name_full}} ret; > std::vector<uint8_t>::const_iterator m = dataBegin; > - std::vector<int32_t>::const_iterator n = fdsBegin; > + std::vector<FileDescriptor>::const_iterator n = fdsBegin; > > size_t dataSize = std::distance(dataBegin, dataEnd); > [[maybe_unused]] size_t fdsSize = std::distance(fdsBegin, fdsEnd); > @@ -255,7 +255,7 @@ > {%- macro deserializer_fd_simple(struct, namespace) %} > static {{struct|name_full}} > deserialize(std::vector<uint8_t> &data, > - [[maybe_unused]] std::vector<int32_t> &fds, > + [[maybe_unused]] std::vector<FileDescriptor> &fds, > ControlSerializer *cs = nullptr) > { > return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs); > @@ -264,8 +264,8 @@ > static {{struct|name_full}} > deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, > - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, > + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, > ControlSerializer *cs = nullptr) > { > return IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs);
diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h index 519093bd..4353e07b 100644 --- a/include/libcamera/internal/ipa_data_serializer.h +++ b/include/libcamera/internal/ipa_data_serializer.h @@ -66,7 +66,7 @@ template<typename T> class IPADataSerializer { public: - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> serialize(const T &data, ControlSerializer *cs = nullptr); static T deserialize(const std::vector<uint8_t> &data, @@ -76,12 +76,12 @@ public: ControlSerializer *cs = nullptr); static T deserialize(const std::vector<uint8_t> &data, - const std::vector<int32_t> &fds, + const std::vector<FileDescriptor> &fds, ControlSerializer *cs = nullptr); static T deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, - std::vector<int32_t>::const_iterator fdsBegin, - std::vector<int32_t>::const_iterator fdsEnd, + std::vector<FileDescriptor>::const_iterator fdsBegin, + std::vector<FileDescriptor>::const_iterator fdsEnd, ControlSerializer *cs = nullptr); }; @@ -104,11 +104,11 @@ template<typename V> class IPADataSerializer<std::vector<V>> { public: - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> serialize(const std::vector<V> &data, ControlSerializer *cs = nullptr) { std::vector<uint8_t> dataVec; - std::vector<int32_t> fdsVec; + std::vector<FileDescriptor> fdsVec; /* Serialize the length. */ uint32_t vecLen = data.size(); @@ -117,7 +117,7 @@ public: /* Serialize the members. */ for (auto const &it : data) { std::vector<uint8_t> dvec; - std::vector<int32_t> fvec; + std::vector<FileDescriptor> fvec; std::tie(dvec, fvec) = IPADataSerializer<V>::serialize(it, cs); @@ -141,11 +141,11 @@ public: std::vector<uint8_t>::const_iterator dataEnd, ControlSerializer *cs = nullptr) { - std::vector<int32_t> fds; + std::vector<FileDescriptor> fds; return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs); } - static std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds, + static std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<FileDescriptor> &fds, ControlSerializer *cs = nullptr) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); @@ -153,15 +153,15 @@ public: static std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, - std::vector<int32_t>::const_iterator fdsBegin, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, + std::vector<FileDescriptor>::const_iterator fdsBegin, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, ControlSerializer *cs = nullptr) { uint32_t vecLen = readPOD<uint32_t>(dataBegin, 0, dataEnd); std::vector<V> ret(vecLen); std::vector<uint8_t>::const_iterator dataIter = dataBegin + 4; - std::vector<int32_t>::const_iterator fdIter = fdsBegin; + std::vector<FileDescriptor>::const_iterator fdIter = fdsBegin; for (uint32_t i = 0; i < vecLen; i++) { uint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd); uint32_t sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd); @@ -201,11 +201,11 @@ template<typename K, typename V> class IPADataSerializer<std::map<K, V>> { public: - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> serialize(const std::map<K, V> &data, ControlSerializer *cs = nullptr) { std::vector<uint8_t> dataVec; - std::vector<int32_t> fdsVec; + std::vector<FileDescriptor> fdsVec; /* Serialize the length. */ uint32_t mapLen = data.size(); @@ -214,7 +214,7 @@ public: /* Serialize the members. */ for (auto const &it : data) { std::vector<uint8_t> dvec; - std::vector<int32_t> fvec; + std::vector<FileDescriptor> fvec; std::tie(dvec, fvec) = IPADataSerializer<K>::serialize(it.first, cs); @@ -247,11 +247,11 @@ public: std::vector<uint8_t>::const_iterator dataEnd, ControlSerializer *cs = nullptr) { - std::vector<int32_t> fds; + std::vector<FileDescriptor> fds; return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs); } - static std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds, + static std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<FileDescriptor> &fds, ControlSerializer *cs = nullptr) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); @@ -259,8 +259,8 @@ public: static std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, - std::vector<int32_t>::const_iterator fdsBegin, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, + std::vector<FileDescriptor>::const_iterator fdsBegin, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, ControlSerializer *cs = nullptr) { std::map<K, V> ret; @@ -268,7 +268,7 @@ public: uint32_t mapLen = readPOD<uint32_t>(dataBegin, 0, dataEnd); std::vector<uint8_t>::const_iterator dataIter = dataBegin + 4; - std::vector<int32_t>::const_iterator fdIter = fdsBegin; + std::vector<FileDescriptor>::const_iterator fdIter = fdsBegin; for (uint32_t i = 0; i < mapLen; i++) { uint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd); uint32_t sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd); diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h index e58de340..52cc8fa3 100644 --- a/include/libcamera/internal/ipc_pipe.h +++ b/include/libcamera/internal/ipc_pipe.h @@ -11,6 +11,8 @@ #include <libcamera/base/signal.h> +#include <libcamera/file_descriptor.h> + #include "libcamera/internal/ipc_unixsocket.h" namespace libcamera { @@ -26,23 +28,23 @@ public: IPCMessage(); IPCMessage(uint32_t cmd); IPCMessage(const Header &header); - IPCMessage(const IPCUnixSocket::Payload &payload); + IPCMessage(IPCUnixSocket::Payload &payload); IPCUnixSocket::Payload payload() const; Header &header() { return header_; } std::vector<uint8_t> &data() { return data_; } - std::vector<int32_t> &fds() { return fds_; } + std::vector<FileDescriptor> &fds() { return fds_; } const Header &header() const { return header_; } const std::vector<uint8_t> &data() const { return data_; } - const std::vector<int32_t> &fds() const { return fds_; } + const std::vector<FileDescriptor> &fds() const { return fds_; } private: Header header_; std::vector<uint8_t> data_; - std::vector<int32_t> fds_; + std::vector<FileDescriptor> fds_; }; class IPCPipe diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp index fb941e6b..5b183c70 100644 --- a/src/libcamera/ipa_data_serializer.cpp +++ b/src/libcamera/ipa_data_serializer.cpp @@ -7,6 +7,8 @@ #include "libcamera/internal/ipa_data_serializer.h" +#include <unistd.h> + #include <libcamera/base/log.h> /** @@ -141,7 +143,7 @@ namespace { /** * \fn template<typename T> IPADataSerializer<T>::deserialize( * const std::vector<uint8_t> &data, - * const std::vector<int32_t> &fds, + * const std::vector<FileDescriptor> &fds, * ControlSerializer *cs = nullptr) * \brief Deserialize byte vector and fd vector into an object * \tparam T Type of object to deserialize to @@ -162,8 +164,8 @@ namespace { * \fn template<typename T> IPADataSerializer::deserialize( * std::vector<uint8_t>::const_iterator dataBegin, * std::vector<uint8_t>::const_iterator dataEnd, - * std::vector<int32_t>::const_iterator fdsBegin, - * std::vector<int32_t>::const_iterator fdsEnd, + * std::vector<FileDescriptor>::const_iterator fdsBegin, + * std::vector<FileDescriptor>::const_iterator fdsEnd, * ControlSerializer *cs = nullptr) * \brief Deserialize byte vector and fd vector into an object * \tparam T Type of object to deserialize to @@ -187,7 +189,7 @@ namespace { #define DEFINE_POD_SERIALIZER(type) \ \ template<> \ -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> \ +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> \ IPADataSerializer<type>::serialize(const type &data, \ [[maybe_unused]] ControlSerializer *cs) \ { \ @@ -215,7 +217,7 @@ type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \ \ template<> \ type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \ - [[maybe_unused]] const std::vector<int32_t> &fds, \ + [[maybe_unused]] const std::vector<FileDescriptor> &fds, \ ControlSerializer *cs) \ { \ return deserialize(data.cbegin(), data.end(), cs); \ @@ -224,8 +226,8 @@ type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \ template<> \ type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \ std::vector<uint8_t>::const_iterator dataEnd, \ - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, \ - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, \ + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, \ + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, \ ControlSerializer *cs) \ { \ return deserialize(dataBegin, dataEnd, cs); \ @@ -249,7 +251,7 @@ DEFINE_POD_SERIALIZER(double) * function parameter serdes). */ template<> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> IPADataSerializer<std::string>::serialize(const std::string &data, [[maybe_unused]] ControlSerializer *cs) { @@ -276,7 +278,7 @@ IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator template<> std::string IPADataSerializer<std::string>::deserialize(const std::vector<uint8_t> &data, - [[maybe_unused]] const std::vector<int32_t> &fds, + [[maybe_unused]] const std::vector<FileDescriptor> &fds, [[maybe_unused]] ControlSerializer *cs) { return { data.cbegin(), data.cend() }; @@ -286,8 +288,8 @@ template<> std::string IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, [[maybe_unused]] ControlSerializer *cs) { return { dataBegin, dataEnd }; @@ -305,7 +307,7 @@ IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator * be used. The serialized ControlInfoMap will have zero length. */ template<> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> IPADataSerializer<ControlList>::serialize(const ControlList &data, ControlSerializer *cs) { if (!cs) @@ -405,7 +407,7 @@ IPADataSerializer<ControlList>::deserialize(const std::vector<uint8_t> &data, template<> ControlList IPADataSerializer<ControlList>::deserialize(const std::vector<uint8_t> &data, - [[maybe_unused]] const std::vector<int32_t> &fds, + [[maybe_unused]] const std::vector<FileDescriptor> &fds, ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), cs); @@ -415,8 +417,8 @@ template<> ControlList IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, ControlSerializer *cs) { return deserialize(dataBegin, dataEnd, cs); @@ -429,7 +431,7 @@ IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator * X bytes - Serialized ControlInfoMap (using ControlSerializer) */ template<> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> IPADataSerializer<ControlInfoMap>::serialize(const ControlInfoMap &map, ControlSerializer *cs) { @@ -491,7 +493,7 @@ IPADataSerializer<ControlInfoMap>::deserialize(const std::vector<uint8_t> &data, template<> ControlInfoMap IPADataSerializer<ControlInfoMap>::deserialize(const std::vector<uint8_t> &data, - [[maybe_unused]] const std::vector<int32_t> &fds, + [[maybe_unused]] const std::vector<FileDescriptor> &fds, ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), cs); @@ -501,15 +503,15 @@ template<> ControlInfoMap IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, ControlSerializer *cs) { return deserialize(dataBegin, dataEnd, cs); } /* - * FileDescriptors are serialized into a single byte that tells if the + * FileDescriptors are serialized into four bytes that tells if the * FileDescriptor is valid or not. If it is valid, then for serialization * the fd will be written to the fd vector, or for deserialization the * fd vector const_iterator will be valid. @@ -517,42 +519,47 @@ IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_itera * This validity is necessary so that we don't send -1 fd over sendmsg(). It * also allows us to simply send the entire fd vector into the deserializer * and it will be recursively consumed as necessary. - * - * \todo Consider serializing the FileDescriptor in 4 bytes to ensure - * 32-bit alignment of all serialized data */ template<> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> IPADataSerializer<FileDescriptor>::serialize(const FileDescriptor &data, [[maybe_unused]] ControlSerializer *cs) { - std::vector<uint8_t> dataVec = { data.isValid() }; - std::vector<int32_t> fdVec; + std::vector<uint8_t> dataVec; + std::vector<FileDescriptor> fdVec; + + /* + * Store as uint32_t to prepare for conversion from validity flag + * to index, and for alignment. + */ + appendPOD<uint32_t>(dataVec, data.isValid()); + if (data.isValid()) - fdVec.push_back(data.fd()); + fdVec.push_back(data); + return { dataVec, fdVec }; } template<> -FileDescriptor IPADataSerializer<FileDescriptor>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, - std::vector<uint8_t>::const_iterator dataEnd, - std::vector<int32_t>::const_iterator fdsBegin, - std::vector<int32_t>::const_iterator fdsEnd, +FileDescriptor IPADataSerializer<FileDescriptor>::deserialize([[maybe_unused]] std::vector<uint8_t>::const_iterator dataBegin, + [[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd, + std::vector<FileDescriptor>::const_iterator fdsBegin, + std::vector<FileDescriptor>::const_iterator fdsEnd, [[maybe_unused]] ControlSerializer *cs) { - ASSERT(std::distance(dataBegin, dataEnd) >= 1); + ASSERT(std::distance(dataBegin, dataEnd) >= 4); - bool valid = !!(*dataBegin); + uint32_t valid = readPOD<uint32_t>(dataBegin, 0, dataEnd); ASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1)); - return valid ? FileDescriptor(*fdsBegin) : FileDescriptor(); + return valid ? *fdsBegin : FileDescriptor(); } template<> FileDescriptor IPADataSerializer<FileDescriptor>::deserialize(const std::vector<uint8_t> &data, - const std::vector<int32_t> &fds, + const std::vector<FileDescriptor> &fds, [[maybe_unused]] ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end()); @@ -561,19 +568,19 @@ FileDescriptor IPADataSerializer<FileDescriptor>::deserialize(const std::vector< /* * FrameBuffer::Plane is serialized as: * - * 1 byte - FileDescriptor + * 4 byte - FileDescriptor * 4 bytes - uint32_t Length */ template<> -std::tuple<std::vector<uint8_t>, std::vector<int32_t>> +std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> IPADataSerializer<FrameBuffer::Plane>::serialize(const FrameBuffer::Plane &data, [[maybe_unused]] ControlSerializer *cs) { std::vector<uint8_t> dataVec; - std::vector<int32_t> fdsVec; + std::vector<FileDescriptor> fdsVec; std::vector<uint8_t> fdBuf; - std::vector<int32_t> fdFds; + std::vector<FileDescriptor> fdFds; std::tie(fdBuf, fdFds) = IPADataSerializer<FileDescriptor>::serialize(data.fd); dataVec.insert(dataVec.end(), fdBuf.begin(), fdBuf.end()); @@ -588,15 +595,15 @@ template<> FrameBuffer::Plane IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, - std::vector<int32_t>::const_iterator fdsBegin, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, + std::vector<FileDescriptor>::const_iterator fdsBegin, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, [[maybe_unused]] ControlSerializer *cs) { FrameBuffer::Plane ret; - ret.fd = IPADataSerializer<FileDescriptor>::deserialize(dataBegin, dataBegin + 1, + ret.fd = IPADataSerializer<FileDescriptor>::deserialize(dataBegin, dataBegin + 4, fdsBegin, fdsBegin + 1); - ret.length = readPOD<uint32_t>(dataBegin, 1, dataEnd); + ret.length = readPOD<uint32_t>(dataBegin, 4, dataEnd); return ret; } @@ -604,7 +611,7 @@ IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_i template<> FrameBuffer::Plane IPADataSerializer<FrameBuffer::Plane>::deserialize(const std::vector<uint8_t> &data, - const std::vector<int32_t> &fds, + const std::vector<FileDescriptor> &fds, ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); diff --git a/src/libcamera/ipc_pipe.cpp b/src/libcamera/ipc_pipe.cpp index 28e20e03..84136a82 100644 --- a/src/libcamera/ipc_pipe.cpp +++ b/src/libcamera/ipc_pipe.cpp @@ -77,13 +77,17 @@ IPCMessage::IPCMessage(const Header &header) * * This essentially converts an IPCUnixSocket payload into an IPCMessage. * The header is extracted from the payload into the IPCMessage's header field. + * + * If the IPCUnixSocket payload had any valid file descriptors, then they will + * all be invalidated. */ -IPCMessage::IPCMessage(const IPCUnixSocket::Payload &payload) +IPCMessage::IPCMessage(IPCUnixSocket::Payload &payload) { memcpy(&header_, payload.data.data(), sizeof(header_)); data_ = std::vector<uint8_t>(payload.data.begin() + sizeof(header_), payload.data.end()); - fds_ = payload.fds; + for (int32_t &fd : payload.fds) + fds_.push_back(FileDescriptor(std::move(fd))); } /** @@ -104,7 +108,9 @@ IPCUnixSocket::Payload IPCMessage::payload() const /* \todo Make this work without copy */ memcpy(payload.data.data() + sizeof(Header), data_.data(), data_.size()); - payload.fds = fds_; + + for (const FileDescriptor &fd : fds_) + payload.fds.push_back(fd.fd()); return payload; } diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp index bf7e34e2..eca19a66 100644 --- a/test/serialization/ipa_data_serializer_test.cpp +++ b/test/serialization/ipa_data_serializer_test.cpp @@ -53,7 +53,7 @@ template<typename T> int testPodSerdes(T in) { std::vector<uint8_t> buf; - std::vector<int32_t> fds; + std::vector<FileDescriptor> fds; std::tie(buf, fds) = IPADataSerializer<T>::serialize(in); T out = IPADataSerializer<T>::deserialize(buf, fds); @@ -72,7 +72,7 @@ int testVectorSerdes(const std::vector<T> &in, ControlSerializer *cs = nullptr) { std::vector<uint8_t> buf; - std::vector<int32_t> fds; + std::vector<FileDescriptor> fds; std::tie(buf, fds) = IPADataSerializer<std::vector<T>>::serialize(in, cs); std::vector<T> out = IPADataSerializer<std::vector<T>>::deserialize(buf, fds, cs); @@ -92,7 +92,7 @@ int testMapSerdes(const std::map<K, V> &in, ControlSerializer *cs = nullptr) { std::vector<uint8_t> buf; - std::vector<int32_t> fds; + std::vector<FileDescriptor> fds; std::tie(buf, fds) = IPADataSerializer<std::map<K, V>>::serialize(in, cs); std::map<K, V> out = IPADataSerializer<std::map<K, V>>::deserialize(buf, fds, cs); @@ -219,7 +219,7 @@ private: }; std::vector<uint8_t> buf; - std::vector<int32_t> fds; + std::vector<FileDescriptor> fds; if (testVectorSerdes(vecUint8) != TestPass) return TestFail; @@ -291,7 +291,7 @@ private: { { "a", { 1, 2, 3 } }, { "b", { 4, 5, 6 } }, { "c", { 7, 8, 9 } } }; std::vector<uint8_t> buf; - std::vector<int32_t> fds; + std::vector<FileDescriptor> fds; if (testMapSerdes(mapUintStr) != TestPass) return TestFail; @@ -359,7 +359,7 @@ private: std::string strEmpty = ""; std::vector<uint8_t> buf; - std::vector<int32_t> fds; + std::vector<FileDescriptor> fds; if (testPodSerdes(u32min) != TestPass) return TestFail; diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl index a4e008c7..aab1fffb 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl @@ -236,7 +236,7 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) void {{proxy_name}}::{{method.mojom_name}}IPC( std::vector<uint8_t>::const_iterator data, size_t dataSize, - [[maybe_unused]] const std::vector<int32_t> &fds) + [[maybe_unused]] const std::vector<FileDescriptor> &fds) { {%- for param in method.parameters %} {{param|name}} {{param.mojom_name}}; diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl index c222f5f2..1979e68f 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl @@ -65,7 +65,7 @@ private: void {{method.mojom_name}}IPC( std::vector<uint8_t>::const_iterator data, size_t dataSize, - const std::vector<int32_t> &fds); + const std::vector<FileDescriptor> &fds); {% endfor %} /* Helper class to invoke async functions in another thread. */ diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl index ea9cc86b..ebcd2aaa 100644 --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl @@ -54,7 +54,7 @@ {%- for param in params %} std::vector<uint8_t> {{param.mojom_name}}Buf; {%- if param|has_fd %} - std::vector<int32_t> {{param.mojom_name}}Fds; + std::vector<FileDescriptor> {{param.mojom_name}}Fds; std::tie({{param.mojom_name}}Buf, {{param.mojom_name}}Fds) = {%- else %} std::tie({{param.mojom_name}}Buf, std::ignore) = diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl index d8d55807..b8ef8e7b 100644 --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl @@ -40,7 +40,7 @@ retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end()); {%- elif field|is_fd %} std::vector<uint8_t> {{field.mojom_name}}; - std::vector<int32_t> {{field.mojom_name}}Fds; + std::vector<FileDescriptor> {{field.mojom_name}}Fds; std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) = IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}); retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end()); @@ -58,7 +58,7 @@ {%- elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %} std::vector<uint8_t> {{field.mojom_name}}; {%- if field|has_fd %} - std::vector<int32_t> {{field.mojom_name}}Fds; + std::vector<FileDescriptor> {{field.mojom_name}}Fds; std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) = {%- else %} std::tie({{field.mojom_name}}, std::ignore) = @@ -104,9 +104,9 @@ dataSize -= {{field_size}}; {%- endif %} {% elif field|is_fd %} - {%- set field_size = 1 %} + {%- set field_size = 4 %} {{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}} - ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + 1, n, n + 1, cs); + ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}, n, n + 1, cs); {%- if not loop.last %} m += {{field_size}}; dataSize -= {{field_size}}; @@ -177,7 +177,7 @@ # \a struct. #} {%- macro serializer(struct, namespace) %} - static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> + static std::tuple<std::vector<uint8_t>, std::vector<FileDescriptor>> serialize(const {{struct|name_full}} &data, {%- if struct|needs_control_serializer %} ControlSerializer *cs) @@ -187,7 +187,7 @@ { std::vector<uint8_t> retData; {%- if struct|has_fd %} - std::vector<int32_t> retFds; + std::vector<FileDescriptor> retFds; {%- endif %} {%- for field in struct.fields %} {{serializer_field(field, namespace, loop)}} @@ -210,7 +210,7 @@ {%- macro deserializer_fd(struct, namespace) %} static {{struct|name_full}} deserialize(std::vector<uint8_t> &data, - std::vector<int32_t> &fds, + std::vector<FileDescriptor> &fds, {%- if struct|needs_control_serializer %} ControlSerializer *cs) {%- else %} @@ -224,8 +224,8 @@ static {{struct|name_full}} deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, - std::vector<int32_t>::const_iterator fdsBegin, - std::vector<int32_t>::const_iterator fdsEnd, + std::vector<FileDescriptor>::const_iterator fdsBegin, + std::vector<FileDescriptor>::const_iterator fdsEnd, {%- if struct|needs_control_serializer %} ControlSerializer *cs) {%- else %} @@ -234,7 +234,7 @@ { {{struct|name_full}} ret; std::vector<uint8_t>::const_iterator m = dataBegin; - std::vector<int32_t>::const_iterator n = fdsBegin; + std::vector<FileDescriptor>::const_iterator n = fdsBegin; size_t dataSize = std::distance(dataBegin, dataEnd); [[maybe_unused]] size_t fdsSize = std::distance(fdsBegin, fdsEnd); @@ -255,7 +255,7 @@ {%- macro deserializer_fd_simple(struct, namespace) %} static {{struct|name_full}} deserialize(std::vector<uint8_t> &data, - [[maybe_unused]] std::vector<int32_t> &fds, + [[maybe_unused]] std::vector<FileDescriptor> &fds, ControlSerializer *cs = nullptr) { return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs); @@ -264,8 +264,8 @@ static {{struct|name_full}} deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, - [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsBegin, + [[maybe_unused]] std::vector<FileDescriptor>::const_iterator fdsEnd, ControlSerializer *cs = nullptr) { return IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs);