[PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[Ajay Kaher]

> [ Upstream commit 9709bd548f11a092d124698118013f66e1740f9b ]
> 
> Wire up the FAN_FS_ERROR event in the fanotify_mark syscall, allowing
> user space to request the monitoring of FAN_FS_ERROR events.
> 
> These events are limited to filesystem marks, so check it is the
> case in the syscall handler.

Greg,

Without 9709bd548f11 in v5.10.y skips LTP fanotify22 test case, as: 
fanotify22.c:312: TCONF: FAN_FS_ERROR not supported in kernel

With 9709bd548f11 in v5.10.220, LTP fanotify22 is failing because of
timeout as no notification. To fix need to merge following two upstream
commit to v5.10:

124e7c61deb27d758df5ec0521c36cf08d417f7a:
0001-ext4_fix_error_code_saved_on_super_block_during_file_system.patch
https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#mf76930487697d8c1383ed5d21678fe504e8e2305

9a089b21f79b47eed240d4da7ea0d049de7c9b4d:
0001-ext4_Send_notifications_on_error.patch
Link: https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#md1be98e0ecafe4f92d7b61c048e15bcf286cbd53 

-Ajay

Re: [PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[Amir Goldstein]

On Tue, Jul 23, 2024 at 10:06 AM Ajay Kaher <ajay.kaher@broadcom.com> wrote:
>
> > [ Upstream commit 9709bd548f11a092d124698118013f66e1740f9b ]
> >
> > Wire up the FAN_FS_ERROR event in the fanotify_mark syscall, allowing
> > user space to request the monitoring of FAN_FS_ERROR events.
> >
> > These events are limited to filesystem marks, so check it is the
> > case in the syscall handler.
>
> Greg,
>
> Without 9709bd548f11 in v5.10.y skips LTP fanotify22 test case, as:
> fanotify22.c:312: TCONF: FAN_FS_ERROR not supported in kernel
>
> With 9709bd548f11 in v5.10.220, LTP fanotify22 is failing because of
> timeout as no notification. To fix need to merge following two upstream
> commit to v5.10:
>
> 124e7c61deb27d758df5ec0521c36cf08d417f7a:
> 0001-ext4_fix_error_code_saved_on_super_block_during_file_system.patch
> https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#mf76930487697d8c1383ed5d21678fe504e8e2305
>
> 9a089b21f79b47eed240d4da7ea0d049de7c9b4d:
> 0001-ext4_Send_notifications_on_error.patch
> Link: https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#md1be98e0ecafe4f92d7b61c048e15bcf286cbd53
>
> -Ajay

I agree that this is the best approach, because the test has no other
way to test
if ext4 specifically supports FAN_FS_ERROR.

Chuck,

I wonder how those patches end up in 5.15.y but not in 5.10.y?

Also, since you backported *most* of this series:
https://lore.kernel.org/all/20211025192746.66445-1-krisman@collabora.com/

I think it would be wise to also backport the sample code and documentation
patches to 5.15.y and 5.10.y.

9abeae5d4458 docs: Fix formatting of literal sections in fanotify docs
8fc70b3a142f samples: Make fs-monitor depend on libc and headers
c0baf9ac0b05 docs: Document the FAN_FS_ERROR event
5451093081db samples: Add fs error monitoring example.

Gabriel, if 9abeae5d4458 has a Fixes: tag it may have been auto seleced
for 5.15.y after c0baf9ac0b05 was picked up...

Thanks,
Amir.

Re: [PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[Jan Kara]

On Tue 23-07-24 12:36:27, Ajay Kaher wrote:
> > [ Upstream commit 9709bd548f11a092d124698118013f66e1740f9b ]
> > 
> > Wire up the FAN_FS_ERROR event in the fanotify_mark syscall, allowing
> > user space to request the monitoring of FAN_FS_ERROR events.
> > 
> > These events are limited to filesystem marks, so check it is the
> > case in the syscall handler.
> 
> Greg,
> 
> Without 9709bd548f11 in v5.10.y skips LTP fanotify22 test case, as: 
> fanotify22.c:312: TCONF: FAN_FS_ERROR not supported in kernel
> 
> With 9709bd548f11 in v5.10.220, LTP fanotify22 is failing because of
> timeout as no notification. To fix need to merge following two upstream
> commit to v5.10:
> 
> 124e7c61deb27d758df5ec0521c36cf08d417f7a:
> 0001-ext4_fix_error_code_saved_on_super_block_during_file_system.patch
> https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#mf76930487697d8c1383ed5d21678fe504e8e2305
> 
> 9a089b21f79b47eed240d4da7ea0d049de7c9b4d:
> 0001-ext4_Send_notifications_on_error.patch
> Link: https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#md1be98e0ecafe4f92d7b61c048e15bcf286cbd53 

I know Chuck has been backporting the huge pile of fsnotify changes for
stable and he was running LTP so I'm a bit curious if he saw the fanotify22
failure as well. The reason for the test failure seems to be that the
combination of features now present in stable has never been upstream which
confuses the test. As such I'm not sure if backporting more features to
stable is warranted just to fix a broken LTP test... But given the huge
pile Chuck has backported already I'm not strongly opposed to backporting a
few more, there's just a question where does this stop :)

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[Amir Goldstein]

On Tue, Jul 23, 2024 at 12:29 PM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 23-07-24 12:36:27, Ajay Kaher wrote:
> > > [ Upstream commit 9709bd548f11a092d124698118013f66e1740f9b ]
> > >
> > > Wire up the FAN_FS_ERROR event in the fanotify_mark syscall, allowing
> > > user space to request the monitoring of FAN_FS_ERROR events.
> > >
> > > These events are limited to filesystem marks, so check it is the
> > > case in the syscall handler.
> >
> > Greg,
> >
> > Without 9709bd548f11 in v5.10.y skips LTP fanotify22 test case, as:
> > fanotify22.c:312: TCONF: FAN_FS_ERROR not supported in kernel
> >
> > With 9709bd548f11 in v5.10.220, LTP fanotify22 is failing because of
> > timeout as no notification. To fix need to merge following two upstream
> > commit to v5.10:
> >
> > 124e7c61deb27d758df5ec0521c36cf08d417f7a:
> > 0001-ext4_fix_error_code_saved_on_super_block_during_file_system.patch
> > https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#mf76930487697d8c1383ed5d21678fe504e8e2305
> >
> > 9a089b21f79b47eed240d4da7ea0d049de7c9b4d:
> > 0001-ext4_Send_notifications_on_error.patch
> > Link: https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#md1be98e0ecafe4f92d7b61c048e15bcf286cbd53
>
> I know Chuck has been backporting the huge pile of fsnotify changes for
> stable and he was running LTP so I'm a bit curious if he saw the fanotify22
> failure as well. The reason for the test failure seems to be that the
> combination of features now present in stable has never been upstream which
> confuses the test. As such I'm not sure if backporting more features to
> stable is warranted just to fix a broken LTP test... But given the huge
> pile Chuck has backported already I'm not strongly opposed to backporting a
> few more, there's just a question where does this stop :)

I'm not sure is it exactly "more features".
The fanotify patches and ext4 patches that use them where merges as
a feature together.

IOW, FAN_FS_ERROR was merged with support for a single fs (ext4)
it would be weird to backport the feature with support for zero fs.
Also, 5.15.y already has the ext4 patches - not sure why 5.10.y didn't get them.

Thanks,
Amir.

Re: [PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[Jan Kara]

On Tue 23-07-24 13:13:41, Amir Goldstein wrote:
> On Tue, Jul 23, 2024 at 12:29 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Tue 23-07-24 12:36:27, Ajay Kaher wrote:
> > > > [ Upstream commit 9709bd548f11a092d124698118013f66e1740f9b ]
> > > >
> > > > Wire up the FAN_FS_ERROR event in the fanotify_mark syscall, allowing
> > > > user space to request the monitoring of FAN_FS_ERROR events.
> > > >
> > > > These events are limited to filesystem marks, so check it is the
> > > > case in the syscall handler.
> > >
> > > Greg,
> > >
> > > Without 9709bd548f11 in v5.10.y skips LTP fanotify22 test case, as:
> > > fanotify22.c:312: TCONF: FAN_FS_ERROR not supported in kernel
> > >
> > > With 9709bd548f11 in v5.10.220, LTP fanotify22 is failing because of
> > > timeout as no notification. To fix need to merge following two upstream
> > > commit to v5.10:
> > >
> > > 124e7c61deb27d758df5ec0521c36cf08d417f7a:
> > > 0001-ext4_fix_error_code_saved_on_super_block_during_file_system.patch
> > > https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#mf76930487697d8c1383ed5d21678fe504e8e2305
> > >
> > > 9a089b21f79b47eed240d4da7ea0d049de7c9b4d:
> > > 0001-ext4_Send_notifications_on_error.patch
> > > Link: https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#md1be98e0ecafe4f92d7b61c048e15bcf286cbd53
> >
> > I know Chuck has been backporting the huge pile of fsnotify changes for
> > stable and he was running LTP so I'm a bit curious if he saw the fanotify22
> > failure as well. The reason for the test failure seems to be that the
> > combination of features now present in stable has never been upstream which
> > confuses the test. As such I'm not sure if backporting more features to
> > stable is warranted just to fix a broken LTP test... But given the huge
> > pile Chuck has backported already I'm not strongly opposed to backporting a
> > few more, there's just a question where does this stop :)
> 
> I'm not sure is it exactly "more features".  The fanotify patches and
> ext4 patches that use them where merges as a feature together.
> 
> IOW, FAN_FS_ERROR was merged with support for a single fs (ext4)
> it would be weird to backport the feature with support for zero fs.
> Also, 5.15.y already has the ext4 patches - not sure why 5.10.y didn't get them.

I forgot 5.15.y got all the patches. Ok, then pushing them to 5.10.y makes
sense as well.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[Chuck Lever III]

> On Jul 23, 2024, at 5:29 AM, Jan Kara <jack@suse.cz> wrote:
> 
> On Tue 23-07-24 12:36:27, Ajay Kaher wrote:
>>> [ Upstream commit 9709bd548f11a092d124698118013f66e1740f9b ]
>>> 
>>> Wire up the FAN_FS_ERROR event in the fanotify_mark syscall, allowing
>>> user space to request the monitoring of FAN_FS_ERROR events.
>>> 
>>> These events are limited to filesystem marks, so check it is the
>>> case in the syscall handler.
>> 
>> Greg,
>> 
>> Without 9709bd548f11 in v5.10.y skips LTP fanotify22 test case, as: 
>> fanotify22.c:312: TCONF: FAN_FS_ERROR not supported in kernel
>> 
>> With 9709bd548f11 in v5.10.220, LTP fanotify22 is failing because of
>> timeout as no notification. To fix need to merge following two upstream
>> commit to v5.10:
>> 
>> 124e7c61deb27d758df5ec0521c36cf08d417f7a:
>> 0001-ext4_fix_error_code_saved_on_super_block_during_file_system.patch
>> https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#mf76930487697d8c1383ed5d21678fe504e8e2305
>> 
>> 9a089b21f79b47eed240d4da7ea0d049de7c9b4d:
>> 0001-ext4_Send_notifications_on_error.patch
>> Link: https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#md1be98e0ecafe4f92d7b61c048e15bcf286cbd53 
> 
> I know Chuck has been backporting the huge pile of fsnotify changes for
> stable and he was running LTP so I'm a bit curious if he saw the fanotify22
> failure as well.

Fwiw, I didn't see any new failures for either 5.10.y or 5.15.y.


> The reason for the test failure seems to be that the
> combination of features now present in stable has never been upstream which
> confuses the test. As such I'm not sure if backporting more features to
> stable is warranted just to fix a broken LTP test... But given the huge
> pile Chuck has backported already I'm not strongly opposed to backporting a
> few more, there's just a question where does this stop :)
> 
> Honza
> 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR


--
Chuck Lever

Re: [PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[Chuck Lever III]

> On Jul 23, 2024, at 5:20 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> 
> On Tue, Jul 23, 2024 at 10:06 AM Ajay Kaher <ajay.kaher@broadcom.com> wrote:
>> 
>>> [ Upstream commit 9709bd548f11a092d124698118013f66e1740f9b ]
>>> 
>>> Wire up the FAN_FS_ERROR event in the fanotify_mark syscall, allowing
>>> user space to request the monitoring of FAN_FS_ERROR events.
>>> 
>>> These events are limited to filesystem marks, so check it is the
>>> case in the syscall handler.
>> 
>> Greg,
>> 
>> Without 9709bd548f11 in v5.10.y skips LTP fanotify22 test case, as:
>> fanotify22.c:312: TCONF: FAN_FS_ERROR not supported in kernel
>> 
>> With 9709bd548f11 in v5.10.220, LTP fanotify22 is failing because of
>> timeout as no notification. To fix need to merge following two upstream
>> commit to v5.10:
>> 
>> 124e7c61deb27d758df5ec0521c36cf08d417f7a:
>> 0001-ext4_fix_error_code_saved_on_super_block_during_file_system.patch
>> https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#mf76930487697d8c1383ed5d21678fe504e8e2305
>> 
>> 9a089b21f79b47eed240d4da7ea0d049de7c9b4d:
>> 0001-ext4_Send_notifications_on_error.patch
>> Link: https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#md1be98e0ecafe4f92d7b61c048e15bcf286cbd53
>> 
>> -Ajay
> 
> I agree that this is the best approach, because the test has no other
> way to test
> if ext4 specifically supports FAN_FS_ERROR.
> 
> Chuck,
> 
> I wonder how those patches end up in 5.15.y but not in 5.10.y?

The process was by hand. I tried to copy the same
commit ID set from 5.15 to 5.10, but some patches
did not apply to 5.10, or may have been omitted
unintentionally.


> Also, since you backported *most* of this series:
> https://lore.kernel.org/all/20211025192746.66445-1-krisman@collabora.com/
> 
> I think it would be wise to also backport the sample code and documentation
> patches to 5.15.y and 5.10.y.
> 
> 9abeae5d4458 docs: Fix formatting of literal sections in fanotify docs
> 8fc70b3a142f samples: Make fs-monitor depend on libc and headers
> c0baf9ac0b05 docs: Document the FAN_FS_ERROR event
> 5451093081db samples: Add fs error monitoring example.
> 
> Gabriel, if 9abeae5d4458 has a Fixes: tag it may have been auto seleced
> for 5.15.y after c0baf9ac0b05 was picked up...

Or this... I might not have backported those at all
to 5.15, and they got pulled in by another process.

In any event, do you want me to backport these to
5.10, or would you like to do it yourself?


--
Chuck Lever

Re: [PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[Gabriel Krisman Bertazi]

Amir Goldstein <amir73il@gmail.com> writes:

> On Tue, Jul 23, 2024 at 10:06 AM Ajay Kaher <ajay.kaher@broadcom.com> wrote:
>> Without 9709bd548f11 in v5.10.y skips LTP fanotify22 test case, as:
>> fanotify22.c:312: TCONF: FAN_FS_ERROR not supported in kernel
>>
>> With 9709bd548f11 in v5.10.220, LTP fanotify22 is failing because of
>> timeout as no notification. To fix need to merge following two upstream
>> commit to v5.10:
>>
>> 124e7c61deb27d758df5ec0521c36cf08d417f7a:
>> 0001-ext4_fix_error_code_saved_on_super_block_during_file_system.patch
>> https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#mf76930487697d8c1383ed5d21678fe504e8e2305
>>
>> 9a089b21f79b47eed240d4da7ea0d049de7c9b4d:
>> 0001-ext4_Send_notifications_on_error.patch
>> Link: https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#md1be98e0ecafe4f92d7b61c048e15bcf286cbd53
>>
>> -Ajay
>
> I agree that this is the best approach, because the test has no other
> way to test
> if ext4 specifically supports FAN_FS_ERROR.
>
> Chuck,
>
> I wonder how those patches end up in 5.15.y but not in 5.10.y?

I wonder why this was backported to stable in the first place.  I get
there is a lot of refactoring in this series, which might be useful when
backporting further fixes. but 9709bd548f11 just enabled a new feature -
which seems against stable rules.  Considering that "anything is a CVE",
we really need to be cautious about this kind of stuff in stable
kernels.

Is it possible to drop 9709bd548f11 from stable instead?

> Gabriel, if 9abeae5d4458 has a Fixes: tag it may have been auto seleced
> for 5.15.y after c0baf9ac0b05 was picked up...

right.  It would be really cool if we had a way to append this
information after the fact.  How would people feel about using
git-notes in the kernel tree to support that?

-- 
Gabriel Krisman Bertazi

Re: [PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[Chuck Lever III]

> On Jul 23, 2024, at 10:34 AM, Gabriel Krisman Bertazi <gabriel@krisman.be> wrote:
> 
> Amir Goldstein <amir73il@gmail.com> writes:
> 
>> On Tue, Jul 23, 2024 at 10:06 AM Ajay Kaher <ajay.kaher@broadcom.com> wrote:
>>> Without 9709bd548f11 in v5.10.y skips LTP fanotify22 test case, as:
>>> fanotify22.c:312: TCONF: FAN_FS_ERROR not supported in kernel
>>> 
>>> With 9709bd548f11 in v5.10.220, LTP fanotify22 is failing because of
>>> timeout as no notification. To fix need to merge following two upstream
>>> commit to v5.10:
>>> 
>>> 124e7c61deb27d758df5ec0521c36cf08d417f7a:
>>> 0001-ext4_fix_error_code_saved_on_super_block_during_file_system.patch
>>> https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#mf76930487697d8c1383ed5d21678fe504e8e2305
>>> 
>>> 9a089b21f79b47eed240d4da7ea0d049de7c9b4d:
>>> 0001-ext4_Send_notifications_on_error.patch
>>> Link: https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#md1be98e0ecafe4f92d7b61c048e15bcf286cbd53
>>> 
>>> -Ajay
>> 
>> I agree that this is the best approach, because the test has no other
>> way to test
>> if ext4 specifically supports FAN_FS_ERROR.
>> 
>> Chuck,
>> 
>> I wonder how those patches end up in 5.15.y but not in 5.10.y?
> 
> I wonder why this was backported to stable in the first place.

I wanted to fix the myriad problems with the NFSD file cache
in v5.10.y and v5.15.y. These problems were addressed by a
long list of changes from v5.19 to v6.3. I was told that it
was preferred that I do a full backport of all NFSD commits
from v6.3 to the older kernels.

The fanotify patches were dependencies.


> I get
> there is a lot of refactoring in this series, which might be useful when
> backporting further fixes. but 9709bd548f11 just enabled a new feature -
> which seems against stable rules.  Considering that "anything is a CVE",
> we really need to be cautious about this kind of stuff in stable
> kernels.

These patches were posted for review before they were
included. Amir noted there were some changes to fanotify
that would require man page updates and some modifications
to ltp, but there were no other comments.


> Is it possible to drop 9709bd548f11 from stable instead?

I've attempted a revert. It's not clean, but seems to be
fixable by hand.

I can run some tests to see if that breaks NFSD.


>> Gabriel, if 9abeae5d4458 has a Fixes: tag it may have been auto seleced
>> for 5.15.y after c0baf9ac0b05 was picked up...
> 
> right.  It would be really cool if we had a way to append this
> information after the fact.  How would people feel about using
> git-notes in the kernel tree to support that?
> 
> -- 
> Gabriel Krisman Bertazi

--
Chuck Lever

[PATCH v5.15.y] Revert "fanotify: Allow users to request FAN_FS_ERROR events"

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

Gabriel says:
> 9709bd548f11 just enabled a new feature -
> which seems against stable rules.  Considering that "anything is
> a CVE", we really need to be cautious about this kind of stuff in
> stable kernels.
>
> Is it possible to drop 9709bd548f11 from stable instead?

The revert wasn't clean, but adjusting it to fit was straightforward.
This passes NFSD CI, and adds no new failures to the fanotify ltp
tests.

Reported-by: Gabriel Krisman Bertazi <gabriel@krisman.be>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/notify/fanotify/fanotify_user.c | 4 ----
 include/linux/fanotify.h           | 6 +-----
 2 files changed, 1 insertion(+), 9 deletions(-)

Gabriel, is this what you were thinking?


diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index d93418f21386..0d91db1c7249 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1701,10 +1701,6 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 	    group->priority == FS_PRIO_0)
 		goto fput_and_out;
 
-	if (mask & FAN_FS_ERROR &&
-	    mark_type != FAN_MARK_FILESYSTEM)
-		goto fput_and_out;
-
 	/*
 	 * Evictable is only relevant for inode marks, because only inode object
 	 * can be evicted on memory pressure.
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index 558844c8d259..df60b46971c9 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -97,13 +97,9 @@ extern struct ctl_table fanotify_table[]; /* for sysctl */
 #define FANOTIFY_INODE_EVENTS	(FANOTIFY_DIRENT_EVENTS | \
 				 FAN_ATTRIB | FAN_MOVE_SELF | FAN_DELETE_SELF)
 
-/* Events that can only be reported with data type FSNOTIFY_EVENT_ERROR */
-#define FANOTIFY_ERROR_EVENTS	(FAN_FS_ERROR)
-
 /* Events that user can request to be notified on */
 #define FANOTIFY_EVENTS		(FANOTIFY_PATH_EVENTS | \
-				 FANOTIFY_INODE_EVENTS | \
-				 FANOTIFY_ERROR_EVENTS)
+				 FANOTIFY_INODE_EVENTS)
 
 /* Events that require a permission response from user */
 #define FANOTIFY_PERM_EVENTS	(FAN_OPEN_PERM | FAN_ACCESS_PERM | \
-- 
2.45.2

Re: [PATCH v5.15.y] Revert "fanotify: Allow users to request FAN_FS_ERROR events"

[Gabriel Krisman Bertazi]

cel@kernel.org writes:

> From: Chuck Lever <chuck.lever@oracle.com>
>
> Gabriel says:
>> 9709bd548f11 just enabled a new feature -
>> which seems against stable rules.  Considering that "anything is
>> a CVE", we really need to be cautious about this kind of stuff in
>> stable kernels.
>>
>> Is it possible to drop 9709bd548f11 from stable instead?
>
> The revert wasn't clean, but adjusting it to fit was straightforward.
> This passes NFSD CI, and adds no new failures to the fanotify ltp
> tests.
>
> Reported-by: Gabriel Krisman Bertazi <gabriel@krisman.be>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/notify/fanotify/fanotify_user.c | 4 ----
>  include/linux/fanotify.h           | 6 +-----
>  2 files changed, 1 insertion(+), 9 deletions(-)
>
> Gabriel, is this what you were thinking?

Thanks Chuck.

This looks good to me as a way to disable it in stable and prevent
userspace from trying to use it. Up to fanotify maintainers to decide
whether to brign the rest of the series or merge this, but either way:

Reviewed-by: Gabriel Krisman Bertazi <gabriel@krisman.be>


>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index d93418f21386..0d91db1c7249 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1701,10 +1701,6 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>  	    group->priority == FS_PRIO_0)
>  		goto fput_and_out;
>  
> -	if (mask & FAN_FS_ERROR &&
> -	    mark_type != FAN_MARK_FILESYSTEM)
> -		goto fput_and_out;
> -
>  	/*
>  	 * Evictable is only relevant for inode marks, because only inode object
>  	 * can be evicted on memory pressure.
> diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> index 558844c8d259..df60b46971c9 100644
> --- a/include/linux/fanotify.h
> +++ b/include/linux/fanotify.h
> @@ -97,13 +97,9 @@ extern struct ctl_table fanotify_table[]; /* for sysctl */
>  #define FANOTIFY_INODE_EVENTS	(FANOTIFY_DIRENT_EVENTS | \
>  				 FAN_ATTRIB | FAN_MOVE_SELF | FAN_DELETE_SELF)
>  
> -/* Events that can only be reported with data type FSNOTIFY_EVENT_ERROR */
> -#define FANOTIFY_ERROR_EVENTS	(FAN_FS_ERROR)
> -
>  /* Events that user can request to be notified on */
>  #define FANOTIFY_EVENTS		(FANOTIFY_PATH_EVENTS | \
> -				 FANOTIFY_INODE_EVENTS | \
> -				 FANOTIFY_ERROR_EVENTS)
> +				 FANOTIFY_INODE_EVENTS)
>  
>  /* Events that require a permission response from user */
>  #define FANOTIFY_PERM_EVENTS	(FAN_OPEN_PERM | FAN_ACCESS_PERM | \

