From patchwork Sat Nov 20 00:43:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 12629917 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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7DF86C433EF for ; Sat, 20 Nov 2021 00:44:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 098216B0073; Fri, 19 Nov 2021 19:43:31 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0472E6B0074; Fri, 19 Nov 2021 19:43:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E4FA16B0075; Fri, 19 Nov 2021 19:43:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0204.hostedemail.com [216.40.44.204]) by kanga.kvack.org (Postfix) with ESMTP id D72666B0073 for ; Fri, 19 Nov 2021 19:43:30 -0500 (EST) Received: from smtpin14.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 98426184956CF for ; Sat, 20 Nov 2021 00:43:20 +0000 (UTC) X-FDA: 78827459760.14.7FB2E28 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf14.hostedemail.com (Postfix) with ESMTP id 7378960019B0 for ; Sat, 20 Nov 2021 00:43:19 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id 13A116137B; Sat, 20 Nov 2021 00:43:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1637368999; bh=vOWTynVx4zwicGyvUxV2nN/kGrMICANjdSzlNaICxKg=; h=Date:From:To:Subject:In-Reply-To:From; b=FspD0Ml5ZNR1EYPRgaxswa4a/Kj8UfHTc8PKBUN+UOd1TL8OMnhQ9hGwDAwNvjZNy 17sZNs8wz1UG+jG//NLCWLONl9qsYr06wlbiykPPBJwlVFRQDs8ho2DO8cVaAtiowv Zede4B0slOzMlEZRnOZf7EtPX3OjZt6aFzHcFvCE= Date: Fri, 19 Nov 2021 16:43:18 -0800 From: Andrew Morton To: akpm@linux-foundation.org, alexander.mikhalitsyn@virtuozzo.com, avagin@gmail.com, dave@stgolabs.net, ebiederm@xmission.com, gregkh@linuxfoundation.org, linux-mm@kvack.org, manfred@colorfullife.com, mm-commits@vger.kernel.org, ptikhomirov@virtuozzo.com, stable@vger.kernel.org, torvalds@linux-foundation.org, vvs@virtuozzo.com Subject: [patch 02/15] ipc: WARN if trying to remove ipc object which is absent Message-ID: <20211120004318.MW2qxdhTI%akpm@linux-foundation.org> In-Reply-To: <20211119164248.50feee07c5d2cc6cc4addf97@linux-foundation.org> User-Agent: s-nail v14.8.16 X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 7378960019B0 X-Stat-Signature: p161rnn5h4ic1satuscgj5p9cm99p1ei Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=FspD0Ml5; spf=pass (imf14.hostedemail.com: domain of akpm@linux-foundation.org designates 198.145.29.99 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org; dmarc=none X-HE-Tag: 1637368999-75480 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: From: Alexander Mikhalitsyn Subject: ipc: WARN if trying to remove ipc object which is absent Patch series "shm: shm_rmid_forced feature fixes". Some time ago I met kernel crash after CRIU restore procedure, fortunately, it was CRIU restore, so, I had dump files and could do restore many times and crash reproduced easily. After some investigation I've constructed the minimal reproducer. It was found that it's use-after-free and it happens only if sysctl kernel.shm_rmid_forced = 1. The key of the problem is that the exit_shm() function not handles shp's object destroy when task->sysvshm.shm_clist contains items from different IPC namespaces. In most cases this list will contain only items from one IPC namespace. Why this list may contain object from different namespaces? Function exit_shm() designed to clean up this list always when process leaves IPC namespace. But we made a mistake a long time ago and not add exit_shm() call into setns() syscall procedures. 1st second idea was just to add this call to setns() syscall but it's obviously changes semantics of setns() syscall and that's userspace-visible change. So, I gave up this idea. First real attempt to address the issue was just to omit forced destroy if we meet shp object not from current task IPC namespace [1]. But that was not the best idea because task->sysvshm.shm_clist was protected by rwsem which belongs to current task IPC namespace. It means that list corruption may occur. Second approach is just extend exit_shm() to properly handle shp's from different IPC namespaces [2]. This is really non-trivial thing, I've put a lot of effort into that but not believed that it's possible to make it fully safe, clean and clear. Thanks to the efforts of Manfred Spraul working an elegant solution was designed. Thanks a lot, Manfred! Eric also suggested the way to address the issue in ("[RFC][PATCH] shm: In shm_exit destroy all created and never attached segments") Eric's idea was to maintain a list of shm_clists one per IPC namespace, use lock-less lists. But there is some extra memory consumption-related concerns. Alternative solution which was suggested by me was implemented in ("shm: reset shm_clist on setns but omit forced shm destroy") Idea is pretty simple, we add exit_shm() syscall to setns() but DO NOT destroy shm segments even if sysctl kernel.shm_rmid_forced = 1, we just clean up the task->sysvshm.shm_clist list. This chages semantics of setns() syscall a little bit but in comparision to "naive" solution when we just add exit_shm() without any special exclusions this looks like a safer option. [1] https://lkml.org/lkml/2021/7/6/1108 [2] https://lkml.org/lkml/2021/7/14/736 This patch (of 2): Let's produce a warning if we trying to remove non-existing IPC object from IPC namespace kht/idr structures. This allows to catch possible bugs when ipc_rmid() function was called with inconsistent struct ipc_ids*, struct kern_ipc_perm* arguments. Link: https://lkml.kernel.org/r/20211027224348.611025-1-alexander.mikhalitsyn@virtuozzo.com Link: https://lkml.kernel.org/r/20211027224348.611025-2-alexander.mikhalitsyn@virtuozzo.com Co-developed-by: Manfred Spraul Signed-off-by: Manfred Spraul Signed-off-by: Alexander Mikhalitsyn Cc: "Eric W. Biederman" Cc: Davidlohr Bueso Cc: Greg KH Cc: Andrei Vagin Cc: Pavel Tikhomirov Cc: Vasily Averin Cc: Signed-off-by: Andrew Morton --- ipc/util.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) --- a/ipc/util.c~ipc-warn-if-trying-to-remove-ipc-object-which-is-absent +++ a/ipc/util.c @@ -447,8 +447,8 @@ static int ipcget_public(struct ipc_name static void ipc_kht_remove(struct ipc_ids *ids, struct kern_ipc_perm *ipcp) { if (ipcp->key != IPC_PRIVATE) - rhashtable_remove_fast(&ids->key_ht, &ipcp->khtnode, - ipc_kht_params); + WARN_ON_ONCE(rhashtable_remove_fast(&ids->key_ht, &ipcp->khtnode, + ipc_kht_params)); } /** @@ -498,7 +498,7 @@ void ipc_rmid(struct ipc_ids *ids, struc { int idx = ipcid_to_idx(ipcp->id); - idr_remove(&ids->ipcs_idr, idx); + WARN_ON_ONCE(idr_remove(&ids->ipcs_idr, idx) != ipcp); ipc_kht_remove(ids, ipcp); ids->in_use--; ipcp->deleted = true;