[Bionic][SRU][PATCH 0/1] fsnotify: fix ignore mask logic in fsnotify()

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bionic][SRU][PATCH 0/1] fsnotify: fix ignore mask logic in fsnotify()

Po-Hsu Lin (Sam)
BugLink: https://bugs.launchpad.net/bugs/1802454

== Justification ==
Commit 92183a42898d ("fsnotify: fix ignore mask logic in
send_to_group()") acknowledges the use case of ignoring an event on
an inode mark, because of an ignore mask on a mount mark of the same
group (i.e. I want to get all events on this file, except for the events
that came from that mount).

This change depends on correctly merging the inode marks and mount marks
group lists, so that the mount mark ignore mask would be tested in
send_to_group(). Alas, the merging of the lists did not take into
account the case where event in question is not in the mask of any of
the mount marks.

To fix this, completely remove the tests for inode and mount event masks
from the lists merging code.

== Fix ==
9bdda4e9 (fsnotify: fix ignore mask logic in fsnotify())

Test kernels for Bionic can be found here:
http://people.canonical.com/~phlin/kernel/lp-1802454-fanotify10/bionic/

It seems that T/X poses with the same issue and we might be able to backport
it with the same logic, but I think we should target Bionic as it's a fix for
for 92183a42898d which was only applied to Bionic.

== Regression Potential ==
Low.

The backport work is required as it's missing commit 47d9c7cc457 in Bionic,
which generalizes the iteration of marks for inode_mark and vfsmount_mark, from:
    iter_info.inode_mark
to:
    iter_info.marks[FSNOTIFY_OBJ_TYPE_INODE]

But the patch can still be backported with the same logic without this commit.

The test with the syscalls test in LTP test suite shows no sign of regression.

== Test Case ==
Run the fanotify10 test in ubuntu_ltp_syscalls test suite. And it will pass
with the patched kernel.

Full LTP syscalls test with 4.15.0-42 in -proposed:
http://paste.ubuntu.com/p/Wm8nQ932g6/

Full LTP syscalls test with patched 4.15.0-42:
http://paste.ubuntu.com/p/nv4jW6shpJ/


Amir Goldstein (1):
  fsnotify: fix ignore mask logic in fsnotify()

 fs/notify/fsnotify.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[Bionic][SRU][PATCH 1/1] fsnotify: fix ignore mask logic in fsnotify()

Po-Hsu Lin (Sam)
From: Amir Goldstein <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1802454

Commit 92183a42898d ("fsnotify: fix ignore mask logic in
send_to_group()") acknoledges the use case of ignoring an event on
an inode mark, because of an ignore mask on a mount mark of the same
group (i.e. I want to get all events on this file, except for the events
that came from that mount).

This change depends on correctly merging the inode marks and mount marks
group lists, so that the mount mark ignore mask would be tested in
send_to_group(). Alas, the merging of the lists did not take into
account the case where event in question is not in the mask of any of
the mount marks.

To fix this, completely remove the tests for inode and mount event masks
from the lists merging code.

Fixes: 92183a42898d ("fsnotify: fix ignore mask logic in send_to_group")
Signed-off-by: Amir Goldstein <[hidden email]>
Signed-off-by: Jan Kara <[hidden email]>
(backported from commit 9bdda4e9cf2dcecb60a0683b10ffb8cd7e5f2f45)
Signed-off-by: Po-Hsu Lin <[hidden email]>
---
 fs/notify/fsnotify.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 613ec7e..6a862aa 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -305,16 +305,9 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
 
  iter_info.srcu_idx = srcu_read_lock(&fsnotify_mark_srcu);
 
- if ((mask & FS_MODIFY) ||
-    (test_mask & to_tell->i_fsnotify_mask)) {
- iter_info.inode_mark =
- fsnotify_first_mark(&to_tell->i_fsnotify_marks);
- }
-
- if (mnt && ((mask & FS_MODIFY) ||
-    (test_mask & mnt->mnt_fsnotify_mask))) {
- iter_info.inode_mark =
- fsnotify_first_mark(&to_tell->i_fsnotify_marks);
+ iter_info.inode_mark =
+ fsnotify_first_mark(&to_tell->i_fsnotify_marks);
+ if (mnt) {
  iter_info.vfsmount_mark =
  fsnotify_first_mark(&mnt->mnt_fsnotify_marks);
  }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [Bionic][SRU][PATCH 1/1] fsnotify: fix ignore mask logic in fsnotify()

Khaled Elmously
On 2018-11-22 19:31:33 , Po-Hsu Lin wrote:

> From: Amir Goldstein <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1802454
>
> Commit 92183a42898d ("fsnotify: fix ignore mask logic in
> send_to_group()") acknoledges the use case of ignoring an event on
> an inode mark, because of an ignore mask on a mount mark of the same
> group (i.e. I want to get all events on this file, except for the events
> that came from that mount).
>
> This change depends on correctly merging the inode marks and mount marks
> group lists, so that the mount mark ignore mask would be tested in
> send_to_group(). Alas, the merging of the lists did not take into
> account the case where event in question is not in the mask of any of
> the mount marks.
>
> To fix this, completely remove the tests for inode and mount event masks
> from the lists merging code.
>
> Fixes: 92183a42898d ("fsnotify: fix ignore mask logic in send_to_group")
> Signed-off-by: Amir Goldstein <[hidden email]>
> Signed-off-by: Jan Kara <[hidden email]>
> (backported from commit 9bdda4e9cf2dcecb60a0683b10ffb8cd7e5f2f45)
> Signed-off-by: Po-Hsu Lin <[hidden email]>
> ---
>  fs/notify/fsnotify.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 613ec7e..6a862aa 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -305,16 +305,9 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
>  
>   iter_info.srcu_idx = srcu_read_lock(&fsnotify_mark_srcu);
>  
> - if ((mask & FS_MODIFY) ||
> -    (test_mask & to_tell->i_fsnotify_mask)) {
> - iter_info.inode_mark =
> - fsnotify_first_mark(&to_tell->i_fsnotify_marks);
> - }
> -
> - if (mnt && ((mask & FS_MODIFY) ||
> -    (test_mask & mnt->mnt_fsnotify_mask))) {
> - iter_info.inode_mark =
> - fsnotify_first_mark(&to_tell->i_fsnotify_marks);
> + iter_info.inode_mark =
> + fsnotify_first_mark(&to_tell->i_fsnotify_marks);
> + if (mnt) {
>   iter_info.vfsmount_mark =
>   fsnotify_first_mark(&mnt->mnt_fsnotify_marks);
>   }

Acked-by: Khalid Elmously <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [Bionic][SRU][PATCH 1/1] fsnotify: fix ignore mask logic in fsnotify()

Kleber Souza
In reply to this post by Po-Hsu Lin (Sam)
On 11/22/18 12:31 PM, Po-Hsu Lin wrote:

> From: Amir Goldstein <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1802454
>
> Commit 92183a42898d ("fsnotify: fix ignore mask logic in
> send_to_group()") acknoledges the use case of ignoring an event on
> an inode mark, because of an ignore mask on a mount mark of the same
> group (i.e. I want to get all events on this file, except for the events
> that came from that mount).
>
> This change depends on correctly merging the inode marks and mount marks
> group lists, so that the mount mark ignore mask would be tested in
> send_to_group(). Alas, the merging of the lists did not take into
> account the case where event in question is not in the mask of any of
> the mount marks.
>
> To fix this, completely remove the tests for inode and mount event masks
> from the lists merging code.
>
> Fixes: 92183a42898d ("fsnotify: fix ignore mask logic in send_to_group")
> Signed-off-by: Amir Goldstein <[hidden email]>
> Signed-off-by: Jan Kara <[hidden email]>
> (backported from commit 9bdda4e9cf2dcecb60a0683b10ffb8cd7e5f2f45)
> Signed-off-by: Po-Hsu Lin <[hidden email]>

The backport looks correct, good regression test results.

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

> ---
>  fs/notify/fsnotify.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 613ec7e..6a862aa 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -305,16 +305,9 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
>  
>   iter_info.srcu_idx = srcu_read_lock(&fsnotify_mark_srcu);
>  
> - if ((mask & FS_MODIFY) ||
> -    (test_mask & to_tell->i_fsnotify_mask)) {
> - iter_info.inode_mark =
> - fsnotify_first_mark(&to_tell->i_fsnotify_marks);
> - }
> -
> - if (mnt && ((mask & FS_MODIFY) ||
> -    (test_mask & mnt->mnt_fsnotify_mask))) {
> - iter_info.inode_mark =
> - fsnotify_first_mark(&to_tell->i_fsnotify_marks);
> + iter_info.inode_mark =
> + fsnotify_first_mark(&to_tell->i_fsnotify_marks);
> + if (mnt) {
>   iter_info.vfsmount_mark =
>   fsnotify_first_mark(&mnt->mnt_fsnotify_marks);
>   }



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [Bionic][SRU][PATCH 1/1] fsnotify: fix ignore mask logic in fsnotify()

Kleber Souza
In reply to this post by Po-Hsu Lin (Sam)
On 11/22/18 12:31 PM, Po-Hsu Lin wrote:

> From: Amir Goldstein <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1802454
>
> Commit 92183a42898d ("fsnotify: fix ignore mask logic in
> send_to_group()") acknoledges the use case of ignoring an event on
> an inode mark, because of an ignore mask on a mount mark of the same
> group (i.e. I want to get all events on this file, except for the events
> that came from that mount).
>
> This change depends on correctly merging the inode marks and mount marks
> group lists, so that the mount mark ignore mask would be tested in
> send_to_group(). Alas, the merging of the lists did not take into
> account the case where event in question is not in the mask of any of
> the mount marks.
>
> To fix this, completely remove the tests for inode and mount event masks
> from the lists merging code.
>
> Fixes: 92183a42898d ("fsnotify: fix ignore mask logic in send_to_group")
> Signed-off-by: Amir Goldstein <[hidden email]>
> Signed-off-by: Jan Kara <[hidden email]>
> (backported from commit 9bdda4e9cf2dcecb60a0683b10ffb8cd7e5f2f45)
> Signed-off-by: Po-Hsu Lin <[hidden email]>
> ---
>  fs/notify/fsnotify.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 613ec7e..6a862aa 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -305,16 +305,9 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
>  
>   iter_info.srcu_idx = srcu_read_lock(&fsnotify_mark_srcu);
>  
> - if ((mask & FS_MODIFY) ||
> -    (test_mask & to_tell->i_fsnotify_mask)) {
> - iter_info.inode_mark =
> - fsnotify_first_mark(&to_tell->i_fsnotify_marks);
> - }
> -
> - if (mnt && ((mask & FS_MODIFY) ||
> -    (test_mask & mnt->mnt_fsnotify_mask))) {
> - iter_info.inode_mark =
> - fsnotify_first_mark(&to_tell->i_fsnotify_marks);
> + iter_info.inode_mark =
> + fsnotify_first_mark(&to_tell->i_fsnotify_marks);
> + if (mnt) {
>   iter_info.vfsmount_mark =
>   fsnotify_first_mark(&mnt->mnt_fsnotify_marks);
>   }

Applied to bionic/master-next branch.

Thanks,
Kleber


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team