On Thu, 2022-10-27 at 08:31 -0700, 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.
>
>
How about below?
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -614,6 +614,14 @@ static inline int cal_tdmr_size(void)
tdmr_sz += sizeof(struct tdmr_reserved_area) *
tdx_sysinfo.max_reserved_per_tdmr;
+ /*
+ * Do simple check against overflow, and return 0 (invalid)
+ * TDMR_INFO size if it happened. Also WARN() as it should
+ * should never happen in reality.
+ */
+ if (WARN_ON_ONCE(tdmr_sz < 0))
+ return 0;
+
/*
* TDX requires each TDMR_INFO to be 512-byte aligned. Always
* round up TDMR_INFO size to the 512-byte boundary.
@@ -623,19 +631,27 @@ static inline int cal_tdmr_size(void)
static struct tdmr_info *alloc_tdmr_array(int *array_sz)
{
+ int sz;
+
/*
* TDX requires each TDMR_INFO to be 512-byte aligned.
* Use alloc_pages_exact() to allocate all TDMRs at once.
* Each TDMR_INFO will still be 512-byte aligned since
* cal_tdmr_size() always return 512-byte aligned size.
*/
- *array_sz = cal_tdmr_size() * tdx_sysinfo.max_tdmrs;
+ sz = cal_tdmr_size() * tdx_sysinfo.max_tdmrs;
+
+ /* Overflow */
+ if (!sz || WARN_ON_ONCE(sz < 0))
+ return NULL;
+
+ *array_sz = sz;
/*
* Zero the buffer so 'struct tdmr_info::size' can be
* used to determine whether a TDMR is valid.
*/
- return alloc_pages_exact(*array_sz, GFP_KERNEL | __GFP_ZERO);
+ return alloc_pages_exact(sz, GFP_KERNEL | __GFP_ZERO);
}
Btw, should I use alloc_contig_pages() instead of alloc_pages_exact() as IIUC
the latter should fail if the size is larger than 4MB? In reality, the entire
array only takes dozens of KBs, though.
