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 12F44D637A8 for ; Wed, 13 Nov 2024 19:11:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A218A6B008A; Wed, 13 Nov 2024 14:11:58 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9D1426B008C; Wed, 13 Nov 2024 14:11:58 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8C0416B0093; Wed, 13 Nov 2024 14:11:58 -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 70E126B008A for ; Wed, 13 Nov 2024 14:11:58 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 24E27C032C for ; Wed, 13 Nov 2024 19:11:58 +0000 (UTC) X-FDA: 82782015330.10.10D2C84 Received: from mail-ed1-f43.google.com (mail-ed1-f43.google.com [209.85.208.43]) by imf13.hostedemail.com (Postfix) with ESMTP id B7F9820024 for ; Wed, 13 Nov 2024 19:11:11 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=TxZA5cEU; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of amir73il@gmail.com designates 209.85.208.43 as permitted sender) smtp.mailfrom=amir73il@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1731525053; a=rsa-sha256; cv=none; b=72Hjt0au9qh7qew6j31knBOon3Fg/933DT5EnaiRLG58NF+Ew/ZJFL9ywl/2ojFC3s/QxD idgCWzwTs20DMeywIpy/AsnoK2dOrT1BAYB3T9KkOZ9amVPbZrXx9ZOhat08m/3/KwjibP k/LBNfAWnIZX886chUe/PL/4Kk5+Mwc= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=TxZA5cEU; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of amir73il@gmail.com designates 209.85.208.43 as permitted sender) smtp.mailfrom=amir73il@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1731525053; 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=5Q+8FNc2ytDg7RieFT9goBSoHoyxH/PoPvWuBdU9IS4=; b=wwPbXNj2lF0rtKZeZimWuiyjo/fv9+Pij9fmA+7RrM8TckEM68P+FcbCqQsrAIsA1IExS5 i4TlMrNH4sk4YrwgXPEwxqdH+/l/tXLZAOx8D7vhLoILbbvnNwV4oPpV47fGaobDOcwILp CuPV07nqY4Tn3BMHMbxVg9eiOtDRB7E= Received: by mail-ed1-f43.google.com with SMTP id 4fb4d7f45d1cf-5cefc36c5d4so9684491a12.0 for ; Wed, 13 Nov 2024 11:11:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731525115; x=1732129915; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=5Q+8FNc2ytDg7RieFT9goBSoHoyxH/PoPvWuBdU9IS4=; b=TxZA5cEUk1L/abawcY/60l7PM4hHNOYlpdgAN/ZFqjkoIoTbmhAiV+ZWokwhQOD+0G LQJjp+vPRaAPkC/ib1H1cnD3gTns98ePjt7gnaS2RHxUPBXpVoaFQwTNZ90wqr9xSO9b rLWJkLZEnyLN4+J2IlTzXQbpji3iWQHwSo4M+pSKobc+kkxL4ZF0ZSTWqz4UPvmWXC/h a1228ol3R92kNx4tmnzunPfkFlk310RBD63AAnV/RrkiWOIZMaAavaQxMjLPh6OamQC/ qIWeKFAOFmtSGDBsrmvWX5inW+ov3/fWeOnFwryHs4Gyu7pyQmoH1O+bH+NTd9kWSBns mHKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731525115; x=1732129915; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5Q+8FNc2ytDg7RieFT9goBSoHoyxH/PoPvWuBdU9IS4=; b=Uh85AqnG8lTd88YiDf8QTpESTilwGgBRI6qQzv5jJHJY0SAw4Bc8/njwOnC4TlYGIs g+OclVxbRoTdZSCo54QFx/zmszJSlRS5dmZkPpIOjtRdk28hOeehoM5j8GVSxFSBanMc JZznoBR7QdY31AH3fgNfitT/+ciQmnKJi6NenGyWXRFtwonD1EdNZrTQjankFad9ey56 dfotojSxsf1yE+4FCHm5XR/UarDYyzBqJ0HhWqq7s1FNGchhU7RL+ZzV5NVgJwJdg0yf TYYE3WRazT3THw9o/Iov3ML8h0le2RtMH4g2Dw2AIvjxR68Wp2AsetzkY1N3roFo2B3e QvFw== X-Forwarded-Encrypted: i=1; AJvYcCU9YgHes7OjNIGK+QzDTlOOKPaM2bjvAc1mZHHsKRVbMFeN+9lls3AGmieoYWjdbg74erTIuN78qg==@kvack.org X-Gm-Message-State: AOJu0YwwEab/I55X1XcWrRCek3VYRqkJBnNQzMceAvvWXBwGWsqdLBWa g02jgylWMlhlwqcWN5wqqQMwNrHuM0iVvXM8w3FTt1rhkPyqKNbgLrvqaAMGl9inCP3RUCCWiFd ntdSuDP0WuXPislqUdMdJUviTTHQ= X-Google-Smtp-Source: AGHT+IFjcv0P8IIDO97PcCN5S7SxQV9ik2hLtub35kKA7vznAH6u+jV8u+K37Im4T2XozTvcOCmVqkexlWfmeUKgKM4= X-Received: by 2002:a17:907:3f8a:b0:a9e:c954:6afb with SMTP id a640c23a62f3a-a9ef0018b25mr2166360666b.51.1731525113311; Wed, 13 Nov 2024 11:11:53 -0800 (PST) MIME-Version: 1.0 References: <141e2cc2dfac8b2f49c1c8d219dd7c20925b2cef.1731433903.git.josef@toxicpanda.com> In-Reply-To: From: Amir Goldstein Date: Wed, 13 Nov 2024 20:11:39 +0100 Message-ID: Subject: Re: [PATCH v7 05/18] fsnotify: introduce pre-content permission events To: Linus Torvalds Cc: Josef Bacik , kernel-team@fb.com, linux-fsdevel@vger.kernel.org, jack@suse.cz, brauner@kernel.org, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-mm@kvack.org, linux-ext4@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Queue-Id: B7F9820024 X-Rspamd-Server: rspam11 X-Stat-Signature: wwtwt4ie664o64p9u7per7hhjfswdm19 X-HE-Tag: 1731525071-640171 X-HE-Meta: U2FsdGVkX1+STn+GWqv1fF5DrHtaPDK6oBdRWSzZxfBnMcpViNJTklZU4Bj4wdiL/+xCUxQr+U4L6Kt99bEBVZyRhAdunzEmfNA67k73u9+YjtcxRwM5CAuVATLtqgD3EdqUXbOwpVdVprbiHSEcbw7bIiRMGQOaqilicqougunCbk0Be3EsLsr5t+9mg4WDeoo7nvNQ+EFySDngri/4GpZG7IN4lmskym5yS1WWCp7Hz9XMXfyFO9MO+UVQ+XYNV1oD5shfTRtfPyWzj+DiukOWRlIle0jr/Yygp9ZkiJgEBbBtSpmLvA2qkBrxRlPPTQT/8N8yYkgCc2W+uSZ97kXZxK8TmYL6gIRyyw372btYqPXFgBk/aR3SL0t6Cgxy4H70Z31D44/noaxnBFA7PBbVaX4I60tJOJo/F8Xk6vL673RGIMvWtN5dcQJdveWve/wmNs7Yj+4QjWJrOOGCaVJkjgTIHTx/lExSoqt1tMNJtClq/Byq26V24GtGzrzNLla/3aq+7b3caIclvQGoIal8vi7FBi1XxnJcMJBnG6GUdqmnHHotcuftjd4v+j+oNVVFEF9KI2csYDow4nYsoTk/s3JdmBcOiXOJGso8kTFmiFGRgSJSZdophW9sn1HVyvREjBx7UCsj+zflvkmz933A0Cp3xNDoSxpozB7GTSai5VO58/GGX25vY6VeLsQXw22nLNzoFDoDnj3EWXNCwU+OTCZwHeUzcNVCC/JSih/SjibOdo016sEq6ysMoQA9NKSOMvolXzhU4b0vF/hyzIwsmlw2eao8zVqwh0DGIC7A56MSmmDNVAStwDpiMDmEZglI1yETDrY9QWMA0iDIaD8nwau6GqHi0fQRaOZksd/m2Ce2VZFpgnhn/gDbyEQ/8so437BVcvIEY3+UXmJRFAvqzc9OO0FcX76LqV8qCqs0eTxRkcPKwYAdunI/ms+f9rZZ0GI4SmsW0bW/xOn FFbyL5xh NqV6AYf3i9K12R7bOA75LOIadCgrF7ge58eNSe+VtrHo1YsgVQ2lZpeGey0zNVY1uKaWGPKHox7f8L1+q2wZxOJ9e1BOPu4RxD/4IpRKWIKTAHWx1YgN0k97+of8nV1UxIrPcFH66HChYFbi+q+2/uzhgSGX1f1yjYgca5Il98YFmlRPOV0YjCRaNxR0VVyhxLpL5aWpwDtnLF/ac394xsWB5cGfJV3LYnSzIwpKw4iY1ZJAqDSmYYWfBsqfYpHGEdOYC02UAMyRO4v96aTez4SGdurq0KZSuKgf2QcdRqgbD+gEc2nM75aC6ppYHsV52Kgp6n/UgHxdEAlDAXzWIb+zhEUP2yfJYsMwy/OwCjaf6watD2EM/ySHc0JrKaD3nl1yL8sjrdWQsfZFrtougqKH24Cgf1gKWEUQcJvl3pfYNN/CBamCMr0durWciSEwUKKvm 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 Tue, Nov 12, 2024 at 9:12=E2=80=AFPM Linus Torvalds wrote: > > On Tue, 12 Nov 2024 at 09:56, Josef Bacik wrote: > > > > #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS > > +static inline int fsnotify_pre_content(struct file *file) > > +{ > > + struct inode *inode =3D file_inode(file); > > + > > + /* > > + * Pre-content events are only reported for regular files and d= irs > > + * if there are any pre-content event watchers on this sb. > > + */ > > + if ((!S_ISDIR(inode->i_mode) && !S_ISREG(inode->i_mode)) || > > + !(inode->i_sb->s_iflags & SB_I_ALLOW_HSM) || > > + !fsnotify_sb_has_priority_watchers(inode->i_sb, > > + FSNOTIFY_PRIO_PRE_CONTEN= T)) > > + return 0; > > + > > + return fsnotify_file(file, FS_PRE_ACCESS); > > +} > > Yeah, no. > > None of this should check inode->i_sb->s_iflags at any point. > > The "is there a pre-content" thing should check one thing, and one > thing only: that "is this file watched" flag. > > The whole indecipherable mess of inline functions that do random > things in needs to be cleaned up, not made even > more indecipherable. > > I'm NAKing this whole series until this is all sane and cleaned up, > and I don't want to see a new hacky version being sent out tomorrow > with just another layer of new hacks, with random new inline functions > that call other inline functions and have complex odd conditionals > that make no sense. > > Really. If the new hooks don't have that *SINGLE* bit test, they will > not get merged. > > And that *SINGLE* bit test had better not be hidden under multiple > layers of odd inline functions. > > You DO NOT get to use the same old broken complex function for the new > hooks that then mix these odd helpers. Up to here I understand. > > This whole "add another crazy inline function using another crazy > helper needs to STOP. Later on in the patch series you do > The patch that I sent did add another convenience helper fsnotify_path(), but as long as it is not hiding crazy tests, and does not expand to huge inlined code, I don't see the problem. Those convenience helpers help me to maintain readability and code reuse. I do agree that the new hooks that can use the new open-time check semantics should not expand to huge inlined code. > +/* > + * fsnotify_truncate_perm - permission hook before file truncate > + */ > +static inline int fsnotify_truncate_perm(const struct path *path, > loff_t length) > +{ > + return fsnotify_pre_content(path, &length, 0); > +} > This example that you pointed at, I do not understand. truncate() does not happen on an open file, so I cannot use the FMODE_NONOTIFY_ test. This is what I have in my WIP branch: static inline int fsnotify_file_range(const struct path *path, const loff_t *ppos, size_t count) { struct file_range range; const void *data; int data_type; /* Report page aligned range only when pos is known */ if (ppos) { range.path =3D path; range.pos =3D PAGE_ALIGN_DOWN(*ppos); range.count =3D PAGE_ALIGN(*ppos + count) - range.pos; data =3D ⦥ data_type =3D FSNOTIFY_EVENT_FILE_RANGE; } else { data =3D path; data_type =3D FSNOTIFY_EVENT_PATH; } return fsnotify_parent(path->dentry, FS_PRE_ACCESS, data, data_type= ); } /* * fsnotify_truncate_perm - permission hook before file truncate */ static inline int fsnotify_truncate_perm(const struct path *path, loff_t le= ngth) { struct inode *inode =3D d_inode(path->dentry); /* * Pre-content events are only reported for regular files and dirs * if there are any pre-content event watchers on this sb. */ if ((!S_ISDIR(inode->i_mode) && !S_ISREG(inode->i_mode)) || !(inode->i_sb->s_iflags & SB_I_ALLOW_HSM) || !unlikely(fsnotify_sb_has_priority_watchers(inode->i_sb, FSNOTIFY_PRIO_PRE_CONTENT))) return 0; return fsnotify_file_range(path, &length, 0); } fsnotify_file_range() does not need to be inlined, but I do want to reuse the code of fsnotify_file_range() which is also called for the common file pre-access hook. So did you mean that the unlikely stuff (i.e. fsnotify_file_range()) should be an indirect call? or something else? Thanks, Amir.