| Message ID | 20240730232708.17399-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, thank you for the patch. Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > 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. Makes sense to me. Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > 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;
Quoting Milan Zamazal (2024-07-31 08:24:29) > Hi Laurent, > > thank you for the patch. > > Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > > > 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. > > Makes sense to me. > > Reviewed-by: Milan Zamazal <mzamazal@redhat.com> Haha, I was going to ask about this in the previous patch - then I discovered this ;-) But should we - with the same logic also add MemFd::Seal::Grow to the allocFromUDmaBuf() implementation too? Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > 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; >
On Wed, Jul 31, 2024 at 09:40:11AM +0100, Kieran Bingham wrote: > Quoting Milan Zamazal (2024-07-31 08:24:29) > > Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > > > > > 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. > > > > Makes sense to me. > > > > Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > > Haha, I was going to ask about this in the previous patch - then I > discovered this ;-) > > But should we - with the same logic also add MemFd::Seal::Grow to the > allocFromUDmaBuf() implementation too? I think we could. Do you want to submit a patch ? :-) > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > 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;
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;
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 <laurent.pinchart@ideasonboard.com> --- src/libcamera/shared_mem_object.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)