libcamera: Avoid variable-length arrays

Unlike in C where they have been standardized since C99, variable-length
arrays in C++ are an extension supported by gcc and clang. Clang started
warning about this with -Wall in version 18:

src/libcamera/ipc_unixsocket.cpp:250:11: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
  250 |         char buf[CMSG_SPACE(num * sizeof(uint32_t))];
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

One simple option is to disable the warning. However, usage of VLAs in
C++ is discouraged by some, usually due to security reasons, based on
the rationale that developers are often unaware of unintentional use of
VLAs and how they may affect the security of the code when the array
size is not properly validated.

This rationale may sound dubious, as the most commonly proposed fix is
to replace VLAs with vectors (or just arrays dynamically allocated with
new() wrapped in unique pointers), without adding any size validation.
This will not produce much better results. However, keeping the VLA
warning and converting the code to dynamic allocation may still be
slightly better, as it can prompt developers to notice VLAs and check if
size validation is required.

For these reasons, convert all VLAs to std::vector. Most of the VLAs
don't need extra size validation, as the size is bound through different
constraints (e.g. image width for line buffers). An arguable exception
may be the buffers in IPCUnixSocket::sendData() and
IPCUnixSocket::recvData() as the number of fds is not bound-checked
locally, but we will run out of file descriptors before we could
overflow the buffer size calculation.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
This commit is contained in:
Laurent Pinchart 2024-07-29 20:16:11 +03:00
parent d5cbf69a7f
commit 7f33dfc100
6 changed files with 27 additions and 22 deletions

View file

