From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) by mx.groups.io with SMTP id smtpd.web08.1392.1617734716074801805 for ; Tue, 06 Apr 2021 11:45:16 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=Q0mFlMHD; spf=pass (domain: gmail.com, ip: 209.85.210.172, mailfrom: kuqin12@gmail.com) Received: by mail-pf1-f172.google.com with SMTP id v10so11064284pfn.5 for ; Tue, 06 Apr 2021 11:45:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=l0Lp3SPXAwyJ705kLH26vOwprHK9hwlocFCYNuy6kvc=; b=Q0mFlMHD+0tPFUCsa6WB6lOiGpPVTqODn71z9LL2c37J9vCT0DlRrPU1DFCauogn9J MoHLmIaxXGRk9a2o7EF7b4HR8ZVIWfMAkEseQhp2Ttkbm0fzCDJLryeCnekCKEnw95mt D2lnwy4OlOGfw97vJaT/HQvKjudUKPveebt5JpPATTeKif8O+FHiedv8t0XjNm24H9PC 1cjn/oU5y9Flx3hAURQaiATOh/4G8jq0QYp78NXyshnIk5mTmAcU+T3M/SVPjAUhcHtj rfKK+uHCoOepYUzdL40CX5cPZxlI8UwQtFgCylKgnRpFWUJVgOq52G+HNKnzBq5QiiB/ HyuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=l0Lp3SPXAwyJ705kLH26vOwprHK9hwlocFCYNuy6kvc=; b=hYOFXRoj+vZ2Xb/WiMQn6AQqdKoqZuo8y4OOaYTycXfYcdqA+GE0JFdIw+0kJlfqhl rb9OEPKyOJ9wvXPsjPZ90aPTe7V8P2Wf7ewhVXps7SAY2aJJyL7/B5d6UzgHCEC8ZzNf EIgNggd1gu2Z8ePkBIrj4wbBD4ept8BnGWyV4nWaKO1eWiPYlz+Oq6ZCZAJLKjeZTcQq WhjNWC0DONuLoWjuo55Jg6TcGl3fE4q73J1sPb6jG3mfmmYzNT6be+leZHoZNMHA+0Ko +I9W7jB0L2o9F71ZnFuVdGPqu5l8+FdQEbHt3TRizSdbw+ygI+EK2zQSgZcD/LE+eeJM ZcRA== X-Gm-Message-State: AOAM531wAVHWnb+tLTMQgLKxLyUl24MDsP/tQpcFmeTwfvyl6WGDlQJK o1WsTgW4NqSPWfj9tqvMMYE= X-Google-Smtp-Source: ABdhPJwmNmVx+WCaeOTBQdoRPvbPPJyjNry5DF3pv7nNDdsMzP4Zc6zT9zP5fh2hF1zg/wfyfqdKLg== X-Received: by 2002:a65:63ce:: with SMTP id n14mr28403064pgv.279.1617734715604; Tue, 06 Apr 2021 11:45:15 -0700 (PDT) Return-Path: Received: from [192.168.50.18] ([50.35.88.161]) by smtp.gmail.com with ESMTPSA id u20sm8833813pgl.27.2021.04.06.11.45.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 06 Apr 2021 11:45:15 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: PiSmmCpuDxeSmm: Check buffer size before accessing To: Laszlo Ersek , devel@edk2.groups.io Cc: Eric Dong , Ray Ni , Rahul Kumar References: <20210326234142.1973-1-kuqin12@gmail.com> <20210326234142.1973-2-kuqin12@gmail.com> <1211a5f3-f2b5-60e5-326e-ecef11e0ccac@redhat.com> From: "Kun Qin" Message-ID: Date: Tue, 6 Apr 2021 11:45:15 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <1211a5f3-f2b5-60e5-326e-ecef11e0ccac@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Laszlo, Thanks for the feedback. I will update the description in v2. Regards, Kun On 04/06/2021 05:09, Laszlo Ersek wrote: > On 03/27/21 00:41, Kun Qin wrote: >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3283 >> >> Current SMM Save State routine does not check the number of bytes to be >> read, when it comse to read IO_INFO, before casting the incoming buffer >> to EFI_SMM_SAVE_STATE_IO_INFO. This could potentially cause memory >> corruption due to extra bytes are written out of buffer boundary. >> >> This change adds a width check before copying IoInfo into output buffer. >> >> Cc: Eric Dong >> Cc: Ray Ni >> Cc: Laszlo Ersek >> Cc: Rahul Kumar >> >> Signed-off-by: Kun Qin >> --- >> UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c >> index 661cc51f361a..ec760e4c37ca 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c >> @@ -418,6 +418,13 @@ ReadSaveStateRegister ( >> return EFI_NOT_FOUND; >> } >> >> + // >> + // Make sure the incoming buffer is large enough to hold IoInfo before accessing >> + // >> + if (Width < sizeof (EFI_SMM_SAVE_STATE_IO_INFO)) { >> + return EFI_INVALID_PARAMETER; >> + } >> + >> // >> // Zero the IoInfo structure that will be returned in Buffer >> // >> > > Please update the description of the EFI_INVALID_PARAMETER return code > in the function's leading comment as well. > > With that: > > Reviewed-by: Laszlo Ersek > > Thanks > Laszlo >