Re: [PATCH] drm/i915/display: Fix lfp_vbt_mode memory leak

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 31 Jul 2024, José Roberto de Souza <jose.souza@intel.com> wrote:
> In case anything in intel_edp_init_connector() after
> intel_bios_init_panel_early() causes connector initialization to fail,
> it leaks lfp_vbt_mode allocated in:
> intel_bios_init_panel_early()
> 	intel_bios_init_panel()
> 		parse_lfp_data()
> 			parse_lfp_panel_dtd()
> 				panel_fixed_mode = kzalloc();
> 				panel->vbt.lfp_vbt_mode = panel_fixed_mode;
>
> So here adding intel_bios_fini_panel() to the error path to free
> this memory.

It's kind of correct, but also kind of wrong...

The error path also leaks connector->panel.fixed_modes. Should maybe
call intel_panel_fini(). But it's really awkward how parts of the panel
structure are initialized here and there, and not a whole lot happens at
intel_panel_init().

BR,
Jani.

>
> root@DUT147LNL:/home/gta# more /sys/kernel/debug/kmemleak
> unreferenced object 0xffff88811832da00 (size 128):
>   comm "modprobe", pid 1134, jiffies 4294792711
>   hex dump (first 32 bytes):
>     e8 fd 00 00 00 04 18 04 a0 04 40 05 00 00 00 03  ..........@.....
>     03 03 09 03 26 03 00 00 0a 00 00 00 00 00 00 00  ....&...........
>   backtrace (crc 7448f6b4):
>     [<ffffffff812d9db0>] __kmalloc_cache_noprof+0x270/0x330
>     [<ffffffffa038e56b>] intel_bios_init_panel+0x188b/0x1c70 [xe]
>     [<ffffffffa03e1594>] intel_dp_init_connector+0x544/0xf00 [xe]
>     [<ffffffffa03b1afb>] intel_ddi_init+0xb5b/0xf10 [xe]
>     [<ffffffffa039086c>] intel_bios_for_each_encoder+0x2c/0x40 [xe]
>     [<ffffffffa03c25cf>] intel_setup_outputs+0x1ff/0x470 [xe]
>     [<ffffffffa03c4b63>] intel_display_driver_probe_nogem+0x133/0x1d0 [xe]
>     [<ffffffffa037b659>] xe_display_init_noaccel+0x19/0x60 [xe]
>     [<ffffffffa02e19f8>] xe_device_probe+0x5c8/0x720 [xe]
>     [<ffffffffa032a627>] xe_pci_probe+0x7b7/0xd30 [xe]
>     [<ffffffff8172d903>] pci_device_probe+0xa3/0x170
>     [<ffffffff818868a0>] really_probe+0xd0/0x320
>     [<ffffffff81886b63>] __driver_probe_device+0x73/0x110
>     [<ffffffff81886c1a>] driver_probe_device+0x1a/0x90
>     [<ffffffff81886e2a>] __driver_attach+0xaa/0x1b0
>     [<ffffffff81884685>] bus_for_each_dev+0x75/0xc0
>
> (gdb) list *(intel_bios_init_panel+0x188b)
> 0xb75ab is in intel_bios_init_panel (drivers/gpu/drm/i915/display/intel_bios.c:849).
> 844	        panel_dvo_timing = get_lfp_dvo_timing(lfp_data,
> 845			                                              lfp_data_ptrs,
> 846			                                              panel_type);
> 847
> 848	        panel_fixed_mode = kzalloc(sizeof(*panel_fixed_mode), GFP_KERNEL);
> 849	        if (!panel_fixed_mode)
> 850		                return;
> 851
> 852	        fill_detail_timing_data(i915, panel_fixed_mode, panel_dvo_timing);
> 853
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 09789d62001f7..bd644cc0336bc 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6764,6 +6764,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	return true;
>  
>  out_vdd_off:
> +	intel_bios_fini_panel(&intel_connector->panel);
>  	intel_pps_vdd_off_sync(intel_dp);
>  
>  	return false;

-- 
Jani Nikula, Intel