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=-2.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, USER_AGENT_MUTT 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 07B10C10F0E for ; Mon, 15 Apr 2019 10:42:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C09D020684 for ; Mon, 15 Apr 2019 10:42:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=kroah.com header.i=@kroah.com header.b="pPUnhXMM"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="DatHPXc3" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726476AbfDOKmd (ORCPT ); Mon, 15 Apr 2019 06:42:33 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:52177 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726214AbfDOKmd (ORCPT ); Mon, 15 Apr 2019 06:42:33 -0400 Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id D166B260E1; Mon, 15 Apr 2019 06:42:31 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Mon, 15 Apr 2019 06:42:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kroah.com; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm3; bh=rCoFLmZiI6c42FKh3hvT3veDVht KBBQ6T/QTFykk4XY=; b=pPUnhXMMNSkuemN9PNqctZ7qNr8eUoMPB56z+xB7Cyb RfjgfcRZNKVZpbhBm+AdM8P9eLYQDojL/+IpzWoq5csG272szmImOhSX7c3u9qiC lLrRrEo6mMJJ7gHg+SMsqQDe81D3b2suw1Lln6UpU2auARbh31xBBMguDw3qmaVJ piOs8Nmb6hsIHW3r3jlcU0VEp3f/C1ME5zpVaYSHP0FToQLlB9fndWx+UreTkIDf F8lTfINuDSWNUnBcjcDzD2u2UZFoj5D0urezgDuLh3vdKEc0rFvbTdPBPTI5DDet 1CnW1brnRD6syw1YWrePtJwy/D1QczNiM7+YOC79WyQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=rCoFLm ZiI6c42FKh3hvT3veDVhtKBBQ6T/QTFykk4XY=; b=DatHPXc3kGV31P6HZ368xy TyADxNjLapU6pd4/xQB1rgV/Zu1GWq+RKInFNshJEdjvcuH+MJc1CfvSEtlKbgM1 mSisUjrliPizBIWCBXW/9anYg4KKpjISkhkqvBIy7Nb5OAjFg+LYimXAbr/Vi9vi qdGvFu5rMxjfgTmPZfL+lWSCFIF2B4ZjJEU+5nOcQQ93ttUDSoX8H9XOBMyjZtuq c4wQqCHXn6NYMMC8hgdmrPDjBBd70CC5BidJW9IFKOAVZ3sUU9X34wd4MdGmsRKN VnLTxFrz9kgZHXcaKcyi8NHJlOEVcs1w5+VGaWUR5qcA9K+f22xz1RTMy0kMWAig == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrvdelgdefudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvuffkfhggtggujggfsehttdertddtredvnecuhfhrohhmpefirhgvghcu mffjuceoghhrvghgsehkrhhorghhrdgtohhmqeenucffohhmrghinhepkhgvrhhnvghlrd horhhgnecukfhppeekfedrkeeirdekledruddtjeenucfrrghrrghmpehmrghilhhfrhho mhepghhrvghgsehkrhhorghhrdgtohhmnecuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) by mail.messagingengine.com (Postfix) with ESMTPA id 8430EE4448; Mon, 15 Apr 2019 06:42:30 -0400 (EDT) Date: Mon, 15 Apr 2019 12:42:28 +0200 From: Greg KH To: Jan Kara Cc: Sasha Levin , stable-commits@vger.kernel.org, stable@vger.kernel.org, Matthew Ruffell Subject: Re: Patch "fanotify: Release SRCU lock when waiting for userspace response" has been added to the 4.9-stable tree Message-ID: <20190415104228.GA9397@kroah.com> References: <20190411152628.8E2682173C@mail.kernel.org> <20190412084412.GB29850@quack2.suse.cz> <20190412144344.GR11568@sasha-vm> <20190415090825.GA3031@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190415090825.GA3031@quack2.suse.cz> User-Agent: Mutt/1.11.4 (2019-03-13) Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org On Mon, Apr 15, 2019 at 11:08:25AM +0200, Jan Kara wrote: > On Fri 12-04-19 10:43:44, Sasha Levin wrote: > > On Fri, Apr 12, 2019 at 10:44:12AM +0200, Jan Kara wrote: > > > On Thu 11-04-19 11:26:27, Sasha Levin wrote: > > > > This is a note to let you know that I've just added the patch titled > > > > > > > > fanotify: Release SRCU lock when waiting for userspace response > > > > > > > > to the 4.9-stable tree which can be found at: > > > > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary > > > > > > > > The filename of the patch is: > > > > fanotify-release-srcu-lock-when-waiting-for-userspac.patch > > > > and it can be found in the queue-4.9 subdirectory. > > > > > > > > If you, or anyone else, feels it should not be added to the stable tree, > > > > please let know about it. > > > > > > I'd be careful with this series. You're missing at least the fixup series > > > from Miklos culminating in f37650f1c7c7 "fanotify: fix > > > fsnotify_prepare_user_wait() failure". And you seem to be missing also > > > quite some prerequisites reworking lifetime of fsnotify marks (series > > > culminating with f09b04a03e0 "fsnotify: Remove special handling of mark > > > destruction on group shutdown"). So you're just introducing subtle > > > use-after-free issues to fanotify code. Overall I think the chances for > > > regressions here are much bigger than the problem you'll be fixing unless > > > you'll go for something like wholesale update of fs/notify/* to state in > > > f37650f1c7c7. > > > > I've pulled this series based on the request here: > > https://lore.kernel.org/stable/20190411032430.17353-1-matthew.ruffell@canonical.com/ > > Thanks for reference! I've added Matthew into CC so that he's aware of the > potential problems with the patches they backported. > > > There are a few reasons why I'd prefer to keep it in: > > > > 1. It fixes a very real bug which has affected quite a few of our > > customers as well, so (at least for me) this is more than a minor > > bugfix. > > I have my reservations about it being a kernel bug :) Primarily, it is a > problem in userspace not responding to fanotify permission events properly. > With these patches, the misbehaving application will take down just the > filesystem it is working on (processes blocked in D state), without them it > will take down the whole machine. So sure the patches improve the situation > but more often than not you have to reboot anyway. > > And yes, it is bad that misbehaving userspace can take the kernel down > rather easily but that's the problem with how fanotify permission events > API has been designed and generally with the idea of AV vendors that they > need to intercept and approve all writes / opens with their AV solution. > > > 2. It came with a straightforward testcase. > > > > 3. Given that Canonical pulled it in as well, it (hopefully) received > > more testing than some other random patches. > > Sure, seeing the reference I don't blame you that you've included the > patches. > > > If there are missing patches here I'd be happy to take them in and > > re-test the kernel, but I'd really like to avoid *not* taking these > > patches just because we fear a regression but can't show it. > > So the three patches as you took them are definitely wrong because they > introduce use after free issues. Ubuntu guys have backported the part that > takes care of dropping SRCU when waiting for response but didn't backport > the part that makes sure fanotify marks, inodes, and mounts stay allocated > while we are waiting. This could be even exploitable as attacker can force > inode eviction via rm(1). So please don't include them as they are into > -stable. > > Matthew, if you really want to backport the patches changing how fanotify > uses SRCU (and honestly I'm not convinced you have to since without fixing > the AV applications the system will not work good anyway), you have to also > backport the series 5198adf649a0 "fsnotify: Remove unnecessary tests when > showing fdinfo" .. f09b04a03e0 "fsnotify: Remove special handling of mark > destruction on group shutdown") - yes, it is big and it completely reworks > lifetime of notification marks and their inode / mount references in the > kernel. And then as a cherry on top you also need to backport followup > fixes 24c20305c7f "fsnotify: clean up fsnotify_prepare/finish_user_wait()" > .. f37650f1c7c7 "fanotify: fix fsnotify_prepare_user_wait() failure". And > as a warning these are only the prerequisites I'm aware of. Given the > amount of patches, I might have easily forgotten about something. Thank you for the detailed review and for catching this. I've now dropped both of these series from the 4.4.y and 4.9.y trees. Matthew, if you can fix these up properly, feel free to resend them. thanks, greg k-h