From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D5695C282C2 for ; Thu, 7 Feb 2019 18:44:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A035421872 for ; Thu, 7 Feb 2019 18:44:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="g0GrSg02" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726654AbfBGSoM (ORCPT ); Thu, 7 Feb 2019 13:44:12 -0500 Received: from mail-vs1-f65.google.com ([209.85.217.65]:33331 "EHLO mail-vs1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726454AbfBGSoL (ORCPT ); Thu, 7 Feb 2019 13:44:11 -0500 Received: by mail-vs1-f65.google.com with SMTP id p74so589405vsc.0 for ; Thu, 07 Feb 2019 10:44:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=GdUcA31369xF1FmEUY0RkeddSMsmwKpW3C4liR1hUSs=; b=g0GrSg02qKaKUxJyL2wPllBmfeAtBiiUgkfzdaRkekake0DMOQ6YZ37eKv83wpTc5x +y+58qu7d3gnk8pNSE6ljPO4fjIRNRGwVJlflGN+oxL0GMG03fnpfm9Dk4CNvZMXGLcM TqjRxZmRrhQ3Jg1mgFk22gebIIaHKoMRouzBd9kqA60Dh03j9iq5/7NBZzmuea/XGrZ0 n70z8R4Ckl++Hvfcdh03oY7iAfaxYoq/EXsAItt/oqA+ErvpIN4hf98qHJrGONin0qYN 03UfdQyVHrgakpTmil0l/LIAQgZJSTTYwbmwn3o7I43NLVTBLidAc+VsF3ipXOTW3GcO xweg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=GdUcA31369xF1FmEUY0RkeddSMsmwKpW3C4liR1hUSs=; b=q+DNPXA88bTe9po8QWD1pRURrnbG3gpQIdbYskhaFWNV3YW7kEAv+VwEeTXh4wz1Wu 6xdo1KXeflSw2IaQ6+dGpE7fZOFnGmQIJKRJfbYcm5VI6/0p7VBK3K0p4wsyH5vb3lVR GYikVwEzYjb4mC37NlR/gJdzmePg2HyC/iX4xGo5mNM/RtHJ9qBsGxD/WOkd8DwLCrY+ l+7WjZnAqmAID379qqAxR3D80zTF7rhzvffKdhk1K67U0OkQ7+DOo8WiPoCGRtguCMdf sBo+MecW5T8/A+lkxpl3EGMnskQaT2QFrFL25dKz2Ed0x4K2gdCi2/NFzCXM2tMMW0Ft aeAQ== X-Gm-Message-State: AHQUAubCKcpu+Q5spKoMpt5ITCMsxR+ymdpiAM0Yuqavq0zKghaPAycz LBFUfm1IKcZ1E3MMANsWTot56V/mqPE8+a98aKvdtA== X-Google-Smtp-Source: AHgI3Ibpm7o1dsD0a8fkln+L15N2NhTTGgKmd73qp8xHL01BxvKlYThjmLegFQxgT5rkZtguCWh5VEawVCOidfC5Pwc= X-Received: by 2002:a67:8314:: with SMTP id f20mr7412460vsd.121.1549565050338; Thu, 07 Feb 2019 10:44:10 -0800 (PST) MIME-Version: 1.0 References: <20190206195354.40576-1-sqazi@google.com> <20190207041455.GY2217@ZenIV.linux.org.uk> In-Reply-To: <20190207041455.GY2217@ZenIV.linux.org.uk> From: Salman Qazi Date: Thu, 7 Feb 2019 10:43:58 -0800 Message-ID: Subject: Re: [PATCH] fs, ipc: Use an asynchronous version of kern_unmount in IPC To: Al Viro Cc: Eric Biederman , Eric Dumazet , linux-fsdevel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Wed, Feb 6, 2019 at 8:14 PM Al Viro wrote: > > On Wed, Feb 06, 2019 at 11:53:54AM -0800, Salman Qazi wrote: > > > This patch solves the issue by removing synchronize_rcu from mq_put_mnt. > > This is done by implementing an asynchronous version of kern_unmount. > > > > Since mntput() sleeps, it needs to be deferred to a work queue. > > > > Additionally, the callers of mq_put_mnt appear to be safe having > > it behave asynchronously. In particular, put_ipc_ns calls > > mq_clear_sbinfo which renders the inode inaccessible for the purposes of > > mqueue_create by making s_fs_info NULL. This appears > > to be the thing that prevents access while free_ipc_ns is taking place. > > So, the unmount should be able to proceed lazily. > > Ugh... I really doubt that it's correct. The caller is > mq_put_mnt(ns); > free_ipc_ns(ns); > and we have > static void mqueue_evict_inode(struct inode *inode) > { > > ... > > ipc_ns = get_ns_from_inode(inode); > > with > > static struct ipc_namespace *get_ns_from_inode(struct inode *inode) > { > struct ipc_namespace *ns; > > spin_lock(&mq_lock); > ns = __get_ns_from_inode(inode); > spin_unlock(&mq_lock); > return ns; > } > > and > > static inline struct ipc_namespace *__get_ns_from_inode(struct inode *inode) > { > return get_ipc_ns(inode->i_sb->s_fs_info); > } > > with ->s_fs_info being the ipc_namespace we are freeing after mq_put_ns() > > Are you saying that get_ipc_ns() after free_ipc_ns() is safe? Because > ->evict_inode() *IS* called on umount. What happens to your patch if > there was a regular file left on that filesystem? > > Smells like a memory corruptor... Actually, the full context in the caller is if (refcount_dec_and_lock(&ns->count, &mq_lock)) { mq_clear_sbinfo(ns); spin_unlock(&mq_lock); mq_put_mnt(ns); free_ipc_ns(ns); } And void mq_clear_sbinfo(struct ipc_namespace *ns) { ns->mq_mnt->mnt_sb->s_fs_info = NULL; } Therefore, s_fs_info should be NULL before we proceed to unmount. So, as far as I know, it should not be possible to find the ipc_namespace from the mount.