linux-kernel - Re: [PATCH v6 12/21] x86/virt/tdx: Add placeholder to construct TDMRs to cover all TDX memory regions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 3 Nov 2022 08:05:14 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Andi Kleen <ak@...ux.intel.com>, Kai Huang <kai.huang@...el.com>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc:     linux-mm@...ck.org, seanjc@...gle.com, pbonzini@...hat.com,
        dan.j.williams@...el.com, rafael.j.wysocki@...el.com,
        kirill.shutemov@...ux.intel.com, reinette.chatre@...el.com,
        len.brown@...el.com, tony.luck@...el.com, peterz@...radead.org,
        isaku.yamahata@...el.com, chao.gao@...el.com,
        sathyanarayanan.kuppuswamy@...ux.intel.com, bagasdotme@...il.com,
        sagis@...gle.com, imammedo@...hat.com
Subject: Re: [PATCH v6 12/21] x86/virt/tdx: Add placeholder to construct TDMRs
 to cover all TDX memory regions

On 10/27/22 08:31, Andi Kleen wrote:
> 
>> +/* Calculate the actual TDMR_INFO size */
>> +static inline int cal_tdmr_size(void)
>> +{
>> +    int tdmr_sz;
>> +
>> +    /*
>> +     * The actual size of TDMR_INFO depends on the maximum number
>> +     * of reserved areas.
>> +     */
>> +    tdmr_sz = sizeof(struct tdmr_info);
>> +    tdmr_sz += sizeof(struct tdmr_reserved_area) *
>> +           tdx_sysinfo.max_reserved_per_tdmr;
> 
> would seem safer to have a overflow check here.

tdmr_reserved_area is 16 bytes.  To overflow a signed int, tdmr_sz would
need to be for an allocation >2GB.  alloc_pages_exact() tops out at
supplying 4MB allocations.

So, sure, this breaks at max_reserved_per_tdmr>2^27, but it actually
breaks *EARLIER* at max_reserved_per_tdmr>2^18 because the page
allocator is borked.

Plus, max_reserved_per_tdmr is barely in double digits today.  It's a
*LOOOOOOOOONG* way from either of those limits.  If you want to add a
warning here, then go for it and enforce a sane value on
max_reserved_per_tdmr.

But, the overflow is *LITERALLY* an order of magnitude more obscure than
overwhelming the page allocator.  Let's not clutter up the code with
silly checks like that.