From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f174.google.com (mail-pg1-f174.google.com [209.85.215.174]) by mx.groups.io with SMTP id smtpd.web10.103011.1674829340185528303 for ; Fri, 27 Jan 2023 06:22:20 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=jCz2+s4T; spf=pass (domain: gmail.com, ip: 209.85.215.174, mailfrom: pedro.falcato@gmail.com) Received: by mail-pg1-f174.google.com with SMTP id d10so3281337pgm.13 for ; Fri, 27 Jan 2023 06:22:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=ikC6XA7MsmCBQ00WMTkPQsQlhEgRcChowsGt19Z7p0c=; b=jCz2+s4TvqzJmW+swchK46tKY7BuqgVB3uJ3fEjF/I775EAX7aq6zXat9t2EakVM6T zYqFK11Mifq1g0HCT9/ui3APOKrKHz9wf0lNQV0aRZ3Kv1aK7lcYastzhWkUZS3mILzC d7VOgGKcIS+g1UpFRC4vdnh2Dsd+0yp4XygcB4CxA3f+QodVuRRXB3Oh59txY8qhKQ9N yg0wdUhfld8/EGVMEx6Wr7aIAGT+JNRmoJEW46qQp39Wi2lz2YLawcAuYXePwjYA1KpF /EdfO+iR8zlqsVtoDFbGjnU/f77uiC5tx67ivZoBF90qgxzcepHGd4E9izLKZb31u9Ec GiZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ikC6XA7MsmCBQ00WMTkPQsQlhEgRcChowsGt19Z7p0c=; b=LHd7MPKN2P6KGX1omHXIjw8EIjXbuQyXkFo+iOx8hhXkXFoUsmYrKWdl77+zdL9+eh j6t81Ws3uT1h4hbq7DlkshnixbfmHBOmOpQ59GYXpGWABWc80AgPnndILiZtMXxa1P15 1AtOJwFPKRN8diVXZowy9dQAMYXjZI1PrAmd8zg3CFG3jBY8fti5XluV+zxh4ct/Go+S vyhspTSD9knXTMww5o8Q4+RQnn0IOUSKNn0VKCTFyjvpLGIlvtXd/r7ycfEcQBvISOIn go30LkuZbfL7FjJHSEB5qTT8JjJqEqkJ+CAl1APLH0wO2RRVNShy9A07unkfvOmTq2su DU2A== X-Gm-Message-State: AFqh2kpsbwy7nhwxCy5Kfu9AHYvtddDB1yOTLqbgjj7hfsCCPRWuqsVs wBmpQN/S6UuBkqUi7bPziGbvKPAkWuqFI2T45zc= X-Google-Smtp-Source: AMrXdXtQdRns4j9W9OXLoLL4/RUrLMhIOX2nzVInsgFb1g/Kc60EhPEB36LgPgvPwOSst7Aghdufm1uqqt6IbDYSFf8= X-Received: by 2002:aa7:96f8:0:b0:582:a224:e73f with SMTP id i24-20020aa796f8000000b00582a224e73fmr4881661pfq.30.1674829339647; Fri, 27 Jan 2023 06:22:19 -0800 (PST) MIME-Version: 1.0 References: <20230127092945.94389-1-savvamtr@gmail.com> <20230127092945.94389-6-savvamtr@gmail.com> In-Reply-To: <20230127092945.94389-6-savvamtr@gmail.com> From: "Pedro Falcato" Date: Fri, 27 Jan 2023 14:22:08 +0000 Message-ID: Subject: Re: [edk2-platforms][PATCH v3 05/11] Ext4Pkg: Fix shift out of bounds in Ext4OpenSuperblock To: Savva Mitrofanov Cc: devel@edk2.groups.io, =?UTF-8?Q?Marvin_H=C3=A4user?= , Vitaly Cheptsov Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Jan 27, 2023 at 9:29 AM Savva Mitrofanov wrote= : > > Missing check for wrong s_log_block_size exponent leads to shift out of > bounds. Limit block size to 2 MiB > > Cc: Marvin H=C3=A4user > Cc: Pedro Falcato > Cc: Vitaly Cheptsov > Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.") > Signed-off-by: Savva Mitrofanov > --- > Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 14 ++++++++++++++ > Features/Ext4Pkg/Ext4Dxe/Superblock.c | 5 +++++ > 2 files changed, 19 insertions(+) > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dx= e/Ext4Dxe.h > index 2e489ce4dd86..a23323319a59 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > @@ -40,6 +40,20 @@ > #define EXT4_EFI_PATH_MAX 4096 > #define EXT4_DRIVER_VERSION 0x0000 > > +// > +// The EXT4 Specification doesn't strictly limit block size and this val= ue could be up to 2^31, > +// but in practice it is limited by PAGE_SIZE due to performance signifi= cant impact. > +// Many EXT4 implementations have size of block limited to PAGE_SIZE. In= many cases it's limited > +// to 4096, which is a commonly supported page size on most MMU-capable = hardware, and up to 65536. > +// So, to take a balance between compatibility and security measures, it= is decided to use the > +// value of 2MiB as the limit, which is equal to page size on new hardwa= re. Nit: s/page size/large page size/ I can change this for you when pushing, no need for a v4 on this one. > +// As for supporting big block sizes, EXT4 has a RO_COMPAT_FEATURE calle= d BIGALLOC, which changes > +// EXT4 to use clustered allocation, so that each bit in the ext4 block = allocation bitmap addresses > +// a power of two number of blocks. So it would be wiser to implement an= d use this feature > +// if there is such a need instead of big block size. > +// > +#define EXT4_LOG_BLOCK_SIZE_MAX 11 > + > /** > Opens an ext4 partition and installs the Simple File System protocol. > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Superblock.c b/Features/Ext4Pkg/Ext= 4Dxe/Superblock.c > index be3527e4d618..3f56de93c105 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Superblock.c > +++ b/Features/Ext4Pkg/Ext4Dxe/Superblock.c > @@ -248,6 +248,11 @@ Ext4OpenSuperblock ( > return EFI_VOLUME_CORRUPTED; > } > > + if (Sb->s_log_block_size > EXT4_LOG_BLOCK_SIZE_MAX) { > + DEBUG ((DEBUG_ERROR, "[ext4] SuperBlock s_log_block_size %lu is too = big\n", Sb->s_log_block_size)); > + return EFI_UNSUPPORTED; > + } > + > Partition->BlockSize =3D (UINT32)LShiftU64 (1024, Sb->s_log_block_size= ); > > // The size of a block group can also be calculated as 8 * Partition->= BlockSize > -- > 2.39.0 > Reviewed-by: Pedro Falcato --=20 Pedro