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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8F01CD75BDF for ; Thu, 21 Nov 2024 09:39:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1BC3D6B0082; Thu, 21 Nov 2024 04:39:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 145356B0088; Thu, 21 Nov 2024 04:39:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ED9FB6B0089; Thu, 21 Nov 2024 04:39:23 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id CBC256B0082 for ; Thu, 21 Nov 2024 04:39:23 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 856411A0AE0 for ; Thu, 21 Nov 2024 09:39:23 +0000 (UTC) X-FDA: 82809603114.18.1A34A4A Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf03.hostedemail.com (Postfix) with ESMTP id 0C27020011 for ; Thu, 21 Nov 2024 09:38:56 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=fyborDxb; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="ngieK/es"; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=fyborDxb; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="ngieK/es"; spf=pass (imf03.hostedemail.com: domain of jack@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=jack@suse.cz; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1732181757; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=rN7zACpXhWBvmyhdQ9L7AJcccIoJA6vlCdi8480LQC0=; b=t10/ZpqGDfUxSV/RqgEbHwJ/E13q8GcEFsy7h7RHMMd1xk+j0+u6hHKpPMQzHDMg1sO7++ aqETiGo2QSVeUhM1qLCVoJSf4SOxIM9kEu7C7flFtjiwIY1s4pwkl5R4kTQhxYHW4el9Gg iGY0iH7TtfB4+jYLe7stoISTSlSUGtQ= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=fyborDxb; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="ngieK/es"; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=fyborDxb; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="ngieK/es"; spf=pass (imf03.hostedemail.com: domain of jack@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=jack@suse.cz; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732181757; a=rsa-sha256; cv=none; b=7BP4vjv4gyP7Tpj+/QRpLwpKxXIIZLz9mUpUpANBd4MRncL9o/8DUQWa5SCI0zfiEazGsN /giX7xNkzMgpIRP0ptDOLxMjybedYj+0oDExwQlsiVBNOE1dJxZR1bIievIMd1GlsTWQDT muGn24HMa+E4181HV5Z9IBer7Qhbcuk= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 4190F2124D; Thu, 21 Nov 2024 09:39:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1732181959; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rN7zACpXhWBvmyhdQ9L7AJcccIoJA6vlCdi8480LQC0=; b=fyborDxbGV5tOJEsgZOCdPTpEXPwLs66tKq/7XlYBvT+lfOhORmFAM9Gp0L9Rcfc07ZGtO M3ahEd0zTbnol02LyixbnSz+aEeUU74bdiQBgK9SZ6opo5+v6Cbk7y9PHaY2Yyv7k/zygS r/+fmMO6YcmaB0BZ4nVfbL81omS9NKw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1732181959; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rN7zACpXhWBvmyhdQ9L7AJcccIoJA6vlCdi8480LQC0=; b=ngieK/es157uCVvN4o0UKFZk5Uky2BzuJoAKbgaLqLG+1DDJlEU6UyIHIFwC8RN8yjRAAK K1QEBo+WuebSeABg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1732181959; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rN7zACpXhWBvmyhdQ9L7AJcccIoJA6vlCdi8480LQC0=; b=fyborDxbGV5tOJEsgZOCdPTpEXPwLs66tKq/7XlYBvT+lfOhORmFAM9Gp0L9Rcfc07ZGtO M3ahEd0zTbnol02LyixbnSz+aEeUU74bdiQBgK9SZ6opo5+v6Cbk7y9PHaY2Yyv7k/zygS r/+fmMO6YcmaB0BZ4nVfbL81omS9NKw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1732181959; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rN7zACpXhWBvmyhdQ9L7AJcccIoJA6vlCdi8480LQC0=; b=ngieK/es157uCVvN4o0UKFZk5Uky2BzuJoAKbgaLqLG+1DDJlEU6UyIHIFwC8RN8yjRAAK K1QEBo+WuebSeABg== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 2B6D213927; Thu, 21 Nov 2024 09:39:19 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id hrlKCsf/PmcJXAAAD6G6ig (envelope-from ); Thu, 21 Nov 2024 09:39:19 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id C30E5A089E; Thu, 21 Nov 2024 10:39:18 +0100 (CET) Date: Thu, 21 Nov 2024 10:39:18 +0100 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , Josef Bacik , kernel-team@fb.com, linux-fsdevel@vger.kernel.org, brauner@kernel.org, torvalds@linux-foundation.org, viro@zeniv.linux.org.uk, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-mm@kvack.org, linux-ext4@vger.kernel.org Subject: Re: [PATCH v8 02/19] fsnotify: opt-in for permission events at file open time Message-ID: <20241121093918.d2ml5lrfcqwknffb@quack3> References: <5ea5f8e283d1edb55aa79c35187bfe344056af14.1731684329.git.josef@toxicpanda.com> <20241120155309.lecjqqhohgcgyrkf@quack3> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Action: no action X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 0C27020011 X-Stat-Signature: fon5x3e85xbfjs5ejbwe5a716dkhs6jg X-Rspam-User: X-HE-Tag: 1732181936-722473 X-HE-Meta: U2FsdGVkX1//FyCs/3j5g/5s0qhVC8MvWpe38Ht0DjsDToY3DFDfupEAloBf6I0fnFXJOhG+AfzHYUUhiztZnU9m9noYhn0UkVmWuaaFwM93uIaaUABruyhOPl6HLb0JQKU7uKMOxJMz+PTERNWRITwcRGhz0ZtO7jWacdFotRZgE9+szb7dceFy2Bez3kKD7QfhnONVwIYJpM0NYA9VRvPR3zEMcaL818HJ7ew9PlcG+yt7+2kRBU17rNLENn2dEznEJ22kKSf4kVfa4iS0m+bu6d0J8I2jpiSWUjrAcEtg29VAnplOTGHDjAyUwVFbNNRYslfkTPWLNAJMHd0YD5hxXSvdwSzYi+VA7AAbtev0OaXRwIFtaIB3rY87SOxYpNaqiN+NyhaqScUL4ljyuaBZ0Een3qq3FobE7TommmLFpxZyjuyl9ore6DyWZpZ418X7QqVv8cJUKSFGDyb4cXRD4lHldoelYi39lRR99xgnnfhHV2d1ZU0bmV6NBU4rTiM2EmpKO3ukCAdmghBZ4ff5m5rM8eI9yRpN5sjRwZxnl5RuY8LpvAMN0s3hWfwkbiXl4PqJ/m+jjty3ErRUxudFlRQ2oSSZ3mwQvCYmV0NMlhI06ZSTeioIZFdG5fAU8yHRSoVcyNDoAgDXx6b7qLsfidaX7chstsbfZ/nbQzXGJ0uSyarSHeLNt9aiTqJt6vz9oYbTjKEhcnwHxB9F0DI4UtC9PdERRyiIsSC7llqa2j5JASldW8GzHG9ZkGSRBP3eDIEUDr1PRo004+zmMZ1ni6eDyED5sQ7+7Qhs+PxAKOPp9AW2lpupNSJt7GPYHZxskZ+e9+QxuAQIyjjcpZSL/rXydn+EZplIlODGIryuHTyvbRDUnCdXmworxXcxcOKosycB9J4i2dG8x4fJ23b/H/b1NkFAcxEsEfZtLwGqLIiGY8BA3jn4+0OsnV/ljxjBlOHdFDcmLb9wyCY 6eM6vYiR cMcrgin7yotrBcWTUmou5sOkrlP0/U2Vw4Rcz4dYFbwHzBF3uzBBGHk25Zal+TLkC++F76xDCAkJl+knjLVLhMZGIeKA2aylcoI4GbC7zJvxGFCcgmYPAUlUnDtJViInVOzEWwsmmYUUez5HiXNZP00N5E+Csk+xuEvTxUNC+nXMRd3zPdYJSkqKfnRSS/JBEv8kNMr+Ea6fEmjK2n7NxN6UxrwaWjuU/TI2mI7XhSra1foPkKOZHNV/I0IL/R8nHxls9LmuRNwCiFHorHp59qKnPfcaml8LjTBm/igdHk7GbZesPrMg2lfjXLhrTT11czonJmY5fFfV8leftqbIt9vg5/k4ecXLwIyUhFE7e8Em9LBV8YUoQXxx1Ul4biKzAuJZwJZboGmAsg9qWq2IifAJj8nMzsxNkB5uidBkSiIgdxS39jXzYkuFd48qa7iKIHQIDTXpixhtLvUr+Ut1O+UQCWoMzhCSFJqNI 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: List-Subscribe: List-Unsubscribe: On Wed 20-11-24 17:12:21, Amir Goldstein wrote: > On Wed, Nov 20, 2024 at 4:53 PM Jan Kara wrote: > > > > On Fri 15-11-24 10:30:15, Josef Bacik wrote: > > > From: Amir Goldstein > > > > > > Legacy inotify/fanotify listeners can add watches for events on inode, > > > parent or mount and expect to get events (e.g. FS_MODIFY) on files that > > > were already open at the time of setting up the watches. > > > > > > fanotify permission events are typically used by Anti-malware sofware, > > > that is watching the entire mount and it is not common to have more that > > > one Anti-malware engine installed on a system. > > > > > > To reduce the overhead of the fsnotify_file_perm() hooks on every file > > > access, relax the semantics of the legacy FAN_ACCESS_PERM event to generate > > > events only if there were *any* permission event listeners on the > > > filesystem at the time that the file was opened. > > > > > > The new semantic is implemented by extending the FMODE_NONOTIFY bit into > > > two FMODE_NONOTIFY_* bits, that are used to store a mode for which of the > > > events types to report. > > > > > > This is going to apply to the new fanotify pre-content events in order > > > to reduce the cost of the new pre-content event vfs hooks. > > > > > > Suggested-by: Linus Torvalds > > > Link: https://lore.kernel.org/linux-fsdevel/CAHk-=wj8L=mtcRTi=NECHMGfZQgXOp_uix1YVh04fEmrKaMnXA@mail.gmail.com/ > > > Signed-off-by: Amir Goldstein > > > > FWIW I've ended up somewhat massaging this patch (see below). > > > > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > > index 23bd058576b1..8e5c783013d2 100644 > > > --- a/include/linux/fs.h > > > +++ b/include/linux/fs.h > > > @@ -173,13 +173,14 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset, > > > > > > #define FMODE_NOREUSE ((__force fmode_t)(1 << 23)) > > > > > > -/* FMODE_* bit 24 */ > > > - > > > /* File is embedded in backing_file object */ > > > -#define FMODE_BACKING ((__force fmode_t)(1 << 25)) > > > +#define FMODE_BACKING ((__force fmode_t)(1 << 24)) > > > > > > -/* File was opened by fanotify and shouldn't generate fanotify events */ > > > -#define FMODE_NONOTIFY ((__force fmode_t)(1 << 26)) > > > +/* File shouldn't generate fanotify pre-content events */ > > > +#define FMODE_NONOTIFY_HSM ((__force fmode_t)(1 << 25)) > > > + > > > +/* File shouldn't generate fanotify permission events */ > > > +#define FMODE_NONOTIFY_PERM ((__force fmode_t)(1 << 26)) > > > > Firstly, I've kept FMODE_NONOTIFY to stay a single bit instead of two bit > > constant. I've seen too many bugs caused by people expecting the constant > > has a single bit set when it actually had more in my life. So I've ended up > > with: > > > > +/* > > + * Together with FMODE_NONOTIFY_PERM defines which fsnotify events shouldn't be > > + * generated (see below) > > + */ > > +#define FMODE_NONOTIFY ((__force fmode_t)(1 << 25)) > > + > > +/* > > + * Together with FMODE_NONOTIFY defines which fsnotify events shouldn't be > > + * generated (see below) > > + */ > > +#define FMODE_NONOTIFY_PERM ((__force fmode_t)(1 << 26)) > > > > and > > > > +/* > > + * The two FMODE_NONOTIFY* define which fsnotify events should not be generated > > + * for a file. These are the possible values of (f->f_mode & > > + * FMODE_FSNOTIFY_MASK) and their meaning: > > + * > > + * FMODE_NONOTIFY - suppress all (incl. non-permission) events. > > + * FMODE_NONOTIFY_PERM - suppress permission (incl. pre-content) events. > > + * FMODE_NONOTIFY | FMODE_NONOTIFY_PERM - suppress only pre-content events. > > + */ > > +#define FMODE_FSNOTIFY_MASK \ > > + (FMODE_NONOTIFY | FMODE_NONOTIFY_PERM) > > + > > +#define FMODE_FSNOTIFY_NONE(mode) \ > > + ((mode & FMODE_FSNOTIFY_MASK) == FMODE_NONOTIFY) > > +#define FMODE_FSNOTIFY_PERM(mode) \ > > + (!(mode & FMODE_NONOTIFY_PERM)) > > That looks incorrect - > It gives the wrong value for FMODE_NONOTIFY | FMODE_NONOTIFY_PERM > > should be: > != FMODE_NONOTIFY_PERM && > != FMODE_NONOTIFY > > The simplicity of the single bit test is for permission events > is why I chose my model, but I understand your reasoning. Ah, thanks for catching this! I've fixed this to: +#define FMODE_FSNOTIFY_PERM(mode) \ + ((mode & FMODE_FSNOTIFY_MASK) == 0 || \ + (mode & FMODE_FSNOTIFY_MASK) == (FMODE_NONOTIFY | FMODE_NONOTIFY_PERM)) It is not a single bit test so it ends up being: 0x0000000060180345 <+101>: mov 0x20(%r12),%edx 0x000000006018034a <+106>: and $0x6000000,%edx 0x0000000060180350 <+112>: je 0x6018035a 0x0000000060180352 <+114>: cmp $0x6000000,%edx 0x0000000060180358 <+120>: jne 0x6018032e But I guess that's not terrible either. > > +#define FMODE_FSNOTIFY_HSM(mode) \ > > + ((mode & FMODE_FSNOTIFY_MASK) == 0) > > > > Also I've moved file_set_fsnotify_mode() out of line into fsnotify.c. The > > function gets quite big and the call is not IMO so expensive to warrant > > inlining. Furthermore it saves exporting some fsnotify internals to modules > > (in later patches). > > Sounds good. > Since you wanted to refrain from defining a two bit constant, > I wonder how you annotated for NONOTIFY_HSM case > > return FMODE_NONOTIFY | FMODE_NONOTIFY_PERM; I'm not sure I understand. What do you mean by "annotated"? It is not that I object to "two bit constants". FMODE_FSNOTIFY_MASK is a two-bit constant and a good one. But the name clearly suggests it is not a single bit constant. When you have all FMODE_FOO and FMODE_BAR things single bit except for FMODE_BAZ which is multi-bit, then this is IMHO a recipe for problems and I rather prefer explicitely spelling the combination out as FMODE_NONOTIFY | FMODE_NONOTIFY_PERM in the few places that need this instead of hiding it behind some other name. Honza -- Jan Kara SUSE Labs, CR