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 CD35BD6ACC3 for ; Wed, 27 Nov 2024 12:21:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2CEBB6B0082; Wed, 27 Nov 2024 07:21:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 27EE76B0085; Wed, 27 Nov 2024 07:21:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 146C66B0092; Wed, 27 Nov 2024 07:21:09 -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 EC7A06B0082 for ; Wed, 27 Nov 2024 07:21:08 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id A39C2AD4F5 for ; Wed, 27 Nov 2024 12:21:08 +0000 (UTC) X-FDA: 82831784448.10.EF79CBE Received: from mail-ed1-f48.google.com (mail-ed1-f48.google.com [209.85.208.48]) by imf27.hostedemail.com (Postfix) with ESMTP id F0EC34000D for ; Wed, 27 Nov 2024 12:20:57 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=IPLdHAln; spf=pass (imf27.hostedemail.com: domain of amir73il@gmail.com designates 209.85.208.48 as permitted sender) smtp.mailfrom=amir73il@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1732710061; 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=g4AJxcgbGXlGttAV3Sp9Te5JfEMnLRstmnBehcMGYgI=; b=7+BNtgtkkKVjPGMgED/HjqxTHlE+nhYjwK4zI2Dht6kgkU7JlPm56coEC2tEYYoUevkYIC Bd8B66HZKEy6oYl6ksbNvcXjMNQB/8GAMflLvwHnzEakLaqHfTNc/PUbQWFVVox5oCaxNK dG4h0gqhr/YyCNDSu+93oJhs+HgOJfA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732710061; a=rsa-sha256; cv=none; b=rFpUVn/+kleg+WMtoTdOm5xcccNG3hG57qKUcTbrUrWJ64+2NJ+7bMG6ne+D2WLYDTlfaz 3sXVk5kvDSk/zCOLSx/ENuUZjcZavQM6GC/DnDqPrqOicXkao8TgpZ5vo8Hwr3P1J1tJQA RHgqUmLTcczTKumKJI9cePVqjhtwzko= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=IPLdHAln; spf=pass (imf27.hostedemail.com: domain of amir73il@gmail.com designates 209.85.208.48 as permitted sender) smtp.mailfrom=amir73il@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-ed1-f48.google.com with SMTP id 4fb4d7f45d1cf-5cfd2978f95so9329842a12.0 for ; Wed, 27 Nov 2024 04:21:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732710065; x=1733314865; 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=g4AJxcgbGXlGttAV3Sp9Te5JfEMnLRstmnBehcMGYgI=; b=IPLdHAlnEdw9v2lcKQG7fC/ZNpgXsb37uCZEopbCUKd+4mgRf/t+D2IHaymnIRzis1 zNQjpbACoDDuUdc2lGM/NY8VoPAlzLayuM0bNEDFxleqGmY+1NclxmjooIFrb8vxrXz+ MVn2W7dxw7OhStDg4GQ7OfrVjno8HMzaWchDnjioO39zP8ivUN28vd/ZJKxEbNNf3dEV JUNv62o2IYEy8BrM0a1vUL3qvHOROiQRsuvfE+OKq++zdXXO7EF/y/7Ou6tryQzKZzBK PxgKsvj6xHu1IN1opDYgBSx7DhO3j+B+IzdGd3TndzeKKXaY9UmrtaHPcuEzSMXBaBiJ iLhw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732710065; x=1733314865; 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=g4AJxcgbGXlGttAV3Sp9Te5JfEMnLRstmnBehcMGYgI=; b=rbm2DfU8fCRNxawKxAp0KdAV8q50VyRMoeP+x0bCFAsUrpIr28/rl/2iKKCsIoCTyi Z0ZMo3WN2YuPX7pd3gitDm0fAE8UOVjP0xGiNAnbZtBs03MljmSMpr62PtPDc5IJ5IC9 u4A6pZ4sEAYtTd6brV495idOZZLpMtu9/O3EuKhgBkKGlNOwfXNTGX0xdISTLsRGe5X2 0tou/N4KxT4cnEdsXP93dOmX749LLb1r4g6Gu1ZIBThRJoiL9aqaDEFG8Edz/oKwCTbe ra1ZeYFQkwSZsZKcjP4UQfuH1eDK9BjsjDZTVQRmilk24mDAWAvMBR4fU5VKxjClOFVy 4mDw== X-Forwarded-Encrypted: i=1; AJvYcCUecz6L8chD7XeVJaQBz//V8F5JV/FsTWPzJw2GLv4Z3iGzKCVSnpscO7Gioq5V/Z186jomT2Ge7Q==@kvack.org X-Gm-Message-State: AOJu0YyyAXl3qmrXFnN2IvtadJvtbY36cgsw1Tjf67FzJHKp+sawEY6y GNBl7PXzvtc5tw7zEVHRY7ZJB+MS+t4h28+Njyu5VYh7O8LpnmVfJnCKdjK8SYkMR/Gir1Vhy8X V5cpdkQlD920XN1mFDRSwvPGdv8A= X-Gm-Gg: ASbGncvw5rSqrGh6vHbfC/F6XLMAeZ9Gdo0ifvimXxRbaIfM86fk2+vAs5sakHmoqzm z7XxwN83ru7+CADOS6KAKpPSkJVOX15k= X-Google-Smtp-Source: AGHT+IFZo9o4G9xu1yI2+N8EIMu+e3DL435vyXxWc0NrJTLb0aMbN7osMheigvrNa3XxJ9bTAmpltIRm5/4kmYOjn4g= X-Received: by 2002:a17:906:315b:b0:aa5:3853:553e with SMTP id a640c23a62f3a-aa58103dbb5mr264470966b.47.1732710064815; Wed, 27 Nov 2024 04:21:04 -0800 (PST) MIME-Version: 1.0 References: <20241121104428.wtlrfhadcvipkjia@quack3> <20241121163618.ubz7zplrnh66aajw@quack3> <20241122124215.3k3udv5o6eys6ffy@quack3> <20241127121838.3fmhjx26cfxcegro@quack3> In-Reply-To: <20241127121838.3fmhjx26cfxcegro@quack3> From: Amir Goldstein Date: Wed, 27 Nov 2024 13:20:52 +0100 Message-ID: Subject: Re: [PATCH v8 10/19] fanotify: introduce FAN_PRE_ACCESS permission event To: Jan Kara Cc: 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: F0EC34000D X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: g6wokwjxgutpts9aqgwm5zcn9bq5fuzc X-HE-Tag: 1732710057-827570 X-HE-Meta: U2FsdGVkX18crpVILTtFvTUDPiCKTsoChibsEV1Cvs738YvbgFHB78CzfPyVapfvP4TOW605Xxcv3/BPb1YyrbdWw6qooEOYGZfd8Wyl8+cnfl5zD09uT1E/0jK92udgbAPTXWCebjD2fj4slOzTyaxtb6rV3oBNXsgKEIcfURqq0uPMArxTiJpjP64wLBevR8URtwfF3aCPw5cBKnOEhTnZKqHC7bMdgOVRK3r2XlBHN6yW44W++mOF99uI/lG3/53jO9yLKvknxkjfkmtoYJCFpH1xHyW+81L2Nq86PBlapg/gEKOhvBI0TEjZaW9i1xzhZtWGNfRSibIt3QELjofdt0k64xLFxpzmHR2I7aPiAho88fihi7MLF7knth2Fe+ohwmobT8a2mtxHspcn862n7dlk+cLZxMI+kLy/mUkqndim/taCAFjWpGitPs128kwjgC+KQV08ofpXP385AK+02/57iLAbq8peGey1JZpL+acbCxoZuLHGK3tK4gapHi0YpiL1hJwptZP8qmEV6LAPWp8dGt+zQ5zRyQGHVL0lhaEK+xpWsYAKXOPWPti/YsIsovdjH0XPKBcDQXmSDqCd9EDTvhUmkNmLRDIjwkLK7Sq/IdhfYuZ4PDmenfG1X/HrVNvhddIs07P/pf5S+V8eom8hSZCv2ga7FLQ9ltqQCojBrz6RFPmikX4LG+Glv7l/l4OE5oaGdPADliZHOoXYtbZNNv/d3Zue915S3CNEF2k148a5Ypbi5KEFCmkDvfAMZ4Wdt4Y22kQRP7QMjV/+qIsVZYWzNvAhSIwaRu+QrrVTtREILqKdEkLmwbekAdEFVljOghk8r5EXmdR3o6G6wugNOfThRr8uLgbacIwk0R7vn0kEaq9j0IafL7uqvcCAZS99mbzTWEnhw0ZaeCVF+7O3uTVEuOvJnezcSwT/uxIc9YBuZKdmszwvAJDfm7Aauul/20KHV8Ffqfh TLkEB+/Y w+77899TrJvgKDRvoUIcUeYe+9AmteXJEQdvUmdf0yYoLRriTVV8InWZzZrT94OhoUxk3DCyIY6Ell+LTpy4ojsdpr1UBShvineKHd0WmpeKMIJ6xvKArTwx5CPFUH9BBpFAT4wCNhajm1X2qIuw1NrIwhHqXKopdiHQ2rXpHTUjq6voFOL3qYW3mBzjKt0vrzVL/ABh+NbU+fAFBybwNWdgF6/8dv6lmIGBCM97tITrCZ1DfnlWIfkg920nW59gLv6XsobVDuwckMPrdFdVcRwJlQaURqmFzE7S9w/NmRiwkB9rZFbOK5fO+wSrkG0ZaxSpR10xZZM9pVHCzOF2AIS+ucvOKK19F6GU4dU3dp/6skRNsGtmtxTa3h7sEhQQdwWVQ 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, Nov 27, 2024 at 1:18=E2=80=AFPM Jan Kara wrote: > > On Fri 22-11-24 14:51:23, Amir Goldstein wrote: > > On Fri, Nov 22, 2024 at 1:42=E2=80=AFPM Jan Kara wrote: > > > > > > On Thu 21-11-24 19:37:43, Amir Goldstein wrote: > > > > On Thu, Nov 21, 2024 at 7:31=E2=80=AFPM Amir Goldstein wrote: > > > > > On Thu, Nov 21, 2024 at 5:36=E2=80=AFPM Jan Kara w= rote: > > > > > > On Thu 21-11-24 15:18:36, Amir Goldstein wrote: > > > > > > > On Thu, Nov 21, 2024 at 11:44=E2=80=AFAM Jan Kara wrote: > > > > > > > and also always emitted ACCESS_PERM. > > > > > > > > > > > > I know that and it's one of those mostly useless events AFAICT. > > > > > > > > > > > > > my POC is using that PRE_ACCESS to populate > > > > > > > directories on-demand, although the functionality is incomple= te without the > > > > > > > "populate on lookup" event. > > > > > > > > > > > > Exactly. Without "populate on lookup" doing "populate on readdi= r" is ok for > > > > > > a demo but not really usable in practice because you can get sp= urious > > > > > > ENOENT from a lookup. > > > > > > > > > > > > > > avoid the mistake of original fanotify which had some event= s available on > > > > > > > > directories but they did nothing and then you have to ponde= r hard whether > > > > > > > > you're going to break userspace if you actually start emitt= ing them... > > > > > > > > > > > > > > But in any case, the FAN_ONDIR built-in filter is applicable = to PRE_ACCESS. > > > > > > > > > > > > Well, I'm not so concerned about filtering out uninteresting ev= ents. I'm > > > > > > more concerned about emitting the event now and figuring out la= ter that we > > > > > > need to emit it in different places or with some other info whe= n actual > > > > > > production users appear. > > > > > > > > > > > > But I've realized we must allow pre-content marks to be placed = on dirs so > > > > > > that such marks can be placed on parents watching children. Wha= t we'd need > > > > > > to forbid is a combination of FAN_ONDIR and FAN_PRE_ACCESS, wou= ldn't we? > > > > > > > > > > Yes, I think that can work well for now. > > > > > > > > > > > > > Only it does not require only check at API time that both flags are= not > > > > set, because FAN_ONDIR can be set earlier and then FAN_PRE_ACCESS > > > > can be added later and vice versa, so need to do this in > > > > fanotify_may_update_existing_mark() AFAICT. > > > > > > I have now something like: > > > > > > @@ -1356,7 +1356,7 @@ static int fanotify_group_init_error_pool(struc= t fsnotify_group *group) > > > } > > > > > > static int fanotify_may_update_existing_mark(struct fsnotify_mark *f= sn_mark, > > > - unsigned int fan_flags) > > > + __u32 mask, unsigned int= fan_flags) > > > { > > > /* > > > * Non evictable mark cannot be downgraded to evictable mark. > > > @@ -1383,6 +1383,11 @@ static int fanotify_may_update_existing_mark(s= truct fsnotify_mark *fsn_mark, > > > fsn_mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY) > > > return -EEXIST; > > > > > > + /* For now pre-content events are not generated for directori= es */ > > > + mask |=3D fsn_mark->mask; > > > + if (mask & FANOTIFY_PRE_CONTENT_EVENTS && mask & FAN_ONDIR) > > > + return -EEXIST; > > > + > > > > EEXIST is going to be confusing if there was never any mark. > > Either return -EINVAL here or also check this condition on the added ma= sk > > itself before calling fanotify_add_mark() and return -EINVAL there. > > > > I prefer two distinct errors, but probably one is also good enough. > > That's actually a good point. My previous change allowed setting > FAN_PRE_ACCESS | FAN_ONDIR on a new mark because that doesn't get to > fanotify_may_update_existing_mark(). So I now have: > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fano= tify_user.c > index 0919ea735f4a..38a46865408e 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -1356,7 +1356,7 @@ static int fanotify_group_init_error_pool(struct fs= notify_group *group) > } > > static int fanotify_may_update_existing_mark(struct fsnotify_mark *fsn_m= ark, > - unsigned int fan_flags) > + __u32 mask, unsigned int fan= _flags) > { > /* > * Non evictable mark cannot be downgraded to evictable mark. > @@ -1383,6 +1383,11 @@ static int fanotify_may_update_existing_mark(struc= t fsnotify_mark *fsn_mark, > fsn_mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY) > return -EEXIST; > > + /* For now pre-content events are not generated for directories *= / > + mask |=3D fsn_mark->mask; > + if (mask & FANOTIFY_PRE_CONTENT_EVENTS && mask & FAN_ONDIR) > + return -EEXIST; > + > return 0; > } > > @@ -1409,7 +1414,7 @@ static int fanotify_add_mark(struct fsnotify_group = *group, > /* > * Check if requested mark flags conflict with an existing mark f= lags. > */ > - ret =3D fanotify_may_update_existing_mark(fsn_mark, fan_flags); > + ret =3D fanotify_may_update_existing_mark(fsn_mark, mask, fan_fla= gs); > if (ret) > goto out; > > @@ -1905,6 +1910,10 @@ static int do_fanotify_mark(int fanotify_fd, unsig= ned int flags, __u64 mask, > if (mask & FAN_RENAME && !(fid_mode & FAN_REPORT_NAME)) > goto fput_and_out; > > + /* Pre-content events are not currently generated for directories= . */ > + if (mask & FANOTIFY_PRE_CONTENT_EVENTS && mask & FAN_ONDIR) > + goto fput_and_out; > + > if (mark_cmd =3D=3D FAN_MARK_FLUSH) { > ret =3D 0; > if (mark_type =3D=3D FAN_MARK_MOUNT) > -- > 2.35.3 > Looks good. Thanks, Amir.