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.

>> 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].


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

Reply via email to