On 6/27/23 07:43, Julia Lawall wrote:
> Use vmalloc_array and vcalloc to protect against
> multiplication overflows.
...
> diff -u -p a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -628,7 +628,7 @@ static bool __init sgx_setup_epc_section
> if (!section->virt_addr)
> return false;
>
> - section->pages = vmalloc(nr_pages * sizeof(struct sgx_epc_page));
> + section->pages = vmalloc_array(nr_pages, sizeof(struct sgx_epc_page));
> if (!section->pages) {
I'm not sure that changelog matches the code.
'nr_pages' here is an 'unsigned long' and The sizeof()==32. In
practice, the multiplication can be done with a shift, and the ulong is
a *LONG* way from overflowing.
I'll accept that, as a general rule, vmalloc_array() is the preferred
form. It's totally possible that someone could copy and paste the
nr_foo*sizeof(struct bar) code over to a place where nr_foo is a more
troublesome type.
But, if that's the true motivation, could we please say that in the
changelog? As it stands, it's a bit silly to be talking about
multiplication overflows, unless I'm missing something totally obvious.
