From patchwork Tue Feb 23 13:23:00 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tim Gardner X-Patchwork-Id: 1443484 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DlKW20tgyz9sVV; Wed, 24 Feb 2021 00:23:24 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1lEXen-0006qI-0H; Tue, 23 Feb 2021 13:23:17 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lEXel-0006q3-Az for kernel-team@lists.ubuntu.com; Tue, 23 Feb 2021 13:23:15 +0000 Received: from mail-pj1-f71.google.com ([209.85.216.71]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lEXek-0006X7-Ur for kernel-team@lists.ubuntu.com; Tue, 23 Feb 2021 13:23:15 +0000 Received: by mail-pj1-f71.google.com with SMTP id lk3so1548287pjb.9 for ; Tue, 23 Feb 2021 05:23:14 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ueQdOXFhwjkEXhXc7jp2gNvbG2Lxrdt3JV6olL7p+jM=; b=PLLG3N3ywKpb9dQOTJ564QW5rYHS6WjoydrBJ8LyoU9Ed/P1VIlYfOityvM6/Jsk+E O6vTsHEmJ+ULOc4OpW2gOgAesL0aSad8AuREFI79pIgJ1fXMs+t23XEtd+QwwFYgjd8B zRJvdEIMYclmhEMCT3ord7czS1oVPSevoPYsWB/eqIZ/Y8MeQ4+toNYjaPCkCH6QiYCB A/w+4Nepz3cEj86qneO/Pf0j3IJiS85glolUP+G/Pfk83CW7QXq3T3My+xVXMh7xry/D 4414EyXEdG6/1ngKTT82wjRnuyekzWAOWilIVwagfbIKkaUxvYxhe+B+JevAm4fWU6Ko XUVQ== X-Gm-Message-State: AOAM532IxBzPIScGgNe7M+0Pq1j3yMu19m2YuFC13abO/Op1f80tuGDz pZYpUtGKoEbDk0xJiA20fAWXEuEhSJBdKJf7VKEB7HH1MWkZTy+AoL36d7bdsVHGL6MCG697y1W NWI3eoZbaoUeEt9J+4pyqMXefVwbOJmntIjvSRbAliA== X-Received: by 2002:a17:90a:5b0c:: with SMTP id o12mr29449060pji.68.1614086593278; Tue, 23 Feb 2021 05:23:13 -0800 (PST) X-Google-Smtp-Source: ABdhPJyGRDQh+pp3Lney9pT/bC5DSbg0+NofBNX5RTAOfQNmPi8JomOV/lwOG12/nSedmp9kXfpq6Q== X-Received: by 2002:a17:90a:5b0c:: with SMTP id o12mr29449042pji.68.1614086592978; Tue, 23 Feb 2021 05:23:12 -0800 (PST) Received: from localhost.localdomain ([69.163.84.166]) by smtp.gmail.com with ESMTPSA id o21sm3176615pjp.42.2021.02.23.05.23.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Feb 2021 05:23:12 -0800 (PST) From: Tim Gardner To: kernel-team@lists.ubuntu.com Subject: [PATCH] xen/events: avoid removing an event channel while handling it Date: Tue, 23 Feb 2021 06:23:00 -0700 Message-Id: <20210223132300.2651-2-tim.gardner@canonical.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20210223132300.2651-1-tim.gardner@canonical.com> References: <20210223132300.2651-1-tim.gardner@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Juergen Gross Today it can happen that an event channel is being removed from the system while the event handling loop is active. This can lead to a race resulting in crashes or WARN() splats when trying to access the irq_info structure related to the event channel. Fix this problem by using a rwlock taken as reader in the event handling loop and as writer when deallocating the irq_info structure. As the observed problem was a NULL dereference in evtchn_from_irq() make this function more robust against races by testing the irq_info pointer to be not NULL before dereferencing it. And finally make all accesses to evtchn_to_irq[row][col] atomic ones in order to avoid seeing partial updates of an array element in irq handling. Note that irq handling can be entered only for event channels which have been valid before, so any not populated row isn't a problem in this regard, as rows are only ever added and never removed. This is XSA-331. Cc: stable@vger.kernel.org Reported-by: Marek Marczykowski-Górecki Reported-by: Jinoh Kang Signed-off-by: Juergen Gross Reviewed-by: Stefano Stabellini Reviewed-by: Wei Liu (backported from commit 073d0552ead5bfc7a3a9c01de590e924f11b5dd2) Signed-off-by: Tim Gardner Conflicts: drivers/xen/events/events_base.c Minor context adjustment in xen_free_irq(). Acked-by: Thadeu Lima de Souza Cascardo Acked-by: Stefan Bader --- drivers/xen/events/events_base.c | 41 ++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c index 499eff7d3f65..c56fc81a409f 100644 --- a/drivers/xen/events/events_base.c +++ b/drivers/xen/events/events_base.c @@ -33,6 +33,7 @@ #include #include #include +#include #ifdef CONFIG_X86 #include @@ -70,6 +71,23 @@ const struct evtchn_ops *evtchn_ops; */ static DEFINE_MUTEX(irq_mapping_update_lock); +/* + * Lock protecting event handling loop against removing event channels. + * Adding of event channels is no issue as the associated IRQ becomes active + * only after everything is setup (before request_[threaded_]irq() the handler + * can't be entered for an event, as the event channel will be unmasked only + * then). + */ +static DEFINE_RWLOCK(evtchn_rwlock); + +/* + * Lock hierarchy: + * + * irq_mapping_update_lock + * evtchn_rwlock + * IRQ-desc lock + */ + static LIST_HEAD(xen_irq_list_head); /* IRQ <-> VIRQ mapping. */ @@ -102,7 +120,7 @@ static void clear_evtchn_to_irq_row(unsigned row) unsigned col; for (col = 0; col < EVTCHN_PER_ROW; col++) - evtchn_to_irq[row][col] = -1; + WRITE_ONCE(evtchn_to_irq[row][col], -1); } static void clear_evtchn_to_irq_all(void) @@ -139,7 +157,7 @@ static int set_evtchn_to_irq(unsigned evtchn, unsigned irq) clear_evtchn_to_irq_row(row); } - evtchn_to_irq[row][col] = irq; + WRITE_ONCE(evtchn_to_irq[row][col], irq); return 0; } @@ -149,7 +167,7 @@ int get_evtchn_to_irq(unsigned evtchn) return -1; if (evtchn_to_irq[EVTCHN_ROW(evtchn)] == NULL) return -1; - return evtchn_to_irq[EVTCHN_ROW(evtchn)][EVTCHN_COL(evtchn)]; + return READ_ONCE(evtchn_to_irq[EVTCHN_ROW(evtchn)][EVTCHN_COL(evtchn)]); } /* Get info for IRQ */ @@ -247,10 +265,14 @@ static void xen_irq_info_cleanup(struct irq_info *info) */ unsigned int evtchn_from_irq(unsigned irq) { - if (WARN(irq >= nr_irqs, "Invalid irq %d!\n", irq)) + const struct irq_info *info = NULL; + + if (likely(irq < nr_irqs)) + info = info_for_irq(irq); + if (!info) return 0; - return info_for_irq(irq)->evtchn; + return info->evtchn; } unsigned irq_from_evtchn(unsigned int evtchn) @@ -426,16 +448,21 @@ static int __must_check xen_allocate_irq_gsi(unsigned gsi) static void xen_free_irq(unsigned irq) { struct irq_info *info = irq_get_handler_data(irq); + unsigned long flags; if (WARN_ON(!info)) return; + write_lock_irqsave(&evtchn_rwlock, flags); + list_del(&info->list); irq_set_handler_data(irq, NULL); WARN_ON(info->refcnt > 0); + write_unlock_irqrestore(&evtchn_rwlock, flags); + kfree(info); /* Legacy IRQ descriptors are managed by the arch. */ @@ -1218,6 +1245,8 @@ static void __xen_evtchn_do_upcall(void) struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu); int cpu = smp_processor_id(); + read_lock(&evtchn_rwlock); + do { vcpu_info->evtchn_upcall_pending = 0; @@ -1228,6 +1257,8 @@ static void __xen_evtchn_do_upcall(void) virt_rmb(); /* Hypervisor can set upcall pending. */ } while (vcpu_info->evtchn_upcall_pending); + + read_unlock(&evtchn_rwlock); } void xen_evtchn_do_upcall(struct pt_regs *regs)