Hi Kees, On 29-Jul-25 2:46 AM, Kees Cook wrote: > On Sat, Jul 26, 2025 at 02:24:51PM +0200, Hans de Goede wrote: >> Hi Kees, >> >> On 24-Jul-25 10:08 AM, Kees Cook wrote: >>> When gmin_get_config_var() calls efi.get_variable() and the EFI variable >>> is larger than the expected buffer size, two behaviors combine to create >>> a stack buffer overflow: >>> >>> 1. gmin_get_config_var() does not return the proper error code when >>> efi.get_variable() fails. It returns the stale 'ret' value from >>> earlier operations instead of indicating the EFI failure. >>> >>> 2. When efi.get_variable() returns EFI_BUFFER_TOO_SMALL, it updates >>> *out_len to the required buffer size but writes no data to the output >>> buffer. However, due to bug #1, gmin_get_var_int() believes the call >>> succeeded. >>> >>> The caller gmin_get_var_int() then performs: >>> - Allocates val[CFG_VAR_NAME_MAX + 1] (65 bytes) on stack >>> - Calls gmin_get_config_var(dev, is_gmin, var, val, &len) with len=64 >>> - If EFI variable is >64 bytes, efi.get_variable() sets len=required_size >>> - Due to bug #1, thinks call succeeded with len=required_size >>> - Executes val[len] = 0, writing past end of 65-byte stack buffer >>> >>> This creates a stack buffer overflow when EFI variables are larger than >>> 64 bytes. Since EFI variables can be controlled by firmware or system >>> configuration, this could potentially be exploited for code execution. >>> >>> Fix the bug by returning proper error codes from gmin_get_config_var() >>> based on EFI status instead of stale 'ret' value. >>> >>> The gmin_get_var_int() function is called during device initialization >>> for camera sensor configuration on Intel Bay Trail and Cherry Trail >>> platforms using the atomisp camera stack. >>> >>> Reported-by: zepta <z3p...@gmail.com> >>> Closes: >>> https://lore.kernel.org/all/capbs6koqym7fmdpwouxtexsoe44x4h3f8fw+y_qwq6e+odm...@mail.gmail.com >>> Fixes: 38d4f74bc148 ("media: atomisp_gmin_platform: stop abusing efivar >>> API") >>> Signed-off-by: Kees Cook <k...@kernel.org> >> >> Thanks, patch looks good to me: >> >> Reviewed-by: Hans de Goede <ha...@kernel.org> >> >> I've already send an atomisp pull-request for 6.17 out >> and this is already in media-committers/next now and >> the media subsystem is typically not good in merging >> fixes just before the merge window. >> >> Kees, the file touched here is unchanged in >> media-committers/next vs Linus' latest master, can you >> send this fix to Linus yourself ? > > I apologize; this slipped through the cracks. Shall I take it for -rc2, > or do you want to snag it?
I'm just back from vacation and I see you've send this to Linus already: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/staging/media/atomisp?id=ee4cf798202d285dcbe85e4467a094c44f5ed8e6 Which is great, thank you. Regards, Hans
