| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <02ac025c0ecf17354377c7f2c2fc83b40a3a41e1.camel@ndufresne.ca>
Date: Wed, 24 Dec 2025 10:39:33 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Sven Püschel <s.pueschel@...gutronix.de>, Jacob Chen
<jacob-chen@...wrt.com>, Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
Mauro Carvalho Chehab
<mchehab@...nel.org>, Heiko Stuebner <heiko@...ech.de>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>
Cc: linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH v2 11/22] media: rockchip: rga: check scaling factor
Le mercredi 03 décembre 2025 à 16:52 +0100, Sven Püschel a écrit :
> Check the scaling factor to avoid potential problems. This is relevant
> for the upcoming RGA3 support, as it can hang when the scaling factor
> is exceeded.
>
> There are two relevant scenarios that have to be considered to protect
> against invalid scaling values:
>
> When the output or capture is already streaming, setting the format on
> the other side should consider the max scaling factor and clamp it
> accordingly. This is only done in the streaming case, as it otherwise
> may unintentionally clamp the value when the application sets the first
> format (due to a default format on the other side).
>
> When the format is set on both sides first, then the format won't be
> corrected by above means. Therefore the second streamon call has to
> check the scaling factor and fail otherwise.
In codec specifications, we resolve this issue by resetting the capture format
every-time the output format is set. But without specification for color
transforms, its impossible to say if this is right or wrong, and I don't expect
perfect interroperability between drivers until someone make the effort to
specify this type of hardware.
What you describe is fine of course, but its a bit off nature of the way format
is normally being fixed by the driver to stay valid.
>
> Signed-off-by: Sven Püschel <s.pueschel@...gutronix.de>
> ---
> drivers/media/platform/rockchip/rga/rga-hw.c | 1 +
> drivers/media/platform/rockchip/rga/rga-hw.h | 1 +
> drivers/media/platform/rockchip/rga/rga.c | 60 +++++++++++++++++++++++++---
> drivers/media/platform/rockchip/rga/rga.h | 1 +
> 4 files changed, 58 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
> index 8cdfe089fd636..2ed4f22a999d5 100644
> --- a/drivers/media/platform/rockchip/rga/rga-hw.c
> +++ b/drivers/media/platform/rockchip/rga/rga-hw.c
> @@ -624,6 +624,7 @@ const struct rga_hw rga2_hw = {
> .max_width = MAX_WIDTH,
> .min_height = MIN_HEIGHT,
> .max_height = MAX_HEIGHT,
> + .max_scaling_factor = MAX_SCALING_FACTOR,
> .stride_alignment = 4,
>
> .setup_cmdbuf = rga_hw_setup_cmdbuf,
> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.h b/drivers/media/platform/rockchip/rga/rga-hw.h
> index f4752aa823051..fffcab0131225 100644
> --- a/drivers/media/platform/rockchip/rga/rga-hw.h
> +++ b/drivers/media/platform/rockchip/rga/rga-hw.h
> @@ -14,6 +14,7 @@
>
> #define MIN_WIDTH 34
> #define MIN_HEIGHT 34
> +#define MAX_SCALING_FACTOR 16
>
> #define RGA_TIMEOUT 500
>
> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index f02ae02de26ca..46dc94db6f85e 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -346,18 +346,47 @@ static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
> static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
> {
> struct v4l2_pix_format_mplane *pix_fmt = &f->fmt.pix_mp;
> - struct rockchip_rga *rga = video_drvdata(file);
> + struct rga_ctx *ctx = file_to_rga_ctx(file);
> + struct rockchip_rga *rga = ctx->rga;
> const struct rga_hw *hw = rga->hw;
> struct rga_fmt *fmt;
> + u32 min_width = hw->min_width;
> + u32 max_width = hw->max_width;
> + u32 min_height = hw->min_height;
> + u32 max_height = hw->max_height;
>
> fmt = rga_fmt_find(rga, pix_fmt->pixelformat);
> if (!fmt)
> fmt = &hw->formats[0];
>
> - pix_fmt->width = clamp(pix_fmt->width,
> - hw->min_width, hw->max_width);
> - pix_fmt->height = clamp(pix_fmt->height,
> - hw->min_height, hw->max_height);
> + if (V4L2_TYPE_IS_OUTPUT(f->type) &&
> + v4l2_m2m_get_dst_vq(ctx->fh.m2m_ctx)->streaming) {
What if userspace wanted to get the buffer size computed, so it can allocate
externally before it calls streamoff ? Hans did mention recently that try
function should only be state aware if specified.
I'd like other reviewers feedback on what should be done here, of course writing
a spec would be ideal.
Nicolas
> + min_width =
> + MAX(min_width, DIV_ROUND_UP(ctx->out.pix.width,
> + hw->max_scaling_factor));
> + max_width = MIN(max_width,
> + ctx->out.pix.width * hw->max_scaling_factor);
> + min_height =
> + MAX(min_height, DIV_ROUND_UP(ctx->out.pix.height,
> + hw->max_scaling_factor));
> + max_height = MIN(max_height,
> + ctx->out.pix.height * hw->max_scaling_factor);
> + } else if (V4L2_TYPE_IS_CAPTURE(f->type) &&
> + v4l2_m2m_get_src_vq(ctx->fh.m2m_ctx)->streaming) {
> + min_width =
> + MAX(min_width, DIV_ROUND_UP(ctx->in.pix.width,
> + hw->max_scaling_factor));
> + max_width = MIN(max_width,
> + ctx->in.pix.width * hw->max_scaling_factor);
> + min_height =
> + MAX(min_height, DIV_ROUND_UP(ctx->in.pix.height,
> + hw->max_scaling_factor));
> + max_height = MIN(max_height,
> + ctx->in.pix.height * hw->max_scaling_factor);
> + }
> +
> + pix_fmt->width = clamp(pix_fmt->width, min_width, max_width);
> + pix_fmt->height = clamp(pix_fmt->height, min_height, max_height);
>
> v4l2_fill_pixfmt_mp_aligned(pix_fmt, pix_fmt->pixelformat,
> pix_fmt->width, pix_fmt->height, hw->stride_alignment);
> @@ -523,12 +552,33 @@ static int vidioc_s_selection(struct file *file, void *priv,
> return ret;
> }
>
> +static bool check_scaling(const struct rga_hw *hw, u32 src_size, u32 dst_size)
> +{
> + if (src_size < dst_size)
> + return src_size * hw->max_scaling_factor >= dst_size;
> + else
> + return dst_size * hw->max_scaling_factor >= src_size;
> +}
> +
> static int vidioc_streamon(struct file *file, void *priv,
> enum v4l2_buf_type type)
> {
> struct rga_ctx *ctx = file_to_rga_ctx(file);
> const struct rga_hw *hw = ctx->rga->hw;
>
> + if ((V4L2_TYPE_IS_OUTPUT(type) &&
> + v4l2_m2m_get_dst_vq(ctx->fh.m2m_ctx)->streaming) ||
> + (V4L2_TYPE_IS_CAPTURE(type) &&
> + v4l2_m2m_get_src_vq(ctx->fh.m2m_ctx)->streaming)) {
> + /*
> + * As the other side is already streaming,
> + * check that the max scaling factor isn't exceeded.
> + */
> + if (!check_scaling(hw, ctx->in.pix.width, ctx->out.pix.width) ||
> + !check_scaling(hw, ctx->in.pix.height, ctx->out.pix.height))
> + return -EINVAL;
> + }
> +
> hw->setup_cmdbuf(ctx);
>
> return v4l2_m2m_streamon(file, ctx->fh.m2m_ctx, type);
> diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
> index 93162b118d069..d02d5730b4e3b 100644
> --- a/drivers/media/platform/rockchip/rga/rga.h
> +++ b/drivers/media/platform/rockchip/rga/rga.h
> @@ -152,6 +152,7 @@ struct rga_hw {
> size_t cmdbuf_size;
> u32 min_width, min_height;
> u32 max_width, max_height;
> + u8 max_scaling_factor;
> u8 stride_alignment;
>
> void (*setup_cmdbuf)(struct rga_ctx *ctx);
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists