From patchwork Tue Jun 18 09:42:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 13702021 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 24FE4C2BA1A for ; Tue, 18 Jun 2024 09:43:08 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.742910.1149786 (Exim 4.92) (envelope-from ) id 1sJVMV-0002bd-IJ; Tue, 18 Jun 2024 09:42:47 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 742910.1149786; Tue, 18 Jun 2024 09:42:47 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sJVMV-0002bW-FW; Tue, 18 Jun 2024 09:42:47 +0000 Received: by outflank-mailman (input) for mailman id 742910; Tue, 18 Jun 2024 09:42:46 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sJVMU-0002bQ-6c for xen-devel@lists.xenproject.org; Tue, 18 Jun 2024 09:42:46 +0000 Received: from mail-ot1-x335.google.com (mail-ot1-x335.google.com [2607:f8b0:4864:20::335]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 1c6c19f9-2d57-11ef-b4bb-af5377834399; Tue, 18 Jun 2024 11:42:44 +0200 (CEST) Received: by mail-ot1-x335.google.com with SMTP id 46e09a7af769-6fb840d8ffdso2493323a34.1 for ; Tue, 18 Jun 2024 02:42:44 -0700 (PDT) Received: from localhost ([122.172.82.13]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-6fedf0bef16sm7753098a12.43.2024.06.18.02.42.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Jun 2024 02:42:42 -0700 (PDT) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 1c6c19f9-2d57-11ef-b4bb-af5377834399 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1718703763; x=1719308563; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=8UALesfbksrHHNtNwGYa4E+DNFkRXivhYiDbx6+KbJc=; b=S+74Pnur6hpPfzhn/qtKmLoZ3J1fVWoLtYJQkVpqpm8Rsz+XPBfu/OiEHwUt81k307 nrIWbLVS9z43ZtMP8KPs0fn3a8/dZQsxRZHFxqcbEhSZ1PpMod4j+dqN5BhSFRuppqUu qyCF+v9maudBj38cK1xCcNOXZc2jxb4/7uCfJ7fTF7IIbYarQOSELxqg8BJgwEyvv29a JEvl0pUgEtnn0GdRGE3bYEXxZX5SqQYQEkMNbtZWKQD3SSIpoaXGI+CM9W4Q7ujOw5wl 6Rel2CVqsrQYkhESv5kjmhbHIsB15CopEK1xYttYptzOmmE9fVRUP3N4V/1Vm4KLa1Tq NO+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718703763; x=1719308563; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=8UALesfbksrHHNtNwGYa4E+DNFkRXivhYiDbx6+KbJc=; b=UALrsZawBzkdHSuxS+TP/mtYkcV0fei8N7BbkKZMthClE8ULJ7+JgJrQc9kJ8maUEz PNbZ9FDdqEa5YwHgwa2gYnG2G51A2tAOkRCCG6vPbyhC34inDsuQHe9si88JBnclPhvK GkLLXAqObPCz90HlY2/N5tWHMN5Qq4AEQxBJX7x9ITAliRZ46y0DTin71SheaaZPOnWJ 7U5wi7BK0y3lAsUaF2loGdRktgok26cnYtMvi1218uesFYg8vEeJmD9DG2iAdhHtvR77 mBXZOJFAshFUzv/491AAVHvpuSiVSjL2E78RWe21morCPikomIRXuZzkFdR5y5PZJtRZ buMQ== X-Forwarded-Encrypted: i=1; AJvYcCW3byWCaxn3dVyGkdu4ofaKYkEYGKSuNoEDrZ6ikYUMNLMUVUFWdJTg8EmnqzKqkBKrsVCqxB7Gt3wpYV9rC/4pQmMHP07vyvcO5A7sMpY= X-Gm-Message-State: AOJu0Yx2Gpe4gbU3hCCaNa4Y0ZSE5ia4qjg6AxbO/YG22VgfPVX0aNr2 Q36lXSnQLQRxHwtNL3+/tYlJYJq/wtFcPeE5DER4n3E+txvwMF/CtvLshLnY/GQ= X-Google-Smtp-Source: AGHT+IHduV0HGFrVmQSxIvryeJgCIEd+h/riwq126F2u6x4F+za92Uiu45uPCTH4ThXJnQkPLkaziw== X-Received: by 2002:a9d:4d91:0:b0:6fa:81b:d4f8 with SMTP id 46e09a7af769-6fb9364ecd9mr12749877a34.1.1718703762611; Tue, 18 Jun 2024 02:42:42 -0700 (PDT) From: Viresh Kumar To: Juergen Gross , Stefano Stabellini , Oleksandr Tyshchenko Cc: Viresh Kumar , Vincent Guittot , =?utf-8?q?Alex_Benn=C3=A9e?= , Manos Pitsidianakis , Paolo Bonzini , Al Viro , xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/2] xen: privcmd: Switch from mutex to spinlock for irqfds Date: Tue, 18 Jun 2024 15:12:28 +0530 Message-Id: X-Mailer: git-send-email 2.31.1.272.g89b43f80a514 MIME-Version: 1.0 irqfd_wakeup() gets EPOLLHUP, when it is called by eventfd_release() by way of wake_up_poll(&ctx->wqh, EPOLLHUP), which gets called under spin_lock_irqsave(). We can't use a mutex here as it will lead to a deadlock. Fix it by switching over to a spin lock. Reported-by: Al Viro Signed-off-by: Viresh Kumar Reviewed-by: Juergen Gross --- drivers/xen/privcmd.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 67dfa4778864..5ceb6c56cf3e 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -13,7 +13,6 @@ #include #include #include -#include #include #include #include @@ -845,7 +844,7 @@ static long privcmd_ioctl_mmap_resource(struct file *file, #ifdef CONFIG_XEN_PRIVCMD_EVENTFD /* Irqfd support */ static struct workqueue_struct *irqfd_cleanup_wq; -static DEFINE_MUTEX(irqfds_lock); +static DEFINE_SPINLOCK(irqfds_lock); static LIST_HEAD(irqfds_list); struct privcmd_kernel_irqfd { @@ -909,9 +908,11 @@ irqfd_wakeup(wait_queue_entry_t *wait, unsigned int mode, int sync, void *key) irqfd_inject(kirqfd); if (flags & EPOLLHUP) { - mutex_lock(&irqfds_lock); + unsigned long flags; + + spin_lock_irqsave(&irqfds_lock, flags); irqfd_deactivate(kirqfd); - mutex_unlock(&irqfds_lock); + spin_unlock_irqrestore(&irqfds_lock, flags); } return 0; @@ -929,6 +930,7 @@ irqfd_poll_func(struct file *file, wait_queue_head_t *wqh, poll_table *pt) static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) { struct privcmd_kernel_irqfd *kirqfd, *tmp; + unsigned long flags; __poll_t events; struct fd f; void *dm_op; @@ -968,18 +970,18 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) init_waitqueue_func_entry(&kirqfd->wait, irqfd_wakeup); init_poll_funcptr(&kirqfd->pt, irqfd_poll_func); - mutex_lock(&irqfds_lock); + spin_lock_irqsave(&irqfds_lock, flags); list_for_each_entry(tmp, &irqfds_list, list) { if (kirqfd->eventfd == tmp->eventfd) { ret = -EBUSY; - mutex_unlock(&irqfds_lock); + spin_unlock_irqrestore(&irqfds_lock, flags); goto error_eventfd; } } list_add_tail(&kirqfd->list, &irqfds_list); - mutex_unlock(&irqfds_lock); + spin_unlock_irqrestore(&irqfds_lock, flags); /* * Check if there was an event already pending on the eventfd before we @@ -1011,12 +1013,13 @@ static int privcmd_irqfd_deassign(struct privcmd_irqfd *irqfd) { struct privcmd_kernel_irqfd *kirqfd; struct eventfd_ctx *eventfd; + unsigned long flags; eventfd = eventfd_ctx_fdget(irqfd->fd); if (IS_ERR(eventfd)) return PTR_ERR(eventfd); - mutex_lock(&irqfds_lock); + spin_lock_irqsave(&irqfds_lock, flags); list_for_each_entry(kirqfd, &irqfds_list, list) { if (kirqfd->eventfd == eventfd) { @@ -1025,7 +1028,7 @@ static int privcmd_irqfd_deassign(struct privcmd_irqfd *irqfd) } } - mutex_unlock(&irqfds_lock); + spin_unlock_irqrestore(&irqfds_lock, flags); eventfd_ctx_put(eventfd); @@ -1073,13 +1076,14 @@ static int privcmd_irqfd_init(void) static void privcmd_irqfd_exit(void) { struct privcmd_kernel_irqfd *kirqfd, *tmp; + unsigned long flags; - mutex_lock(&irqfds_lock); + spin_lock_irqsave(&irqfds_lock, flags); list_for_each_entry_safe(kirqfd, tmp, &irqfds_list, list) irqfd_deactivate(kirqfd); - mutex_unlock(&irqfds_lock); + spin_unlock_irqrestore(&irqfds_lock, flags); destroy_workqueue(irqfd_cleanup_wq); } From patchwork Tue Jun 18 09:42:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 13702020 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B2E02C27C4F for ; Tue, 18 Jun 2024 09:43:07 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.742911.1149797 (Exim 4.92) (envelope-from ) id 1sJVMY-0002qH-Sk; Tue, 18 Jun 2024 09:42:50 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 742911.1149797; Tue, 18 Jun 2024 09:42:50 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sJVMY-0002qA-Pf; Tue, 18 Jun 2024 09:42:50 +0000 Received: by outflank-mailman (input) for mailman id 742911; Tue, 18 Jun 2024 09:42:49 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sJVMX-0002bQ-HV for xen-devel@lists.xenproject.org; Tue, 18 Jun 2024 09:42:49 +0000 Received: from mail-pf1-x429.google.com (mail-pf1-x429.google.com [2607:f8b0:4864:20::429]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id 1e94caf9-2d57-11ef-b4bb-af5377834399; Tue, 18 Jun 2024 11:42:47 +0200 (CEST) Received: by mail-pf1-x429.google.com with SMTP id d2e1a72fcca58-705fff50de2so1318800b3a.1 for ; Tue, 18 Jun 2024 02:42:47 -0700 (PDT) Received: from localhost ([122.172.82.13]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-705ccb92c89sm8582018b3a.213.2024.06.18.02.42.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Jun 2024 02:42:45 -0700 (PDT) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 1e94caf9-2d57-11ef-b4bb-af5377834399 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1718703766; x=1719308566; darn=lists.xenproject.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=jSBvd6UdPXxQAYikzAX9WnO+l3sKpqabC3P6H43CjNw=; b=RLTRwGd40dE3Q8y3kuTkx25H0711nLMlpxztcrfr3F/bbJSK0Eb8yjXunbbbAqb8ZU lv6b6SFthbg7ESD2PnYUO8WSVylKYT3HdgdGkuGZ09AZUMOFw3p8tvzEU7skT7pW1dK1 CjdQR9YaboaFXq0DqA7v6zldvfB4o8ddgr0dGshCsBSFzlMzMtSxpHfJdHPiDlAdRKKY cZMFTBf9Kf8Tu+IX/bAC3K7ADNFqS3ypPTlk9qQ8i1f+ILbMfjIutsrv/OUWxQuJPSlY eEeMO//O7fCeDAfRwzCjQsPgcHmTFEKnrn3jLkeUPKWSxqZESYxMOr+rD4izooWFeAt3 zZ0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718703766; x=1719308566; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=jSBvd6UdPXxQAYikzAX9WnO+l3sKpqabC3P6H43CjNw=; b=q93f5cRtSMTOknJjqiM3w9Tu+9eXNTS/zyW+esjyCiZKPEoart6OBrbbzbndTzFJRO yevkSX3ZK9oJebt1B1awEfYXpW1k28lfRLbXnLqLnKc2ccwCNIC8h1PJwLfn3w230aYg 9raEwBqKXk/3ialWKEUsvQIJF+EQCtC3zHVAhPRsTB9Qf20l/e/Wq+bYX/bQTuSdvoqi 41LN8dtFb7YHwUMtKnQhTHYUlZHPEodKXZyj/7HfHzGbuJ6DkdIx4iHoT3S8Rf/4WRRP K1Pyj47p+PE8cWt8xdbibmEpojkdxbXpb2PZ0fbD5AQ2p3WoD8SlV8XeZCPQWNYWFhSW zPvw== X-Forwarded-Encrypted: i=1; AJvYcCX/IimIETHjuQeQCf2mIX5FqrPT+EzRaZ4Dh+Xa0yiAqFGFqIPYenriA8RA6wy3HZVPmgsf3XSxJ3/S0vy8KMSayTarE4cJxa5fpSbEnIA= X-Gm-Message-State: AOJu0Ywz5H/QwOsCBIwVuR9afmEaCOkt2IW1xPHtkk33aiOfkn/uNMj+ QZ2YYFRzzK9J3z2IN1mt6JcNO2zsi1bQucNiQ3az3eYhNbEhkVGDzMqBnX3a6jA= X-Google-Smtp-Source: AGHT+IHcl/2RoXVQ+71aHNYVjn3NoTwhv2qPyWAbcWs0ParO44xZQCCVzutKlPjbF2/gE9SyhHgTKQ== X-Received: by 2002:a05:6a00:4b47:b0:706:1bb3:fb1d with SMTP id d2e1a72fcca58-7061bb3fc6amr3055052b3a.7.1718703766091; Tue, 18 Jun 2024 02:42:46 -0700 (PDT) From: Viresh Kumar To: Juergen Gross , Stefano Stabellini , Oleksandr Tyshchenko Cc: Viresh Kumar , Vincent Guittot , =?utf-8?q?Alex_Benn=C3=A9e?= , Manos Pitsidianakis , Paolo Bonzini , Al Viro , xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org Subject: [PATCH 2/2] xen: privcmd: Fix possible access to a freed kirqfd instance Date: Tue, 18 Jun 2024 15:12:29 +0530 Message-Id: <9e884af1f1f842eacbb7afc5672c8feb4dea7f3f.1718703669.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.31.1.272.g89b43f80a514 In-Reply-To: References: MIME-Version: 1.0 Nothing prevents simultaneous ioctl calls to privcmd_irqfd_assign() and privcmd_irqfd_deassign(). If that happens, it is possible that a kirqfd created and added to the irqfds_list by privcmd_irqfd_assign() may get removed by another thread executing privcmd_irqfd_deassign(), while the former is still using it after dropping the locks. This can lead to a situation where an already freed kirqfd instance may be accessed and cause kernel oops. Use SRCU locking to prevent the same, as is done for the KVM implementation for irqfds. Reported-by: Al Viro Suggested-by: Paolo Bonzini Signed-off-by: Viresh Kumar Reviewed-by: Juergen Gross --- drivers/xen/privcmd.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 5ceb6c56cf3e..041774750e52 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -845,6 +846,7 @@ static long privcmd_ioctl_mmap_resource(struct file *file, /* Irqfd support */ static struct workqueue_struct *irqfd_cleanup_wq; static DEFINE_SPINLOCK(irqfds_lock); +DEFINE_STATIC_SRCU(irqfds_srcu); static LIST_HEAD(irqfds_list); struct privcmd_kernel_irqfd { @@ -872,6 +874,9 @@ static void irqfd_shutdown(struct work_struct *work) container_of(work, struct privcmd_kernel_irqfd, shutdown); u64 cnt; + /* Make sure irqfd has been initialized in assign path */ + synchronize_srcu(&irqfds_srcu); + eventfd_ctx_remove_wait_queue(kirqfd->eventfd, &kirqfd->wait, &cnt); eventfd_ctx_put(kirqfd->eventfd); kfree(kirqfd); @@ -934,7 +939,7 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) __poll_t events; struct fd f; void *dm_op; - int ret; + int ret, idx; kirqfd = kzalloc(sizeof(*kirqfd) + irqfd->size, GFP_KERNEL); if (!kirqfd) @@ -980,6 +985,7 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) } } + idx = srcu_read_lock(&irqfds_srcu); list_add_tail(&kirqfd->list, &irqfds_list); spin_unlock_irqrestore(&irqfds_lock, flags); @@ -991,6 +997,8 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) if (events & EPOLLIN) irqfd_inject(kirqfd); + srcu_read_unlock(&irqfds_srcu, idx); + /* * Do not drop the file until the kirqfd is fully initialized, otherwise * we might race against the EPOLLHUP.