[0/7] media: string truncate warnings: fix low-hanging fruit

Message

Hans Verkuil Sept. 22, 2023, 10:50 a.m. UTC

While going through the string truncate warnings I found several that
were easy to fix.

The remainder of the warnings (about 33) are all of this type:

	char name1[32];
	char name2[32];

	snprintf(name2, sizeof(name2), "foo:%s", name1);

Since the [32] part of the character arrays is related to uAPI structures,
this is not so easy to fix.

One option might be to create a helper function to do the concatenation
and that warns (once) if actual truncation takes place. Since in most
case the strings are short enough.

Even if we increase the size to e.g. 64 in the uAPI to avoid some of the
current truncates, that will still cause the same warning, so a helper
function that is smarter would probably still be needed.

Comments/ideas are welcome.

Regards,

	Hans

Hans Verkuil (7):
  media: allegro-dvt: increase buffer size in msg_type_name()
  media: cadence: increase buffer size in csi2tx_get_resources()
  media: atomisp: ia_ccs_debug.c: increase enable_info buffer
  media: vivid: avoid integer overflow
  media: ipu-bridge: increase sensor_name size
  media: cx18: increase in_workq_name size
  media: rc: ati_remote: increase mouse_name buffer size

 drivers/media/pci/cx18/cx18-driver.h                            | 2 +-
 drivers/media/platform/allegro-dvt/allegro-mail.c               | 2 +-
 drivers/media/platform/cadence/cdns-csi2tx.c                    | 2 +-
 drivers/media/rc/ati_remote.c                                   | 2 +-
 drivers/media/test-drivers/vivid/vivid-rds-gen.c                | 2 +-
 .../staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c  | 2 +-
 include/media/ipu-bridge.h                                      | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

Comments

Arnd Bergmann Sept. 22, 2023, 11:21 a.m. UTC | #1

On Fri, Sep 22, 2023, at 06:50, Hans Verkuil wrote:
> While going through the string truncate warnings I found several that
> were easy to fix.

These all look good to me:

Acked-by: Arnd Bergmann <[e-mail]>

> The remainder of the warnings (about 33) are all of this type:
>
> 	char name1[32];
> 	char name2[32];
>
> 	snprintf(name2, sizeof(name2), "foo:%s", name1);
>
> Since the [32] part of the character arrays is related to uAPI structures,
> this is not so easy to fix.
>
> One option might be to create a helper function to do the concatenation
> and that warns (once) if actual truncation takes place. Since in most
> case the strings are short enough.
>
> Even if we increase the size to e.g. 64 in the uAPI to avoid some of the
> current truncates, that will still cause the same warning, so a helper
> function that is smarter would probably still be needed.

What are the affected user space interfaces here? It looks like
most of the remaining ones are about the name fields of v4l2_device
and v4l2_subdev, right? I'm probably missing something here, but
as far as I can tell, these are primarily used inside of the
kernel for debug messages that would actually work just as well
with longer strings.

Interfaces that I found that uses the hardcoded name length
is VIDIOC_DBG_G_CHIP_INFO and VIDIOC_G_AUDOUT. At the moment,
these work on the truncated string, which in theory might
be ambiguous. If we use the longer string internally, the
ambiguity could be pushed down into these ioctls specifically,
in one of several ways:

- only compare the first 32 bytes, but warn if the kernel
  side string is longer at the time we call the ioctl

- add a replacement ioctl command for each affected interface,
  using a longer string and possibly addressing other
  problems with the interface at the same time.

- if there is a realistic chance that the strings are not
  unique, find a mangling scheme that converts the long-form
  name into a shorter name that is actually unique.

      Arnd