| Message ID | cee8080d-ea46-432c-8241-e7ec81ef691d@xs4all.nl (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCHv2] media: cec: extron-da-hd-4k-plus: Fix Wformat-truncation | expand |
Hi Hans
snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC",
port->direction, port->port.port,
cec_msg_initiator(msg), cec_msg_destination(msg), buf);
Will need:
W -> 1 byte
%c -> 1 byte
%u -> 1 byte port.port (we might have to add %10 here in case gcc8 is
not clever enough )
* -> 1 byte
%u -> 2 bytes cec_msg_initiator(msg) (we might have to add %16 here
in case gcc8 is not clever enough )
* -> 1 byte
%u -> 2 bytes cec_msg_destination(msg) (we might have to add %16 here
in case gcc8 is not clever enough )
%s -> CEC_MAX_MSG_SIZE * 3 buf
DCEC -> 4 bytes
\0 -> 1 byte
That is a total of CEC_MAX_MSG_SIZE * 3 + 14. Don't we need to add an
extra byte to cmd?
char cmd[CEC_MAX_MSG_SIZE * 3 + 14];
Regards!
On Tue, 1 Apr 2025 at 10:18, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Change the port type to u8 so gcc8 knows that the port fits in a single
> char.
>
> drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:44: warning: 'DCEC' directive output may be truncated writing 4 bytes into a region of size between 0 and 53 [-Wformat-truncation=]
>
> Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
> Reported-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h | 2 +-
> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c | 2 +-
> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h | 6 +++---
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h
> index 7422f7c5719e..fa2e02b26565 100644
> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h
> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h
> @@ -14,7 +14,7 @@ struct cec_splitter;
> struct cec_splitter_port {
> struct cec_splitter *splitter;
> struct cec_adapter *adap;
> - unsigned int port;
> + u8 port;
> bool is_active_source;
> bool found_sink;
> ktime_t lost_sink_ts;
> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> index cfbfc4c1b2e6..c4add8f3f8b4 100644
> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> @@ -1692,7 +1692,7 @@ static int extron_setup_thread(void *_extron)
> log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_TV;
> } else {
> snprintf(log_addrs.osd_name, sizeof(log_addrs.osd_name),
> - "Splitter Out%u", port->port.port);
> + "Splitter Out%u", port->port.port % 10);
> log_addrs.log_addr_type[0] = CEC_LOG_ADDR_TYPE_PLAYBACK;
> log_addrs.primary_device_type[0] = CEC_OP_PRIM_DEVTYPE_PLAYBACK;
> log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_PLAYBACK;
> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h
> index b79f1253ab5d..cd07e90ea32a 100644
> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h
> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h
> @@ -83,9 +83,9 @@ struct extron {
> struct serio *serio;
> /* locks access to serio */
> struct mutex serio_lock;
> - unsigned int num_ports;
> - unsigned int num_in_ports;
> - unsigned int num_out_ports;
> + u8 num_ports;
> + u8 num_in_ports;
> + u8 num_out_ports;
> char unit_name[32];
> char unit_type[64];
> char unit_fw_version[32];
> --
> 2.47.2
>
On 01/04/2025 10:30, Ricardo Ribalda wrote: > Hi Hans > > snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", > port->direction, port->port.port, > cec_msg_initiator(msg), cec_msg_destination(msg), buf); > > Will need: > W -> 1 byte > %c -> 1 byte > %u -> 1 byte port.port (we might have to add %10 here in case gcc8 is > not clever enough ) > * -> 1 byte > %u -> 2 bytes cec_msg_initiator(msg) (we might have to add %16 here > in case gcc8 is not clever enough ) > * -> 1 byte > %u -> 2 bytes cec_msg_destination(msg) (we might have to add %16 here > in case gcc8 is not clever enough ) > %s -> CEC_MAX_MSG_SIZE * 3 buf Actually, this is at most (CEC_MAX_MSG_SIZE - 1) * 3, since the first byte of the CEC message isn't included in the buffer. So this code is safe. I agree that it could be made a bit clearer, but something for another day. I ran this patch through the CI and it passed the build-ancient job. Regards, Hans > DCEC -> 4 bytes > \0 -> 1 byte > > That is a total of CEC_MAX_MSG_SIZE * 3 + 14. Don't we need to add an > extra byte to cmd? > > char cmd[CEC_MAX_MSG_SIZE * 3 + 14]; > > Regards! > > On Tue, 1 Apr 2025 at 10:18, Hans Verkuil <hverkuil@xs4all.nl> wrote: >> >> Change the port type to u8 so gcc8 knows that the port fits in a single >> char. >> >> drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:44: warning: 'DCEC' directive output may be truncated writing 4 bytes into a region of size between 0 and 53 [-Wformat-truncation=] >> >> Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl> >> Reported-by: Ricardo Ribalda <ribalda@chromium.org> >> --- >> drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h | 2 +- >> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c | 2 +- >> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h | 6 +++--- >> 3 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h >> index 7422f7c5719e..fa2e02b26565 100644 >> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h >> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h >> @@ -14,7 +14,7 @@ struct cec_splitter; >> struct cec_splitter_port { >> struct cec_splitter *splitter; >> struct cec_adapter *adap; >> - unsigned int port; >> + u8 port; >> bool is_active_source; >> bool found_sink; >> ktime_t lost_sink_ts; >> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c >> index cfbfc4c1b2e6..c4add8f3f8b4 100644 >> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c >> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c >> @@ -1692,7 +1692,7 @@ static int extron_setup_thread(void *_extron) >> log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_TV; >> } else { >> snprintf(log_addrs.osd_name, sizeof(log_addrs.osd_name), >> - "Splitter Out%u", port->port.port); >> + "Splitter Out%u", port->port.port % 10); >> log_addrs.log_addr_type[0] = CEC_LOG_ADDR_TYPE_PLAYBACK; >> log_addrs.primary_device_type[0] = CEC_OP_PRIM_DEVTYPE_PLAYBACK; >> log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_PLAYBACK; >> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h >> index b79f1253ab5d..cd07e90ea32a 100644 >> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h >> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h >> @@ -83,9 +83,9 @@ struct extron { >> struct serio *serio; >> /* locks access to serio */ >> struct mutex serio_lock; >> - unsigned int num_ports; >> - unsigned int num_in_ports; >> - unsigned int num_out_ports; >> + u8 num_ports; >> + u8 num_in_ports; >> + u8 num_out_ports; >> char unit_name[32]; >> char unit_type[64]; >> char unit_fw_version[32]; >> -- >> 2.47.2 >> > >
Hi Hans On Tue, 1 Apr 2025 at 10:43, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > On 01/04/2025 10:30, Ricardo Ribalda wrote: > > Hi Hans > > > > snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", > > port->direction, port->port.port, > > cec_msg_initiator(msg), cec_msg_destination(msg), buf); > > > > Will need: > > W -> 1 byte > > %c -> 1 byte > > %u -> 1 byte port.port (we might have to add %10 here in case gcc8 is > > not clever enough ) > > * -> 1 byte > > %u -> 2 bytes cec_msg_initiator(msg) (we might have to add %16 here > > in case gcc8 is not clever enough ) > > * -> 1 byte > > %u -> 2 bytes cec_msg_destination(msg) (we might have to add %16 here > > in case gcc8 is not clever enough ) > > %s -> CEC_MAX_MSG_SIZE * 3 buf > > Actually, this is at most (CEC_MAX_MSG_SIZE - 1) * 3, since the first byte > of the CEC message isn't included in the buffer. > > So this code is safe. > > I agree that it could be made a bit clearer, but something for another day. > > I ran this patch through the CI and it passed the build-ancient job. Please check the full log. I have already added the warning to the "allow-list" to move to 6.14 drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c: In function 'extron_cec_adap_transmit': drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:44: warning: 'DCEC' directive output may be truncated writing 4 bytes into a region of size between 1 and 53 [-Wformat-truncation=] snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", ^~~~ drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:2: note: 'snprintf' output between 13 and 65 bytes into a destination of size 61 snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ port->direction, port->port.port, ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cec_msg_initiator(msg), cec_msg_destination(msg), buf); This does the trick diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c index cfbfc4c1b2e6..c56db19eaf16 100644 --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c @@ -1003,7 +1003,7 @@ static int extron_cec_adap_transmit(struct cec_adapter *adap, u8 attempts, { struct extron_port *port = cec_get_drvdata(adap); char buf[CEC_MAX_MSG_SIZE * 3 + 1]; - char cmd[CEC_MAX_MSG_SIZE * 3 + 13]; + char cmd[CEC_MAX_MSG_SIZE * 3 + 15]; unsigned int i; if (port->disconnected) @@ -1012,7 +1012,7 @@ static int extron_cec_adap_transmit(struct cec_adapter *adap, u8 attempts, for (i = 0; i < msg->len - 1; i++) sprintf(buf + i * 3, "%%%02X", msg->msg[i + 1]); snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", - port->direction, port->port.port, + port->direction, port->port.port % 10, cec_msg_initiator(msg), cec_msg_destination(msg), buf); return extron_send_and_wait(port->extron, port, cmd, NULL); } > > Regards, > > Hans > > > DCEC -> 4 bytes > > \0 -> 1 byte > > > > That is a total of CEC_MAX_MSG_SIZE * 3 + 14. Don't we need to add an > > extra byte to cmd? > > > > char cmd[CEC_MAX_MSG_SIZE * 3 + 14]; > > > > Regards! > > > > On Tue, 1 Apr 2025 at 10:18, Hans Verkuil <hverkuil@xs4all.nl> wrote: > >> > >> Change the port type to u8 so gcc8 knows that the port fits in a single > >> char. > >> > >> drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:44: warning: 'DCEC' directive output may be truncated writing 4 bytes into a region of size between 0 and 53 [-Wformat-truncation=] > >> > >> Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl> > >> Reported-by: Ricardo Ribalda <ribalda@chromium.org> > >> --- > >> drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h | 2 +- > >> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c | 2 +- > >> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h | 6 +++--- > >> 3 files changed, 5 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h > >> index 7422f7c5719e..fa2e02b26565 100644 > >> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h > >> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h > >> @@ -14,7 +14,7 @@ struct cec_splitter; > >> struct cec_splitter_port { > >> struct cec_splitter *splitter; > >> struct cec_adapter *adap; > >> - unsigned int port; > >> + u8 port; > >> bool is_active_source; > >> bool found_sink; > >> ktime_t lost_sink_ts; > >> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > >> index cfbfc4c1b2e6..c4add8f3f8b4 100644 > >> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > >> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > >> @@ -1692,7 +1692,7 @@ static int extron_setup_thread(void *_extron) > >> log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_TV; > >> } else { > >> snprintf(log_addrs.osd_name, sizeof(log_addrs.osd_name), > >> - "Splitter Out%u", port->port.port); > >> + "Splitter Out%u", port->port.port % 10); > >> log_addrs.log_addr_type[0] = CEC_LOG_ADDR_TYPE_PLAYBACK; > >> log_addrs.primary_device_type[0] = CEC_OP_PRIM_DEVTYPE_PLAYBACK; > >> log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_PLAYBACK; > >> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h > >> index b79f1253ab5d..cd07e90ea32a 100644 > >> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h > >> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h > >> @@ -83,9 +83,9 @@ struct extron { > >> struct serio *serio; > >> /* locks access to serio */ > >> struct mutex serio_lock; > >> - unsigned int num_ports; > >> - unsigned int num_in_ports; > >> - unsigned int num_out_ports; > >> + u8 num_ports; > >> + u8 num_in_ports; > >> + u8 num_out_ports; > >> char unit_name[32]; > >> char unit_type[64]; > >> char unit_fw_version[32]; > >> -- > >> 2.47.2 > >> > > > > >
On Tue, 1 Apr 2025 at 11:11, Ricardo Ribalda <ribalda@chromium.org> wrote: > > Hi Hans > > On Tue, 1 Apr 2025 at 10:43, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > > > On 01/04/2025 10:30, Ricardo Ribalda wrote: > > > Hi Hans > > > > > > snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", > > > port->direction, port->port.port, > > > cec_msg_initiator(msg), cec_msg_destination(msg), buf); > > > > > > Will need: > > > W -> 1 byte > > > %c -> 1 byte > > > %u -> 1 byte port.port (we might have to add %10 here in case gcc8 is > > > not clever enough ) > > > * -> 1 byte > > > %u -> 2 bytes cec_msg_initiator(msg) (we might have to add %16 here > > > in case gcc8 is not clever enough ) > > > * -> 1 byte > > > %u -> 2 bytes cec_msg_destination(msg) (we might have to add %16 here > > > in case gcc8 is not clever enough ) > > > %s -> CEC_MAX_MSG_SIZE * 3 buf > > > > Actually, this is at most (CEC_MAX_MSG_SIZE - 1) * 3, since the first byte > > of the CEC message isn't included in the buffer. > > > > So this code is safe. > > > > I agree that it could be made a bit clearer, but something for another day. > > > > I ran this patch through the CI and it passed the build-ancient job. > > Please check the full log. I have already added the warning to the > "allow-list" to move to 6.14 > > drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c: In > function 'extron_cec_adap_transmit': > drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:44: > warning: 'DCEC' directive output may be truncated writing 4 bytes into > a region of size between 1 and 53 [-Wformat-truncation=] > snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", > ^~~~ > drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:2: > note: 'snprintf' output between 13 and 65 bytes into a destination of > size 61 > snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > port->direction, port->port.port, > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > cec_msg_initiator(msg), cec_msg_destination(msg), buf); > > This does the trick > diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > index cfbfc4c1b2e6..c56db19eaf16 100644 > --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > @@ -1003,7 +1003,7 @@ static int extron_cec_adap_transmit(struct > cec_adapter *adap, u8 attempts, > { > struct extron_port *port = cec_get_drvdata(adap); > char buf[CEC_MAX_MSG_SIZE * 3 + 1]; > - char cmd[CEC_MAX_MSG_SIZE * 3 + 13]; > + char cmd[CEC_MAX_MSG_SIZE * 3 + 15]; Note that it is 15 and not 14 because I missed an * in my previous email. Regards! > unsigned int i; > > if (port->disconnected) > @@ -1012,7 +1012,7 @@ static int extron_cec_adap_transmit(struct > cec_adapter *adap, u8 attempts, > for (i = 0; i < msg->len - 1; i++) > sprintf(buf + i * 3, "%%%02X", msg->msg[i + 1]); > snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", > - port->direction, port->port.port, > + port->direction, port->port.port % 10, > cec_msg_initiator(msg), cec_msg_destination(msg), buf); > return extron_send_and_wait(port->extron, port, cmd, NULL); > } > > > > > > Regards, > > > > Hans > > > > > DCEC -> 4 bytes > > > \0 -> 1 byte > > > > > > That is a total of CEC_MAX_MSG_SIZE * 3 + 14. Don't we need to add an > > > extra byte to cmd? > > > > > > char cmd[CEC_MAX_MSG_SIZE * 3 + 14]; > > > > > > Regards! > > > > > > On Tue, 1 Apr 2025 at 10:18, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > >> > > >> Change the port type to u8 so gcc8 knows that the port fits in a single > > >> char. > > >> > > >> drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:44: warning: 'DCEC' directive output may be truncated writing 4 bytes into a region of size between 0 and 53 [-Wformat-truncation=] > > >> > > >> Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl> > > >> Reported-by: Ricardo Ribalda <ribalda@chromium.org> > > >> --- > > >> drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h | 2 +- > > >> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c | 2 +- > > >> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h | 6 +++--- > > >> 3 files changed, 5 insertions(+), 5 deletions(-) > > >> > > >> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h > > >> index 7422f7c5719e..fa2e02b26565 100644 > > >> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h > > >> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h > > >> @@ -14,7 +14,7 @@ struct cec_splitter; > > >> struct cec_splitter_port { > > >> struct cec_splitter *splitter; > > >> struct cec_adapter *adap; > > >> - unsigned int port; > > >> + u8 port; > > >> bool is_active_source; > > >> bool found_sink; > > >> ktime_t lost_sink_ts; > > >> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > > >> index cfbfc4c1b2e6..c4add8f3f8b4 100644 > > >> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > > >> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > > >> @@ -1692,7 +1692,7 @@ static int extron_setup_thread(void *_extron) > > >> log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_TV; > > >> } else { > > >> snprintf(log_addrs.osd_name, sizeof(log_addrs.osd_name), > > >> - "Splitter Out%u", port->port.port); > > >> + "Splitter Out%u", port->port.port % 10); > > >> log_addrs.log_addr_type[0] = CEC_LOG_ADDR_TYPE_PLAYBACK; > > >> log_addrs.primary_device_type[0] = CEC_OP_PRIM_DEVTYPE_PLAYBACK; > > >> log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_PLAYBACK; > > >> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h > > >> index b79f1253ab5d..cd07e90ea32a 100644 > > >> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h > > >> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h > > >> @@ -83,9 +83,9 @@ struct extron { > > >> struct serio *serio; > > >> /* locks access to serio */ > > >> struct mutex serio_lock; > > >> - unsigned int num_ports; > > >> - unsigned int num_in_ports; > > >> - unsigned int num_out_ports; > > >> + u8 num_ports; > > >> + u8 num_in_ports; > > >> + u8 num_out_ports; > > >> char unit_name[32]; > > >> char unit_type[64]; > > >> char unit_fw_version[32]; > > >> -- > > >> 2.47.2 > > >> > > > > > > > > > > > -- > Ricardo Ribalda
On 01/04/2025 11:11, Ricardo Ribalda wrote: > Hi Hans > > On Tue, 1 Apr 2025 at 10:43, Hans Verkuil <hverkuil@xs4all.nl> wrote: >> >> On 01/04/2025 10:30, Ricardo Ribalda wrote: >>> Hi Hans >>> >>> snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", >>> port->direction, port->port.port, >>> cec_msg_initiator(msg), cec_msg_destination(msg), buf); >>> >>> Will need: >>> W -> 1 byte >>> %c -> 1 byte >>> %u -> 1 byte port.port (we might have to add %10 here in case gcc8 is >>> not clever enough ) >>> * -> 1 byte >>> %u -> 2 bytes cec_msg_initiator(msg) (we might have to add %16 here >>> in case gcc8 is not clever enough ) >>> * -> 1 byte >>> %u -> 2 bytes cec_msg_destination(msg) (we might have to add %16 here >>> in case gcc8 is not clever enough ) >>> %s -> CEC_MAX_MSG_SIZE * 3 buf >> >> Actually, this is at most (CEC_MAX_MSG_SIZE - 1) * 3, since the first byte >> of the CEC message isn't included in the buffer. >> >> So this code is safe. >> >> I agree that it could be made a bit clearer, but something for another day. >> >> I ran this patch through the CI and it passed the build-ancient job. > > Please check the full log. I have already added the warning to the > "allow-list" to move to 6.14 I wasn't aware it was added to the allow-list, so I didn't check the actual build-ancient log, just the final job status. I'll prepare a v3. Regards, Hans > > drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c: In > function 'extron_cec_adap_transmit': > drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:44: > warning: 'DCEC' directive output may be truncated writing 4 bytes into > a region of size between 1 and 53 [-Wformat-truncation=] > snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", > ^~~~ > drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:2: > note: 'snprintf' output between 13 and 65 bytes into a destination of > size 61 > snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > port->direction, port->port.port, > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > cec_msg_initiator(msg), cec_msg_destination(msg), buf); > > This does the trick > diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > index cfbfc4c1b2e6..c56db19eaf16 100644 > --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > @@ -1003,7 +1003,7 @@ static int extron_cec_adap_transmit(struct > cec_adapter *adap, u8 attempts, > { > struct extron_port *port = cec_get_drvdata(adap); > char buf[CEC_MAX_MSG_SIZE * 3 + 1]; > - char cmd[CEC_MAX_MSG_SIZE * 3 + 13]; > + char cmd[CEC_MAX_MSG_SIZE * 3 + 15]; > unsigned int i; > > if (port->disconnected) > @@ -1012,7 +1012,7 @@ static int extron_cec_adap_transmit(struct > cec_adapter *adap, u8 attempts, > for (i = 0; i < msg->len - 1; i++) > sprintf(buf + i * 3, "%%%02X", msg->msg[i + 1]); > snprintf(cmd, sizeof(cmd), "W%c%u*%u*%u*%sDCEC", > - port->direction, port->port.port, > + port->direction, port->port.port % 10, > cec_msg_initiator(msg), cec_msg_destination(msg), buf); > return extron_send_and_wait(port->extron, port, cmd, NULL); > } > > >> >> Regards, >> >> Hans >> >>> DCEC -> 4 bytes >>> \0 -> 1 byte >>> >>> That is a total of CEC_MAX_MSG_SIZE * 3 + 14. Don't we need to add an >>> extra byte to cmd? >>> >>> char cmd[CEC_MAX_MSG_SIZE * 3 + 14]; >>> >>> Regards! >>> >>> On Tue, 1 Apr 2025 at 10:18, Hans Verkuil <hverkuil@xs4all.nl> wrote: >>>> >>>> Change the port type to u8 so gcc8 knows that the port fits in a single >>>> char. >>>> >>>> drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:44: warning: 'DCEC' directive output may be truncated writing 4 bytes into a region of size between 0 and 53 [-Wformat-truncation=] >>>> >>>> Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl> >>>> Reported-by: Ricardo Ribalda <ribalda@chromium.org> >>>> --- >>>> drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h | 2 +- >>>> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c | 2 +- >>>> .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h | 6 +++--- >>>> 3 files changed, 5 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h >>>> index 7422f7c5719e..fa2e02b26565 100644 >>>> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h >>>> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h >>>> @@ -14,7 +14,7 @@ struct cec_splitter; >>>> struct cec_splitter_port { >>>> struct cec_splitter *splitter; >>>> struct cec_adapter *adap; >>>> - unsigned int port; >>>> + u8 port; >>>> bool is_active_source; >>>> bool found_sink; >>>> ktime_t lost_sink_ts; >>>> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c >>>> index cfbfc4c1b2e6..c4add8f3f8b4 100644 >>>> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c >>>> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c >>>> @@ -1692,7 +1692,7 @@ static int extron_setup_thread(void *_extron) >>>> log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_TV; >>>> } else { >>>> snprintf(log_addrs.osd_name, sizeof(log_addrs.osd_name), >>>> - "Splitter Out%u", port->port.port); >>>> + "Splitter Out%u", port->port.port % 10); >>>> log_addrs.log_addr_type[0] = CEC_LOG_ADDR_TYPE_PLAYBACK; >>>> log_addrs.primary_device_type[0] = CEC_OP_PRIM_DEVTYPE_PLAYBACK; >>>> log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_PLAYBACK; >>>> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h >>>> index b79f1253ab5d..cd07e90ea32a 100644 >>>> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h >>>> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h >>>> @@ -83,9 +83,9 @@ struct extron { >>>> struct serio *serio; >>>> /* locks access to serio */ >>>> struct mutex serio_lock; >>>> - unsigned int num_ports; >>>> - unsigned int num_in_ports; >>>> - unsigned int num_out_ports; >>>> + u8 num_ports; >>>> + u8 num_in_ports; >>>> + u8 num_out_ports; >>>> char unit_name[32]; >>>> char unit_type[64]; >>>> char unit_fw_version[32]; >>>> -- >>>> 2.47.2 >>>> >>> >>> >> > >
diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h index 7422f7c5719e..fa2e02b26565 100644 --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h @@ -14,7 +14,7 @@ struct cec_splitter; struct cec_splitter_port { struct cec_splitter *splitter; struct cec_adapter *adap; - unsigned int port; + u8 port; bool is_active_source; bool found_sink; ktime_t lost_sink_ts; diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c index cfbfc4c1b2e6..c4add8f3f8b4 100644 --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c @@ -1692,7 +1692,7 @@ static int extron_setup_thread(void *_extron) log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_TV; } else { snprintf(log_addrs.osd_name, sizeof(log_addrs.osd_name), - "Splitter Out%u", port->port.port); + "Splitter Out%u", port->port.port % 10); log_addrs.log_addr_type[0] = CEC_LOG_ADDR_TYPE_PLAYBACK; log_addrs.primary_device_type[0] = CEC_OP_PRIM_DEVTYPE_PLAYBACK; log_addrs.all_device_types[0] = CEC_OP_ALL_DEVTYPE_PLAYBACK; diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h index b79f1253ab5d..cd07e90ea32a 100644 --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h @@ -83,9 +83,9 @@ struct extron { struct serio *serio; /* locks access to serio */ struct mutex serio_lock; - unsigned int num_ports; - unsigned int num_in_ports; - unsigned int num_out_ports; + u8 num_ports; + u8 num_in_ports; + u8 num_out_ports; char unit_name[32]; char unit_type[64]; char unit_fw_version[32];
Change the port type to u8 so gcc8 knows that the port fits in a single char. drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c:1014:44: warning: 'DCEC' directive output may be truncated writing 4 bytes into a region of size between 0 and 53 [-Wformat-truncation=] Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl> Reported-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/cec/usb/extron-da-hd-4k-plus/cec-splitter.h | 2 +- .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c | 2 +- .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.h | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-)