| Message ID | 20210818083842.31778-2-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Wed, Aug 18, 2021 at 02:08:41PM +0530, Umang Jain wrote: > In IPCUnixSocket, a payload can be sent/received with empty fd vector, > which leads to passing a nullptr in memcpy() in both sendData() > and recvData(). Add a null check for fd vector's data pointer > to avoid invoking memcpy() with nullptr. > > The issue is noticed by running a test manually testing the vimc > IPA code paths in isolated mode. It is only noticed when the test > is compiled with -Db_sanitize=address,undefined meson built-in option. > > ipc_unixsocket.cpp:268:8: runtime error: null pointer passed as argument 2, which is declared to never be null > ipc_unixsocket.cpp:312:8: runtime error: null pointer passed as argument 1, which is declared to never be null > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/ipc_unixsocket.cpp | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp > index a4ab1a5f..7188cf29 100644 > --- a/src/libcamera/ipc_unixsocket.cpp > +++ b/src/libcamera/ipc_unixsocket.cpp > @@ -260,7 +260,8 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length, > msg.msg_control = cmsg; > msg.msg_controllen = cmsg->cmsg_len; > msg.msg_flags = 0; > - memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t)); > + if (fds) > + memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t)); > > if (sendmsg(fd_, &msg, 0) < 0) { > int ret = -errno; > @@ -304,7 +305,8 @@ int IPCUnixSocket::recvData(void *buffer, size_t length, > return ret; > } > > - memcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t)); > + if (fds) > + memcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t)); > > return 0; > }
Hi Umang, On Wed, Aug 18, 2021 at 02:08:41PM +0530, Umang Jain wrote: > In IPCUnixSocket, a payload can be sent/received with empty fd vector, > which leads to passing a nullptr in memcpy() in both sendData() > and recvData(). Add a null check for fd vector's data pointer > to avoid invoking memcpy() with nullptr. > > The issue is noticed by running a test manually testing the vimc > IPA code paths in isolated mode. It is only noticed when the test > is compiled with -Db_sanitize=address,undefined meson built-in option. > > ipc_unixsocket.cpp:268:8: runtime error: null pointer passed as argument 2, which is declared to never be null > ipc_unixsocket.cpp:312:8: runtime error: null pointer passed as argument 1, which is declared to never be null > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/ipc_unixsocket.cpp | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp > index a4ab1a5f..7188cf29 100644 > --- a/src/libcamera/ipc_unixsocket.cpp > +++ b/src/libcamera/ipc_unixsocket.cpp > @@ -260,7 +260,8 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length, > msg.msg_control = cmsg; > msg.msg_controllen = cmsg->cmsg_len; > msg.msg_flags = 0; > - memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t)); > + if (fds) > + memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t)); > > if (sendmsg(fd_, &msg, 0) < 0) { > int ret = -errno; > @@ -304,7 +305,8 @@ int IPCUnixSocket::recvData(void *buffer, size_t length, > return ret; > } > > - memcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t)); > + if (fds) > + memcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t)); > > return 0; > } > -- > 2.31.0 >
diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp index a4ab1a5f..7188cf29 100644 --- a/src/libcamera/ipc_unixsocket.cpp +++ b/src/libcamera/ipc_unixsocket.cpp @@ -260,7 +260,8 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length, msg.msg_control = cmsg; msg.msg_controllen = cmsg->cmsg_len; msg.msg_flags = 0; - memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t)); + if (fds) + memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t)); if (sendmsg(fd_, &msg, 0) < 0) { int ret = -errno; @@ -304,7 +305,8 @@ int IPCUnixSocket::recvData(void *buffer, size_t length, return ret; } - memcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t)); + if (fds) + memcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t)); return 0; }
In IPCUnixSocket, a payload can be sent/received with empty fd vector, which leads to passing a nullptr in memcpy() in both sendData() and recvData(). Add a null check for fd vector's data pointer to avoid invoking memcpy() with nullptr. The issue is noticed by running a test manually testing the vimc IPA code paths in isolated mode. It is only noticed when the test is compiled with -Db_sanitize=address,undefined meson built-in option. ipc_unixsocket.cpp:268:8: runtime error: null pointer passed as argument 2, which is declared to never be null ipc_unixsocket.cpp:312:8: runtime error: null pointer passed as argument 1, which is declared to never be null Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/libcamera/ipc_unixsocket.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)