@ -125,7 +125,7 @@ void EncoderLibJpeg::compressRGB(const std::vector<Span<uint8_t>> &planes)
*/ */
void EncoderLibJpeg::compressNV(const std::vector<Span<uint8_t>> &planes) void EncoderLibJpeg::compressNV(const std::vector<Span<uint8_t>> &planes)
{ {
uint8_t tmprowbuf[compress_.image_width * 3]; std::vector<uint8_t> tmprowbuf(compress_.image_width * 3);
/* /*
* \todo Use the raw api, and only unpack the cb/cr samples to new line * \todo Use the raw api, and only unpack the cb/cr samples to new line
@ -149,10 +149,10 @@ void EncoderLibJpeg::compressNV(const std::vector<Span<uint8_t>> &planes)
const unsigned char *src_c = planes[1].data(); const unsigned char *src_c = planes[1].data();
JSAMPROW row_pointer[1]; JSAMPROW row_pointer[1];
row_pointer[0] = &tmprowbuf[0]; row_pointer[0] = tmprowbuf.data();
for (unsigned int y = 0; y < compress_.image_height; y++) { for (unsigned int y = 0; y < compress_.image_height; y++) {
unsigned char *dst = &tmprowbuf[0]; unsigned char *dst = tmprowbuf.data();
const unsigned char *src_y = src + y * y_stride; const unsigned char *src_y = src + y * y_stride;
const unsigned char *src_cb = src_c + (y / vertSubSample) * c_stride + cb_pos; const unsigned char *src_cb = src_c + (y / vertSubSample) * c_stride + cb_pos;

View file

@ -11,6 +11,7 @@
#include <endian.h> #include <endian.h>
#include <iostream> #include <iostream>
#include <map> #include <map>
#include <vector>
#include <tiffio.h> #include <tiffio.h>
@ -544,7 +545,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,
* or a thumbnail scanline. The latter will always be much smaller than * or a thumbnail scanline. The latter will always be much smaller than
* the former as we downscale by 16 in both directions. * the former as we downscale by 16 in both directions.
*/ */
uint8_t scanline[(config.size.width * info->bitsPerSample + 7) / 8]; std::vector<uint8_t> scanline((config.size.width * info->bitsPerSample + 7) / 8);
toff_t rawIFDOffset = 0; toff_t rawIFDOffset = 0;
toff_t exifIFDOffset = 0; toff_t exifIFDOffset = 0;
@ -644,10 +645,10 @@ int DNGWriter::write(const char *filename, const Camera *camera,
/* Write the thumbnail. */ /* Write the thumbnail. */
const uint8_t *row = static_cast<const uint8_t *>(data); const uint8_t *row = static_cast<const uint8_t *>(data);
for (unsigned int y = 0; y < config.size.height / 16; y++) { for (unsigned int y = 0; y < config.size.height / 16; y++) {
info->thumbScanline(*info, &scanline, row, info->thumbScanline(*info, scanline.data(), row,
config.size.width / 16, config.stride); config.size.width / 16, config.stride);
if (TIFFWriteScanline(tif, &scanline, y, 0) != 1) { if (TIFFWriteScanline(tif, scanline.data(), y, 0) != 1) {
std::cerr << "Failed to write thumbnail scanline" std::cerr << "Failed to write thumbnail scanline"
<< std::endl; << std::endl;
TIFFClose(tif); TIFFClose(tif);
@ -747,9 +748,9 @@ int DNGWriter::write(const char *filename, const Camera *camera,
/* Write RAW content. */ /* Write RAW content. */
row = static_cast<const uint8_t *>(data); row = static_cast<const uint8_t *>(data);
for (unsigned int y = 0; y < config.size.height; y++) { for (unsigned int y = 0; y < config.size.height; y++) {
info->packScanline(&scanline, row, config.size.width); info->packScanline(scanline.data(), row, config.size.width);
if (TIFFWriteScanline(tif, &scanline, y, 0) != 1) { if (TIFFWriteScanline(tif, scanline.data(), y, 0) != 1) {
std::cerr << "Failed to write RAW scanline" std::cerr << "Failed to write RAW scanline"
<< std::endl; << std::endl;
TIFFClose(tif); TIFFClose(tif);

View file

@ -10,6 +10,7 @@
#include <iomanip> #include <iomanip>
#include <iostream> #include <iostream>
#include <string.h> #include <string.h>
#include <vector>
#include "options.h" #include "options.h"
@ -879,8 +880,8 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)
* Allocate short and long options arrays large enough to contain all * Allocate short and long options arrays large enough to contain all
* options. * options.
*/ */
char shortOptions[optionsMap_.size() * 3 + 2]; std::vector<char> shortOptions(optionsMap_.size() * 3 + 2);
struct option longOptions[optionsMap_.size() + 1]; std::vector<struct option> longOptions(optionsMap_.size() + 1);
unsigned int ids = 0; unsigned int ids = 0;
unsigned int idl = 0; unsigned int idl = 0;
@ -922,7 +923,8 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)
opterr = 0; opterr = 0;
while (true) { while (true) {
int c = getopt_long(argc, argv, shortOptions, longOptions, nullptr); int c = getopt_long(argc, argv, shortOptions.data(),
longOptions.data(), nullptr);
if (c == -1) if (c == -1)
break; break;

View file

@ -9,6 +9,7 @@
#include <functional> #include <functional>
#include <math.h> #include <math.h>
#include <numeric> #include <numeric>
#include <vector>
#include <libcamera/base/log.h> #include <libcamera/base/log.h>
#include <libcamera/base/span.h> #include <libcamera/base/span.h>
@ -496,8 +497,9 @@ void resampleCalTable(const Array2D<double> &calTableIn,
* Precalculate and cache the x sampling locations and phases to save * Precalculate and cache the x sampling locations and phases to save
* recomputing them on every row. * recomputing them on every row.
*/ */
int xLo[X], xHi[X]; std::vector<int> xLo(X);
double xf[X]; std::vector<int> xHi(X);
std::vector<double> xf(X);
double scaleX = cameraMode.sensorWidth / double scaleX = cameraMode.sensorWidth /
(cameraMode.width * cameraMode.scaleX); (cameraMode.width * cameraMode.scaleX);
double xOff = cameraMode.cropX / (double)cameraMode.sensorWidth; double xOff = cameraMode.cropX / (double)cameraMode.sensorWidth;

View file

@ -12,6 +12,7 @@
#include <string.h> #include <string.h>
#include <sys/socket.h> #include <sys/socket.h>
#include <unistd.h> #include <unistd.h>
#include <vector>
#include <libcamera/base/event_notifier.h> #include <libcamera/base/event_notifier.h>
#include <libcamera/base/log.h> #include <libcamera/base/log.h>
@ -247,10 +248,9 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,
iov[0].iov_base = const_cast<void *>(buffer); iov[0].iov_base = const_cast<void *>(buffer);
iov[0].iov_len = length; iov[0].iov_len = length;
char buf[CMSG_SPACE(num * sizeof(uint32_t))]; std::vector<uint8_t> buf(CMSG_SPACE(num * sizeof(uint32_t)));
memset(buf, 0, sizeof(buf));
struct cmsghdr *cmsg = (struct cmsghdr *)buf; struct cmsghdr *cmsg = reinterpret_cast<struct cmsghdr *>(buf.data());
cmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t)); cmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));
cmsg->cmsg_level = SOL_SOCKET; cmsg->cmsg_level = SOL_SOCKET;
cmsg->cmsg_type = SCM_RIGHTS; cmsg->cmsg_type = SCM_RIGHTS;
@ -283,10 +283,9 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,
iov[0].iov_base = buffer; iov[0].iov_base = buffer;
iov[0].iov_len = length; iov[0].iov_len = length;
char buf[CMSG_SPACE(num * sizeof(uint32_t))]; std::vector<uint8_t> buf(CMSG_SPACE(num * sizeof(uint32_t)));
memset(buf, 0, sizeof(buf));
struct cmsghdr *cmsg = (struct cmsghdr *)buf; struct cmsghdr *cmsg = reinterpret_cast<struct cmsghdr *>(buf.data());
cmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t)); cmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));
cmsg->cmsg_level = SOL_SOCKET; cmsg->cmsg_level = SOL_SOCKET;
cmsg->cmsg_type = SCM_RIGHTS; cmsg->cmsg_type = SCM_RIGHTS;

View file

@ -15,6 +15,7 @@
#include <sys/types.h> #include <sys/types.h>
#include <sys/wait.h> #include <sys/wait.h>
#include <unistd.h> #include <unistd.h>
#include <vector>
#include <libcamera/base/event_dispatcher.h> #include <libcamera/base/event_dispatcher.h>
#include <libcamera/base/thread.h> #include <libcamera/base/thread.h>
@ -340,14 +341,14 @@ protected:
for (unsigned int i = 0; i < std::size(strings); i++) { for (unsigned int i = 0; i < std::size(strings); i++) {
unsigned int len = strlen(strings[i]); unsigned int len = strlen(strings[i]);
char buf[len]; std::vector<char> buf(len);
close(fds[i]); close(fds[i]);
if (read(response.fds[0], &buf, len) <= 0) if (read(response.fds[0], buf.data(), len) <= 0)
return TestFail; return TestFail;
if (memcmp(buf, strings[i], len)) if (memcmp(buf.data(), strings[i], len))
return TestFail; return TestFail;
} }