From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: dri-devel@lists.freedesktop.org, Helge Deller <deller@gmx.de>,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Hans Verkuil <hans.verkuil@cisco.com>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Bernard Zhao <bernard@vivo.com>
Subject: Re: [PATCH v1 RFC] video/hdmi: Fix HDMI_VENDOR_INFOFRAME_SIZE
Date: Mon, 6 Feb 2023 11:58:26 +0200 [thread overview]
Message-ID: <Y+DPQjukgC0BELkN@intel.com> (raw)
In-Reply-To: <20230109223110.1165433-1-martin.blumenstingl@googlemail.com>
On Mon, Jan 09, 2023 at 11:31:10PM +0100, Martin Blumenstingl wrote:
> When support for the HDMI vendor infoframe was introduced back with
> commit 7d27becb3532 ("video/hdmi: Introduce helpers for the HDMI vendor
> specific infoframe") it's payload size was either 5 or 6 bytes,
> depending on:
> if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
> When true the size was 6 bytes, otherwise 5 bytes.
>
> Drivers that are using hdmi_infoframe_pack() are reserving 10 bytes (4
> bytes for the header and up to 6 bytes for the infoframe payload data)
> or more (exynos_hdmi reserves 25 bytes).
>
> Over time the frame payload length was reduced to 4 bytes. This however
> does not match the code from hdmi_hdmi_infoframe_pack() where ptr[8] and
> ptr[9] are written, which means the infoframe has to allow up to 6 bytes
> of payload data (considering that the header takes 4 bytes).
>
> Change HDMI_VENDOR_INFOFRAME_SIZE to 6 bytes so
> hdmi_vendor_infoframe_pack_only() can properly check the passed buffer
> size and avoid an out of bounds write to ptr[8] or ptr[9].
The function should return -ENOSPC if the caller didn't
provide a big enough buffer. Are you saying there are
drivers that are passing a bogus size here?
>
> Fixes: c5e69ab35c0d ("video/hdmi: Constify infoframe passed to the pack functions")
> Fixes: d43be2554b58 ("drivers: video: hdmi: cleanup coding style in video a bit")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> I'm not an expert on this topic and I'm not sure if the size still
> depends on that if condition from long time ago. So please share your
> thoughts.
>
>
> include/linux/hdmi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 2f4dcc8d060e..026c5ef5a1a5 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -57,7 +57,7 @@ enum hdmi_infoframe_type {
> #define HDMI_SPD_INFOFRAME_SIZE 25
> #define HDMI_AUDIO_INFOFRAME_SIZE 10
> #define HDMI_DRM_INFOFRAME_SIZE 26
> -#define HDMI_VENDOR_INFOFRAME_SIZE 4
> +#define HDMI_VENDOR_INFOFRAME_SIZE 6
>
> #define HDMI_INFOFRAME_SIZE(type) \
> (HDMI_INFOFRAME_HEADER_SIZE + HDMI_ ## type ## _INFOFRAME_SIZE)
> --
> 2.39.0
--
Ville Syrjälä
Intel
WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: linux-fbdev@vger.kernel.org,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Bernard Zhao <bernard@vivo.com>, Helge Deller <deller@gmx.de>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
Hans Verkuil <hans.verkuil@cisco.com>
Subject: Re: [PATCH v1 RFC] video/hdmi: Fix HDMI_VENDOR_INFOFRAME_SIZE
Date: Mon, 6 Feb 2023 11:58:26 +0200 [thread overview]
Message-ID: <Y+DPQjukgC0BELkN@intel.com> (raw)
In-Reply-To: <20230109223110.1165433-1-martin.blumenstingl@googlemail.com>
On Mon, Jan 09, 2023 at 11:31:10PM +0100, Martin Blumenstingl wrote:
> When support for the HDMI vendor infoframe was introduced back with
> commit 7d27becb3532 ("video/hdmi: Introduce helpers for the HDMI vendor
> specific infoframe") it's payload size was either 5 or 6 bytes,
> depending on:
> if (frame->s3d_struct >= HDMI_3D_STRUCTURE_SIDE_BY_SIDE_HALF)
> When true the size was 6 bytes, otherwise 5 bytes.
>
> Drivers that are using hdmi_infoframe_pack() are reserving 10 bytes (4
> bytes for the header and up to 6 bytes for the infoframe payload data)
> or more (exynos_hdmi reserves 25 bytes).
>
> Over time the frame payload length was reduced to 4 bytes. This however
> does not match the code from hdmi_hdmi_infoframe_pack() where ptr[8] and
> ptr[9] are written, which means the infoframe has to allow up to 6 bytes
> of payload data (considering that the header takes 4 bytes).
>
> Change HDMI_VENDOR_INFOFRAME_SIZE to 6 bytes so
> hdmi_vendor_infoframe_pack_only() can properly check the passed buffer
> size and avoid an out of bounds write to ptr[8] or ptr[9].
The function should return -ENOSPC if the caller didn't
provide a big enough buffer. Are you saying there are
drivers that are passing a bogus size here?
>
> Fixes: c5e69ab35c0d ("video/hdmi: Constify infoframe passed to the pack functions")
> Fixes: d43be2554b58 ("drivers: video: hdmi: cleanup coding style in video a bit")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> I'm not an expert on this topic and I'm not sure if the size still
> depends on that if condition from long time ago. So please share your
> thoughts.
>
>
> include/linux/hdmi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 2f4dcc8d060e..026c5ef5a1a5 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -57,7 +57,7 @@ enum hdmi_infoframe_type {
> #define HDMI_SPD_INFOFRAME_SIZE 25
> #define HDMI_AUDIO_INFOFRAME_SIZE 10
> #define HDMI_DRM_INFOFRAME_SIZE 26
> -#define HDMI_VENDOR_INFOFRAME_SIZE 4
> +#define HDMI_VENDOR_INFOFRAME_SIZE 6
>
> #define HDMI_INFOFRAME_SIZE(type) \
> (HDMI_INFOFRAME_HEADER_SIZE + HDMI_ ## type ## _INFOFRAME_SIZE)
> --
> 2.39.0
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2023-02-06 9:58 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-09 22:31 [PATCH v1 RFC] video/hdmi: Fix HDMI_VENDOR_INFOFRAME_SIZE Martin Blumenstingl
2023-01-09 22:31 ` Martin Blumenstingl
2023-01-10 18:20 ` Jani Nikula
2023-01-10 18:20 ` Jani Nikula
2023-02-05 20:08 ` Martin Blumenstingl
2023-02-05 20:08 ` Martin Blumenstingl
2023-02-06 9:58 ` Ville Syrjälä [this message]
2023-02-06 9:58 ` Ville Syrjälä
2023-02-11 20:43 ` Martin Blumenstingl
2023-02-11 20:43 ` Martin Blumenstingl
2023-02-13 11:11 ` Ville Syrjälä
2023-02-13 11:11 ` Ville Syrjälä
2023-02-14 21:26 ` Martin Blumenstingl
2023-02-14 21:26 ` Martin Blumenstingl
2023-02-14 21:35 ` Ville Syrjälä
2023-02-14 21:35 ` Ville Syrjälä
2023-02-18 15:33 ` Martin Blumenstingl
2023-02-18 15:33 ` Martin Blumenstingl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y+DPQjukgC0BELkN@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=b.zolnierkie@samsung.com \
--cc=bernard@vivo.com \
--cc=deller@gmx.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=hans.verkuil@cisco.com \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.blumenstingl@googlemail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.