[PATCH] drm/bridge: tc358767: clamp the reported AUX read size to the request

[PATCH] drm/bridge: tc358767: clamp the reported AUX read size to the request

[Maoyi Xie]

tc_aux_transfer() clamps an AUX read to the payload limit:

	size_t size = min_t(size_t, DP_AUX_MAX_PAYLOAD_BYTES - 1, msg->size);

After the transfer it replaces size with the byte count the controller
reports in AUX_BYTES:

	if (size)
		size = FIELD_GET(AUX_BYTES, auxstatus);

AUX_BYTES is GENMASK(15, 8), so it can be up to 255. Nothing clamps it
back to the request. tc_aux_read_data() reads that many bytes into the
16-byte auxrdata stack buffer, then copies them into the caller buffer. A
reported count of 255 makes the read run to 256 bytes and overruns both.

The controller should never report more than it was asked to transfer, so
this is defense in depth rather than a live hole. The reported count is
only lightly trusted, and the check is cheap. Clamp it back to the request,
the same way ti-sn65dsi86 does in commit aca58eac52b8 ("drm/bridge:
ti-sn65dsi86: Never store more than msg->size bytes in AUX xfer").

Fixes: 12dfe7c4d9c5 ("drm/bridge: tc358767: Use reported AUX transfer size")
Co-developed-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
Signed-off-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
 drivers/gpu/drm/bridge/tc358767.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 4097fef4b8..26ba5a6c1b 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -527,7 +527,7 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
 	 * address-only transfer
 	 */
 	if (size)
-		size = FIELD_GET(AUX_BYTES, auxstatus);
+		size = min_t(size_t, size, FIELD_GET(AUX_BYTES, auxstatus));
 	msg->reply = FIELD_GET(AUX_STATUS, auxstatus);
 
 	switch (request) {
-- 
2.34.1

Re: [PATCH] drm/bridge: tc358767: clamp the reported AUX read size to the request

[Doug Anderson]

Hi,

On Tue, Jun 30, 2026 at 11:44 PM Maoyi Xie <maoyixie.tju@gmail.com> wrote:
>
> tc_aux_transfer() clamps an AUX read to the payload limit:
>
>         size_t size = min_t(size_t, DP_AUX_MAX_PAYLOAD_BYTES - 1, msg->size);
>
> After the transfer it replaces size with the byte count the controller
> reports in AUX_BYTES:
>
>         if (size)
>                 size = FIELD_GET(AUX_BYTES, auxstatus);
>
> AUX_BYTES is GENMASK(15, 8), so it can be up to 255. Nothing clamps it
> back to the request. tc_aux_read_data() reads that many bytes into the
> 16-byte auxrdata stack buffer, then copies them into the caller buffer. A
> reported count of 255 makes the read run to 256 bytes and overruns both.
>
> The controller should never report more than it was asked to transfer, so
> this is defense in depth rather than a live hole. The reported count is
> only lightly trusted, and the check is cheap. Clamp it back to the request,
> the same way ti-sn65dsi86 does in commit aca58eac52b8 ("drm/bridge:
> ti-sn65dsi86: Never store more than msg->size bytes in AUX xfer").
>
> Fixes: 12dfe7c4d9c5 ("drm/bridge: tc358767: Use reported AUX transfer size")
> Co-developed-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
> Signed-off-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 4097fef4b8..26ba5a6c1b 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -527,7 +527,7 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
>          * address-only transfer
>          */
>         if (size)
> -               size = FIELD_GET(AUX_BYTES, auxstatus);
> +               size = min_t(size_t, size, FIELD_GET(AUX_BYTES, auxstatus));

Being super nitty: we don't technically need the "if (size)" above
anymore since it will be handled by the "min". Then again, if we
remove it then we also ahve to rework the comment about the "if"
test... :-P

...so I'm going to say this is fine the way it is.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

I'll try to land next week unless someone else objects. Another
committer is free to land before that if they feel like it.

-Doug

Re: [PATCH] drm/bridge: tc358767: clamp the reported AUX read size to the request

[Doug Anderson]

Hi,

On Thu, Jul 9, 2026 at 3:42 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Jun 30, 2026 at 11:44 PM Maoyi Xie <maoyixie.tju@gmail.com> wrote:
> >
> > tc_aux_transfer() clamps an AUX read to the payload limit:
> >
> >         size_t size = min_t(size_t, DP_AUX_MAX_PAYLOAD_BYTES - 1, msg->size);
> >
> > After the transfer it replaces size with the byte count the controller
> > reports in AUX_BYTES:
> >
> >         if (size)
> >                 size = FIELD_GET(AUX_BYTES, auxstatus);
> >
> > AUX_BYTES is GENMASK(15, 8), so it can be up to 255. Nothing clamps it
> > back to the request. tc_aux_read_data() reads that many bytes into the
> > 16-byte auxrdata stack buffer, then copies them into the caller buffer. A
> > reported count of 255 makes the read run to 256 bytes and overruns both.
> >
> > The controller should never report more than it was asked to transfer, so
> > this is defense in depth rather than a live hole. The reported count is
> > only lightly trusted, and the check is cheap. Clamp it back to the request,
> > the same way ti-sn65dsi86 does in commit aca58eac52b8 ("drm/bridge:
> > ti-sn65dsi86: Never store more than msg->size bytes in AUX xfer").
> >
> > Fixes: 12dfe7c4d9c5 ("drm/bridge: tc358767: Use reported AUX transfer size")
> > Co-developed-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
> > Signed-off-by: Kaixuan Li <kaixuan.li@ntu.edu.sg>
> > Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
> > ---
> >  drivers/gpu/drm/bridge/tc358767.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> > index 4097fef4b8..26ba5a6c1b 100644
> > --- a/drivers/gpu/drm/bridge/tc358767.c
> > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > @@ -527,7 +527,7 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
> >          * address-only transfer
> >          */
> >         if (size)
> > -               size = FIELD_GET(AUX_BYTES, auxstatus);
> > +               size = min_t(size_t, size, FIELD_GET(AUX_BYTES, auxstatus));
>
> Being super nitty: we don't technically need the "if (size)" above
> anymore since it will be handled by the "min". Then again, if we
> remove it then we also ahve to rework the comment about the "if"
> test... :-P
>
> ...so I'm going to say this is fine the way it is.
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> I'll try to land next week unless someone else objects. Another
> committer is free to land before that if they feel like it.

Even though this is a "Fix", it's a defense-in-depth fix and not
urgent IMO. I'm going to land it in -next instead of -fixes since bake
time is more important than urgency.

Pushed to drm-misc-next:

[1/1] drm/bridge: tc358767: clamp the reported AUX read size to the request
      commit: ec6444a00c49e6c2b5e9a507272a28126677f9ee