-- 
Gabriel Krisman Bertazi

Re: [PATCH v5.15.y] Revert "fanotify: Allow users to request FAN_FS_ERROR events"

[Amir Goldstein]

On Wed, Jul 24, 2024 at 2:24 AM Gabriel Krisman Bertazi
<gabriel@krisman.be> wrote:
>
> cel@kernel.org writes:
>
> > From: Chuck Lever <chuck.lever@oracle.com>
> >
> > Gabriel says:
> >> 9709bd548f11 just enabled a new feature -
> >> which seems against stable rules.  Considering that "anything is
> >> a CVE", we really need to be cautious about this kind of stuff in
> >> stable kernels.
> >>
> >> Is it possible to drop 9709bd548f11 from stable instead?
> >
> > The revert wasn't clean, but adjusting it to fit was straightforward.
> > This passes NFSD CI, and adds no new failures to the fanotify ltp
> > tests.
> >
> > Reported-by: Gabriel Krisman Bertazi <gabriel@krisman.be>
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> >  fs/notify/fanotify/fanotify_user.c | 4 ----
> >  include/linux/fanotify.h           | 6 +-----
> >  2 files changed, 1 insertion(+), 9 deletions(-)
> >
> > Gabriel, is this what you were thinking?
>
> Thanks Chuck.
>
> This looks good to me as a way to disable it in stable and prevent
> userspace from trying to use it. Up to fanotify maintainers to decide
> whether to bring the rest of the series or merge this,

