Hi Mark,
On 4/14/20 11:42 AM, Mark Rutland wrote:
> The aarch32_vdso_pages[] array never has entries allocated in the C_VVAR
> or C_VDSO slots, and as the array is zero initialized these contain
> NULL.
>
> However in __aarch32_alloc_vdso_pages() when
> aarch32_alloc_kuser_vdso_page() fails we attempt to free the page whose
> struct page is at NULL, which is obviously nonsensical.
>
Could you please explain why do you think that free(NULL) is "nonsensical"? And
if this is causing a bug (according to the cover-letter), could you please
provide a stack-trace?
> This patch removes the erroneous page freeing.
>
> Signed-off-by: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> arch/arm64/kernel/vdso.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index 354b11e27c07..033a48f30dbb 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -260,18 +260,7 @@ static int __aarch32_alloc_vdso_pages(void)
> if (ret)
> return ret;
>
> - ret = aarch32_alloc_kuser_vdso_page();
> - if (ret) {
> - unsigned long c_vvar =
> - (unsigned long)page_to_virt(aarch32_vdso_pages[C_VVAR]);
> - unsigned long c_vdso =
> - (unsigned long)page_to_virt(aarch32_vdso_pages[C_VDSO]);
> -
> - free_page(c_vvar);
> - free_page(c_vdso);
> - }
> -
> - return ret;
> + return aarch32_alloc_kuser_vdso_page();
> }
> #else
> static int __aarch32_alloc_vdso_pages(void)
>
--
Regards,
Vincenzo
