From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753304AbdKILBc (ORCPT ); Thu, 9 Nov 2017 06:01:32 -0500 Received: from ozlabs.org ([103.22.144.67]:41863 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753016AbdKILBb (ORCPT ); Thu, 9 Nov 2017 06:01:31 -0500 From: Michael Ellerman To: Sukadev Bhattiprolu Cc: Benjamin Herrenschmidt , mikey@neuling.org, hbabu@us.ibm.com, nicholas.piggin@gmail.com, linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 14/18] powerpc: Define set_thread_used_vas() In-Reply-To: <1507343298-27496-15-git-send-email-sukadev@linux.vnet.ibm.com> References: <1507343298-27496-1-git-send-email-sukadev@linux.vnet.ibm.com> <1507343298-27496-15-git-send-email-sukadev@linux.vnet.ibm.com> Date: Thu, 09 Nov 2017 22:01:28 +1100 Message-ID: <87efp7pvg7.fsf@concordia.ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Sukadev Bhattiprolu writes: > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c > index d861fcd..cb5f108 100644 > --- a/arch/powerpc/kernel/process.c > +++ b/arch/powerpc/kernel/process.c > @@ -1234,17 +1234,17 @@ struct task_struct *__switch_to(struct task_struct *prev, > * The copy-paste buffer can only store into foreign real > * addresses, so unprivileged processes can not see the > * data or use it in any way unless they have foreign real > - * mappings. We don't have a VAS driver that allocates those > - * yet, so no cpabort is required. > + * mappings. If the new process has the foreign real address > + * mappings, we must issue a cp_abort to clear any state and > + * prevent a covert channel being setup. > + * > + * DD1 allows paste into normal system memory so we do an > + * unpaired copy, rather than cp_abort, to clear the buffer, > + * since cp_abort is quite expensive. > */ > - if (cpu_has_feature(CPU_FTR_POWER9_DD1)) { > - /* > - * DD1 allows paste into normal system memory, so we > - * do an unpaired copy here to clear the buffer and > - * prevent a covert channel being set up. > - * > - * cpabort is not used because it is quite expensive. > - */ > + if (new_thread->used_vas) { So this has a bug in that it's not safe to use new_thread here. Because this is on the way out of __switch_to(), this code can run both when switching to a new task and also when switching back to an older task. In the latter case the task pointed to by new_thread may have already been freed. I've fixed it up here to use current_thread_info()->task->thread.used_vas, so that it's always checking current. cheers