From patchwork Tue Jul 30 23:27:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 20721 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 1157DC323E for ; Tue, 30 Jul 2024 23:27:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 73FA463378; Wed, 31 Jul 2024 01:27:33 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="n9QO6vou"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BFCC96336E for ; Wed, 31 Jul 2024 01:27:30 +0200 (CEST) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 344876EF for ; Wed, 31 Jul 2024 01:26:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1722382003; bh=Cosr2DwOfC5eFGZ2Uf2RqFkwfNb7rUrl+LsDLI5vKtc=; h=From:To:Subject:Date:In-Reply-To:References:From; b=n9QO6vouI5aiKJExSSsYQYGpAQZfSjPZ3nU2ix/HscpNgH3zZ6FSo30giiGBqTYXO TBwpzHCxN/yLyZ9h1h/sxVu8p4M1t+TWfH7NS+6PBtKW59p+L+B1ufir+lKxHL3QkW 4KphVlQrCE8zuUpEmkfon4blPbOuVhR8PnvLOEn0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Subject: [PATCH 1/3] libcamera: base: Add MemFd helper class Date: Wed, 31 Jul 2024 02:27:06 +0300 Message-ID: <20240730232708.17399-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.44.2 In-Reply-To: <20240730232708.17399-1-laurent.pinchart@ideasonboard.com> References: <20240730232708.17399-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" libcamera creates memfds in two locations already, duplicating some code. Move the code to a new MemFd helper class. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal Reviewed-by: Kieran Bingham --- include/libcamera/base/memfd.h | 34 +++++++++ include/libcamera/base/meson.build | 1 + src/libcamera/base/memfd.cpp | 112 ++++++++++++++++++++++++++++ src/libcamera/base/meson.build | 1 + src/libcamera/dma_buf_allocator.cpp | 46 +----------- src/libcamera/shared_mem_object.cpp | 21 ++---- 6 files changed, 157 insertions(+), 58 deletions(-) create mode 100644 include/libcamera/base/memfd.h create mode 100644 src/libcamera/base/memfd.cpp diff --git a/include/libcamera/base/memfd.h b/include/libcamera/base/memfd.h new file mode 100644 index 000000000000..b0edd2de5d83 --- /dev/null +++ b/include/libcamera/base/memfd.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Ideas on Board Oy + * + * Anonymous file creation + */ + +#pragma once + +#include + +#include +#include + +namespace libcamera { + +class MemFd +{ +public: + enum class Seal { + None = 0, + Shrink = (1 << 0), + Grow = (1 << 1), + }; + + using Seals = Flags; + + static UniqueFD create(const char *name, std::size_t size, + Seals seals = Seal::None); +}; + +LIBCAMERA_FLAGS_ENABLE_OPERATORS(MemFd::Seal) + +} /* namespace libcamera */ diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build index bace25d56b13..2a0cee317204 100644 --- a/include/libcamera/base/meson.build +++ b/include/libcamera/base/meson.build @@ -21,6 +21,7 @@ libcamera_base_private_headers = files([ 'event_notifier.h', 'file.h', 'log.h', + 'memfd.h', 'message.h', 'mutex.h', 'private.h', diff --git a/src/libcamera/base/memfd.cpp b/src/libcamera/base/memfd.cpp new file mode 100644 index 000000000000..0d50e2d64638 --- /dev/null +++ b/src/libcamera/base/memfd.cpp @@ -0,0 +1,112 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2020, Google Inc. + * + * Anonymous file creation + */ + +#include + +#include +#include +#include +#include +#include + +#include + +/** + * \file base/memfd.h + * \brief Anonymous file creation + */ + +/* uClibc doesn't provide the file sealing API. */ +#ifndef __DOXYGEN__ +#if not HAVE_FILE_SEALS +#define F_ADD_SEALS 1033 +#define F_SEAL_SHRINK 0x0002 +#define F_SEAL_GROW 0x0004 +#endif +#endif + +namespace libcamera { + +LOG_DECLARE_CATEGORY(File) + +/** + * \class MemFd + * \brief Helper class to created anonymous files + */ + +/** + * \enum MemFd::Seal + * \brief Seals for the MemFd::create() function + * \var MemFd::Seal::None + * \brief No seals (used as default value) + * \var MemFd::Seal::Shrink + * \brief Prevent the memfd from shrinking + * \var MemFd::Seal::Grow + * \brief Prevent the memfd from growing + */ + +/** + * \typedef MemFd::Seals + * \brief A bitwise combination of MemFd::Seal values + */ + +/** + * \brief Create an anonymous file + * \param[in] name The file name (displayed in symbolic links in /proc/self/fd/) + * \param[in] size The file size + * \param[in] seals The file seals + * \return The descriptor of the anonymous file is creation succeeded, or an + * + * This function is a helper that wraps anonymous file (memfd) creation and + * sets the file size and optional seals. + * + * invalid UniqueFD otherwise + */ +UniqueFD MemFd::create(const char *name, std::size_t size, Seals seals) +{ +#if HAVE_MEMFD_CREATE + int ret = memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC); +#else + int ret = syscall(SYS_memfd_create, name, MFD_ALLOW_SEALING | MFD_CLOEXEC); +#endif + if (ret < 0) { + ret = errno; + LOG(File, Error) + << "Failed to allocate memfd storage for " << name + << ": " << strerror(ret); + return {}; + } + + UniqueFD memfd(ret); + + ret = ftruncate(memfd.get(), size); + if (ret < 0) { + ret = errno; + LOG(File, Error) + << "Failed to set memfd size for " << name + << ": " << strerror(ret); + return {}; + } + + if (seals) { + int fileSeals = (seals & Seal::Shrink ? F_SEAL_SHRINK : 0) + | (seals & Seal::Grow ? F_SEAL_GROW : 0); + + ret = fcntl(memfd.get(), F_ADD_SEALS, fileSeals); + if (ret < 0) { + ret = errno; + LOG(File, Error) + << "Failed to seal the memfd for " << name + << ": " << strerror(ret); + return {}; + } + } + + return memfd; +} + +} /* namespace libcamera */ diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build index 7a7fd7e4ca87..4a228d335ba4 100644 --- a/src/libcamera/base/meson.build +++ b/src/libcamera/base/meson.build @@ -10,6 +10,7 @@ libcamera_base_sources = files([ 'file.cpp', 'flags.cpp', 'log.cpp', + 'memfd.cpp', 'message.cpp', 'mutex.cpp', 'object.cpp', diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp index c06eca7d04ec..be6efb89fbb7 100644 --- a/src/libcamera/dma_buf_allocator.cpp +++ b/src/libcamera/dma_buf_allocator.cpp @@ -13,7 +13,6 @@ #include #include #include -#include #include #include @@ -22,6 +21,7 @@ #include #include +#include /** * \file dma_buf_allocator.cpp @@ -126,54 +126,16 @@ DmaBufAllocator::~DmaBufAllocator() = default; * \brief Check if the DmaBufAllocator instance is valid * \return True if the DmaBufAllocator is valid, false otherwise */ - -/* uClibc doesn't provide the file sealing API. */ -#ifndef __DOXYGEN__ -#if not HAVE_FILE_SEALS -#define F_ADD_SEALS 1033 -#define F_SEAL_SHRINK 0x0002 -#endif -#endif - UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size) { /* Size must be a multiple of the page size. Round it up. */ std::size_t pageMask = sysconf(_SC_PAGESIZE) - 1; size = (size + pageMask) & ~pageMask; -#if HAVE_MEMFD_CREATE - int ret = memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC); -#else - int ret = syscall(SYS_memfd_create, name, MFD_ALLOW_SEALING | MFD_CLOEXEC); -#endif - if (ret < 0) { - ret = errno; - LOG(DmaBufAllocator, Error) - << "Failed to allocate memfd storage for " << name - << ": " << strerror(ret); - return {}; - } - - UniqueFD memfd(ret); - - ret = ftruncate(memfd.get(), size); - if (ret < 0) { - ret = errno; - LOG(DmaBufAllocator, Error) - << "Failed to set memfd size for " << name - << ": " << strerror(ret); - return {}; - } - /* udmabuf dma-buffers *must* have the F_SEAL_SHRINK seal. */ - ret = fcntl(memfd.get(), F_ADD_SEALS, F_SEAL_SHRINK); - if (ret < 0) { - ret = errno; - LOG(DmaBufAllocator, Error) - << "Failed to seal the memfd for " << name - << ": " << strerror(ret); + UniqueFD memfd = MemFd::create(name, size, MemFd::Seal::Shrink); + if (!memfd.isValid()) return {}; - } struct udmabuf_create create; @@ -182,7 +144,7 @@ UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size) create.offset = 0; create.size = size; - ret = ::ioctl(providerHandle_.get(), UDMABUF_CREATE, &create); + int ret = ::ioctl(providerHandle_.get(), UDMABUF_CREATE, &create); if (ret < 0) { ret = errno; LOG(DmaBufAllocator, Error) diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp index 809fbdaf95de..022645e71a35 100644 --- a/src/libcamera/shared_mem_object.cpp +++ b/src/libcamera/shared_mem_object.cpp @@ -13,9 +13,9 @@ #include #include #include -#include #include -#include + +#include /** * \file shared_mem_object.cpp @@ -58,22 +58,11 @@ SharedMem::SharedMem() = default; */ SharedMem::SharedMem(const std::string &name, std::size_t size) { -#if HAVE_MEMFD_CREATE - int fd = memfd_create(name.c_str(), MFD_CLOEXEC); -#else - int fd = syscall(SYS_memfd_create, name.c_str(), MFD_CLOEXEC); -#endif - if (fd < 0) + UniqueFD memfd = MemFd::create(name.c_str(), size); + if (!memfd.isValid()) return; - fd_ = SharedFD(std::move(fd)); - if (!fd_.isValid()) - return; - - if (ftruncate(fd_.get(), size) < 0) { - fd_ = SharedFD(); - return; - } + fd_ = SharedFD(std::move(memfd)); void *mem = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd_.get(), 0); From patchwork Tue Jul 30 23:27:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 20722 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 39F2CC323E for ; Tue, 30 Jul 2024 23:27:37 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BBCAB63377; Wed, 31 Jul 2024 01:27:36 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="DUmlP148"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6D31E61984 for ; Wed, 31 Jul 2024 01:27:32 +0200 (CEST) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 966796EF for ; Wed, 31 Jul 2024 01:26:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1722382004; bh=W51WlWVDdlpUexfgjY7CBoQOL0kaXN0ViCbnD43c4Y0=; h=From:To:Subject:Date:In-Reply-To:References:From; b=DUmlP148aknBJs/g7whn0zhQCRdPas63ElRd6VaGnLHZV54wOSWUiVivbqAU3+Wwf MYsEaeo8VVq04/FahmcqVfR2ERkgmQRs/hIOe2DkrLrNM95XWIEhJQwwK6Qx9GG+hD 6PpN38SE39HDVOaINsqNgrYgVR0nxQrtXZ+V9Ego= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Subject: [PATCH 2/3] libcamera: shared_mem_object: Prevent memfd from shrinking or growing Date: Wed, 31 Jul 2024 02:27:07 +0300 Message-ID: <20240730232708.17399-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.44.2 In-Reply-To: <20240730232708.17399-1-laurent.pinchart@ideasonboard.com> References: <20240730232708.17399-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The memfd underlying the SharedMem object must not shrink, or memory corruption will happen. Prevent this by setting the shrink seal on the file. As there's no valid use case for growing the memory either, set the grow seal as well. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal Reviewed-by: Kieran Bingham --- src/libcamera/shared_mem_object.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp index 022645e71a35..d4c7991ad16a 100644 --- a/src/libcamera/shared_mem_object.cpp +++ b/src/libcamera/shared_mem_object.cpp @@ -58,7 +58,8 @@ SharedMem::SharedMem() = default; */ SharedMem::SharedMem(const std::string &name, std::size_t size) { - UniqueFD memfd = MemFd::create(name.c_str(), size); + UniqueFD memfd = MemFd::create(name.c_str(), size, MemFd::Seal::Shrink | + MemFd::Seal::Grow); if (!memfd.isValid()) return; From patchwork Tue Jul 30 23:27:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 20723 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id CD509C323E for ; Tue, 30 Jul 2024 23:27:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7EFFF6336E; Wed, 31 Jul 2024 01:27:39 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="AqkeIz3z"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CF8B86337A for ; Wed, 31 Jul 2024 01:27:33 +0200 (CEST) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 048B07CC for ; Wed, 31 Jul 2024 01:26:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1722382006; bh=gQFPnUywOwUDR/yTiZJ+/lmFzUXhK1yTTFXN794V/fI=; h=From:To:Subject:Date:In-Reply-To:References:From; b=AqkeIz3z5VR6T7y0Xiz5Ncoy83hztJ9kRd/NyZY7sY9ZCOyc/BfxQEmRExtyECupr Nn4s6ma11HZK3WV6nJzm82rDe7v5NQx0zGDf6ML8dYGEtF03sym3lXgJ4eE7EWNA/U dwm9xnu7nfbsSHcfQab7NQJ40utHG2uhyKR4IiCw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Subject: [PATCH 3/3] libcamera: software_isp: Remove file seal TODO item Date: Wed, 31 Jul 2024 02:27:08 +0300 Message-ID: <20240730232708.17399-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.44.2 In-Reply-To: <20240730232708.17399-1-laurent.pinchart@ideasonboard.com> References: <20240730232708.17399-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The file seal TODO item has been addressed. Remove it. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal Reviewed-by: Kieran Bingham --- src/libcamera/software_isp/TODO | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO index 6bdc590531ca..9978afc0317b 100644 --- a/src/libcamera/software_isp/TODO +++ b/src/libcamera/software_isp/TODO @@ -1,22 +1,3 @@ -1. Setting F_SEAL_SHRINK and F_SEAL_GROW after ftruncate() - ->> SharedMem::SharedMem(const std::string &name, std::size_t size) ->> : name_(name), size_(size), mem_(nullptr) ->> ->> ... ->> ->> if (ftruncate(fd_.get(), size_) < 0) ->> return; -> -> Should we set the GROW and SHRINK seals (in a separate patch) ? - -Yes, this can be done. -Setting F_SEAL_SHRINK and F_SEAL_GROW after the ftruncate() call above could catch -some potential errors related to improper access to the shared memory allocated by -the SharedMemObject. - ---- - 2. Reconsider stats sharing >>> +void SwStatsCpu::finishFrame(void)