On Mon, Jun 15, 2026 at 1:53 AM Jani Nikula <[email protected]> wrote:
>
> On Sat, 13 Jun 2026, Xiang Mei <[email protected]> wrote:
> > On Wed, Jun 10, 2026 at 5:49 AM Jani Nikula <[email protected]>
> > wrote:
> >>
> >> On Mon, 08 Jun 2026, Xiang Mei <[email protected]> wrote:
> >> > drm_parse_tiled_block() casts the DisplayID block to a
> >> > struct displayid_tiled_block and reads the full fixed layout up to
> >> > tile->topology_id[7] without checking block->num_bytes. The DisplayID
> >> > iterator only validates the declared payload length, so a crafted EDID
> >> > can advertise a tiled-display block (tag DATA_BLOCK_TILED_DISPLAY, or
> >> > DATA_BLOCK_2_TILED_DISPLAY_TOPOLOGY for v2.0) with a small num_bytes at
> >> > the end of a DisplayID extension. The read then runs past the end of the
> >> > exact-sized kmemdup()'d EDID allocation, a heap out-of-bounds read.
> >> >
> >> > Add a minimum-length check, as drm_parse_vesa_mso_data() already does.
> >> >
> >> > BUG: KASAN: slab-out-of-bounds in drm_edid_connector_update
> >> > Read of size 2 at addr ffff888010077700 by task exploit/147
> >> > dump_stack_lvl (lib/dump_stack.c:94 ...)
> >> > print_report (mm/kasan/report.c:378 ...)
> >> > kasan_report (mm/kasan/report.c:595)
> >> > drm_edid_connector_update (drivers/gpu/drm/drm_edid.c:7581)
> >> > bochs_connector_helper_get_modes (drivers/gpu/drm/tiny/bochs.c:574)
> >> > drm_helper_probe_single_connector_modes
> >> > (drivers/gpu/drm/drm_probe_helper.c:426)
> >> > status_store (drivers/gpu/drm/drm_sysfs.c:219)
> >> > ...
> >> > vfs_write (fs/read_write.c:595 fs/read_write.c:688)
> >> > ksys_write (fs/read_write.c:740)
> >> >
> >> > Fixes: 40d9b043a89e ("drm/connector: store tile information from
> >> > displayid (v3)")
> >> > Reported-by: Weiming Shi <[email protected]>
> >> > Assisted-by: Claude:claude-opus-4-8
> >> > Signed-off-by: Xiang Mei <[email protected]>
> >> > ---
> >> > drivers/gpu/drm/drm_edid.c | 7 +++++++
> >> > 1 file changed, 7 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> > index 404208bf23a6..92054ac82fe2 100644
> >> > --- a/drivers/gpu/drm/drm_edid.c
> >> > +++ b/drivers/gpu/drm/drm_edid.c
> >> > @@ -7575,6 +7575,13 @@ static void drm_parse_tiled_block(struct
> >> > drm_connector *connector,
> >> > u8 num_v_tile, num_h_tile;
> >> > struct drm_tile_group *tg;
> >> >
> >> > + if (block->num_bytes < sizeof(*tile) - sizeof(*block)) {
> >>
> >> I'd prefer using fixed values here. If the tiled display topology data
> >> block is ever expanded, we'll need to have them anyway.
> >>
> >> This should be if (block->num_bytes < 22).
> >>
> >
> > I've been advised on other patches that it's better to avoid magic
> > numbers, and I think sizeof(*tile) - sizeof(*block) is preferable here
> > for two reasons:
> >
> > 1. It's easier to understand. A bare 22 forces the reader to know what
> > it stands for, while sizeof(*tile) - sizeof(*block) reads as exactly
> > what it is: the number of payload bytes the code dereferences.
> >
> > 2. It's more maintainable for the expansion case you mention. It stays
> > in lockstep with the struct, so once topology_id is corrected to 9
> > bytes (below) it becomes 22 automatically and the patch still works
> > with no edit. A hardcoded 21 would silently re-open the OOB when the
> > struct grows, and a hardcoded 22 doesn't match what the current code
> > reads today.
>
> I'm not referring to struct changes, I'm referring to spec changes.
>
> If the spec gets changed, the struct needs to reflect the newest/biggest
> size, and access to the end of the struct needs to be gated on the size
> available. If you add that sizeof thing now, it'll need to be changed if
> the spec gets exended.
>
> I'll argue that adding the sizeof or < 21 check now is *wrong* because
> it does not match the *spec*. It's not about the struct, it's about the
> spec. The check should be < 22.
Thanks for the explanation. v2 uses 22 instead of sizeof.
>
> >> And reviewing this, I realize struct displayid_tiled_block definition is
> >> actually only 21 bytes. The topology_id member is 8 bytes, while it
> >> consists of
> >>
> >> - Tiled Display Manufacturer/Vendor ID Field (3 bytes)
> >> - Tiled Display Product ID Code Field (2 bytes)
> >> - Tiled Display Serial Number Field (4 bytes)
> >>
> >> i.e. total of 9 bytes.
> >>
> > Good catch. This looks like a separate, pre-existing issue from the
> > OOB read fixed here, and it also touches group_data[8] and the
> > memcpy/memcmp sites in drm_mode_{get,create}_tile_group(). We don't
> > have much experience with this part of the code, so if it's okay we'd
> > rather leave it for you to assess the impact and the right fix, and
> > keep this patch as the minimal OOB fix.
>
> The fix is at [1].
>
Thanks for the patch!
Xiang
>
> BR,
> Jani.
>
>
> [1] https://lore.kernel.org/r/[email protected]
>
> >
> >
> > Xiang
> >
> >> BR,
> >> Jani.
> >>
> >>
> >> > + drm_dbg_kms(connector->dev,
> >> > + "[CONNECTOR:%d:%s] Unexpected tiled block size
> >> > %u\n",
> >> > + connector->base.id, connector->name,
> >> > block->num_bytes);
> >> > + return;
> >> > + }
> >> > +
> >> > w = tile->tile_size[0] | tile->tile_size[1] << 8;
> >> > h = tile->tile_size[2] | tile->tile_size[3] << 8;
> >>
> >> --
> >> Jani Nikula, Intel
>
> --
> Jani Nikula, Intel
