Message ID | 20250722145334.1777305-1-barnabas.pocze@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
Hi Barnabás, Quoting Barnabás Pőcze (2025-07-22 23:53:34) > Only try to send the "Exit" message if the `ipc_` pointer is non-null. If > the constructor fails, then `ipc_` might remain nullptr, which would lead > to a nullptr dereference in the destructor. > > This change also modifies the constructor so that only a valid `IPCPipeUnixSocket` > object will be saved into the `ipc_` member, which avoids error messages when > `IPCPipeUnixSocket::sendAsync()` is called in the inappropriate state in the > destructor. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=276 > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > changes in v2: > * modify constructor not to save "invalid" IPCPipeUnixSocket into member > > v1: https://patchwork.libcamera.org/patch/23894/ > --- > .../libcamera_templates/module_ipa_proxy.cpp.tmpl | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > index 0f87e7976..18b4ab5e5 100644 > --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > @@ -61,15 +61,16 @@ namespace {{ns}} { > return; > } > > - ipc_ = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(), > - proxyWorkerPath.c_str()); > - if (!ipc_->isConnected()) { > + auto ipc = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(), > + proxyWorkerPath.c_str()); > + if (!ipc->isConnected()) { > LOG(IPAProxy, Error) << "Failed to create IPCPipe"; > return; > } > > - ipc_->recv.connect(this, &{{proxy_name}}::recvMessage); > + ipc->recv.connect(this, &{{proxy_name}}::recvMessage); > > + ipc_ = std::move(ipc); Oh, nice. > valid_ = true; > return; > } > @@ -96,7 +97,7 @@ namespace {{ns}} { > > {{proxy_name}}::~{{proxy_name}}() > { > - if (isolate_) { > + if (ipc_) { Oh ok I see it now. Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > IPCMessage::Header header = > { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ }; > IPCMessage msg(header); > -- > 2.50.1
Quoting Barnabás Pőcze (2025-07-22 15:53:34) > Only try to send the "Exit" message if the `ipc_` pointer is non-null. If > the constructor fails, then `ipc_` might remain nullptr, which would lead > to a nullptr dereference in the destructor. > > This change also modifies the constructor so that only a valid `IPCPipeUnixSocket` > object will be saved into the `ipc_` member, which avoids error messages when > `IPCPipeUnixSocket::sendAsync()` is called in the inappropriate state in the > destructor. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=276 > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > changes in v2: > * modify constructor not to save "invalid" IPCPipeUnixSocket into member > > v1: https://patchwork.libcamera.org/patch/23894/ > --- > .../libcamera_templates/module_ipa_proxy.cpp.tmpl | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > index 0f87e7976..18b4ab5e5 100644 > --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > @@ -61,15 +61,16 @@ namespace {{ns}} { > return; > } > > - ipc_ = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(), > - proxyWorkerPath.c_str()); > - if (!ipc_->isConnected()) { > + auto ipc = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(), > + proxyWorkerPath.c_str()); > + if (!ipc->isConnected()) { > LOG(IPAProxy, Error) << "Failed to create IPCPipe"; > return; > } > > - ipc_->recv.connect(this, &{{proxy_name}}::recvMessage); > + ipc->recv.connect(this, &{{proxy_name}}::recvMessage); > > + ipc_ = std::move(ipc); > valid_ = true; > return; > } > @@ -96,7 +97,7 @@ namespace {{ns}} { > > {{proxy_name}}::~{{proxy_name}}() > { > - if (isolate_) { > + if (ipc_) { > IPCMessage::Header header = > { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ }; > IPCMessage msg(header); > -- > 2.50.1
diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl index 0f87e7976..18b4ab5e5 100644 --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl @@ -61,15 +61,16 @@ namespace {{ns}} { return; } - ipc_ = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(), - proxyWorkerPath.c_str()); - if (!ipc_->isConnected()) { + auto ipc = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(), + proxyWorkerPath.c_str()); + if (!ipc->isConnected()) { LOG(IPAProxy, Error) << "Failed to create IPCPipe"; return; } - ipc_->recv.connect(this, &{{proxy_name}}::recvMessage); + ipc->recv.connect(this, &{{proxy_name}}::recvMessage); + ipc_ = std::move(ipc); valid_ = true; return; } @@ -96,7 +97,7 @@ namespace {{ns}} { {{proxy_name}}::~{{proxy_name}}() { - if (isolate_) { + if (ipc_) { IPCMessage::Header header = { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ }; IPCMessage msg(header);
Only try to send the "Exit" message if the `ipc_` pointer is non-null. If the constructor fails, then `ipc_` might remain nullptr, which would lead to a nullptr dereference in the destructor. This change also modifies the constructor so that only a valid `IPCPipeUnixSocket` object will be saved into the `ipc_` member, which avoids error messages when `IPCPipeUnixSocket::sendAsync()` is called in the inappropriate state in the destructor. Bug: https://bugs.libcamera.org/show_bug.cgi?id=276 Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- changes in v2: * modify constructor not to save "invalid" IPCPipeUnixSocket into member v1: https://patchwork.libcamera.org/patch/23894/ --- .../libcamera_templates/module_ipa_proxy.cpp.tmpl | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) -- 2.50.1