First of all, the "rest of the series" is already queued for 5.10.y.

I too was a bit surprised from willingness of stable tree maintainers
to allow backports of entire features along with Chuck's nfsd backports.
I understand the logic, I was just not aware when this shift in stable ideology
had happened.

But having backported the entire fanotify 6.1 code as is to 5.15 and 5.10
I see no reason to make an exception for FAN_FS_ERROR.
Certainly not because two patches were left out of 5.10.y (and are now queued).

I think that the benefits of fanotify code parity across 6.1 5.15 5.10
outweigh the risk of regressions introduced by this specific feature.

So please do not revert.

Thanks,
Amir.

Re: [PATCH 5.10 387/770] fanotify: Allow users to request FAN_FS_ERROR events

[Amir Goldstein]

On Tue, Jul 23, 2024 at 4:48 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
>
>
>
> > On Jul 23, 2024, at 5:20 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Tue, Jul 23, 2024 at 10:06 AM Ajay Kaher <ajay.kaher@broadcom.com> wrote:
> >>
> >>> [ Upstream commit 9709bd548f11a092d124698118013f66e1740f9b ]
> >>>
> >>> Wire up the FAN_FS_ERROR event in the fanotify_mark syscall, allowing
> >>> user space to request the monitoring of FAN_FS_ERROR events.
> >>>
> >>> These events are limited to filesystem marks, so check it is the
> >>> case in the syscall handler.
> >>
> >> Greg,
> >>
> >> Without 9709bd548f11 in v5.10.y skips LTP fanotify22 test case, as:
> >> fanotify22.c:312: TCONF: FAN_FS_ERROR not supported in kernel
> >>
> >> With 9709bd548f11 in v5.10.220, LTP fanotify22 is failing because of
> >> timeout as no notification. To fix need to merge following two upstream
> >> commit to v5.10:
> >>
> >> 124e7c61deb27d758df5ec0521c36cf08d417f7a:
> >> 0001-ext4_fix_error_code_saved_on_super_block_during_file_system.patch
> >> https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#mf76930487697d8c1383ed5d21678fe504e8e2305
> >>
> >> 9a089b21f79b47eed240d4da7ea0d049de7c9b4d:
> >> 0001-ext4_Send_notifications_on_error.patch
> >> Link: https://lore.kernel.org/stable/1721717240-8786-1-git-send-email-ajay.kaher@broadcom.com/T/#md1be98e0ecafe4f92d7b61c048e15bcf286cbd53
> >>
> >> -Ajay
> >
> > I agree that this is the best approach, because the test has no other
> > way to test
> > if ext4 specifically supports FAN_FS_ERROR.
> >
> > Chuck,
> >
> > I wonder how those patches end up in 5.15.y but not in 5.10.y?
>
> The process was by hand. I tried to copy the same
> commit ID set from 5.15 to 5.10, but some patches
> did not apply to 5.10, or may have been omitted
> unintentionally.
>
>
> > Also, since you backported *most* of this series:
> > https://lore.kernel.org/all/20211025192746.66445-1-krisman@collabora.com/
> >
> > I think it would be wise to also backport the sample code and documentation
> > patches to 5.15.y and 5.10.y.
> >
> > 9abeae5d4458 docs: Fix formatting of literal sections in fanotify docs
> > 8fc70b3a142f samples: Make fs-monitor depend on libc and headers
> > c0baf9ac0b05 docs: Document the FAN_FS_ERROR event
> > 5451093081db samples: Add fs error monitoring example.
> >
> > Gabriel, if 9abeae5d4458 has a Fixes: tag it may have been auto seleced
> > for 5.15.y after c0baf9ac0b05 was picked up...
>
> Or this... I might not have backported those at all
> to 5.15, and they got pulled in by another process.
>
> In any event, do you want me to backport these to
> 5.10, or would you like to do it yourself?
>

Please backport the docs and sample patches above to 5.15 and 5.10
for completion of the backport.

I do not expect you should have any major conflicts with these standalone
document and sample code.

Thanks,
Amir.