Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1229673 > unrolled thread

[PATCH] inotify: hide internal kernel bits from fdinfo

Back to article view | Back to linux.kernel

Contents

  [PATCH] inotify: hide internal kernel bits from fdinfo Dave Hansen <dave@sr71.net> - 2015-09-21 21:10 +0200
    Re: [PATCH] inotify: hide internal kernel bits from fdinfo Cyrill Gorcunov <gorcunov@gmail.com> - 2015-09-21 21:30 +0200
    Re: [PATCH] inotify: hide internal kernel bits from fdinfo Eric Paris <eparis@redhat.com> - 2015-09-21 21:30 +0200
    Re: [PATCH] inotify: hide internal kernel bits from fdinfo Andrey Wagin <avagin@gmail.com> - 2015-09-21 22:00 +0200

#1229673 — [PATCH] inotify: hide internal kernel bits from fdinfo

From: Dave Hansen <dave.hansen@linux.intel.com>

There was a report that my patch:

	inotify: actually check for invalid bits in sys_inotify_add_watch()

broke CRIU.

The reason is that CRIU looks up raw flags in /proc/$pid/fdinfo/*
to figure out how to rebuild inotify watches and then passes those
flags directly back in to the inotify API.  One of those flags
(FS_EVENT_ON_CHILD) is set in mark->mask, but is not part of the
inotify API.  It is used inside the kernel to _implement_ inotify
but it is not and has never been part of the API.

My patch above ensured that we only allow bits which are part of
the API (IN_ALL_EVENTS).  This broke CRIU.

FS_EVENT_ON_CHILD is really internal to the kernel.  It is set
_anyway_ on all inotify marks.  So, CRIU was really just trying
to set a bit that was already set.

This patch hides that bit from fdinfo.  CRIU will not see the
bit, not try to set it, and should work as before.  We should not
have been exposing this bit in the first place, so this is a good
patch independent of the CRIU problem.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Reported-by: Andrey Wagin <avagin@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: xemul@parallels.com
Cc: Eric Paris <eparis@redhat.com>
Cc: john@johnmccutchan.com
Cc: rlove@rlove.org
Cc: linux-kernel@vger.kernel.org
---

 b/fs/notify/fdinfo.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff -puN fs/notify/fdinfo.c~fdinfo-mask fs/notify/fdinfo.c
--- a/fs/notify/fdinfo.c~fdinfo-mask	2015-09-21 10:24:01.031864268 -0700
+++ b/fs/notify/fdinfo.c	2015-09-21 10:25:04.335723826 -0700
@@ -82,9 +82,16 @@ static void inotify_fdinfo(struct seq_fi
 	inode_mark = container_of(mark, struct inotify_inode_mark, fsn_mark);
 	inode = igrab(mark->inode);
 	if (inode) {
+		/*
+		 * IN_ALL_EVENTS represents all of the mask bits
+		 * that we expose to userspace.  There is at
+		 * least one bit (FS_EVENT_ON_CHILD) which is
+		 * used only internally to the kernel.
+		 */
+		u32 mask = mark->mask & IN_ALL_EVENTS;
 		seq_printf(m, "inotify wd:%x ino:%lx sdev:%x mask:%x ignored_mask:%x ",
 			   inode_mark->wd, inode->i_ino, inode->i_sb->s_dev,
-			   mark->mask, mark->ignored_mask);
+			   mask, mark->ignored_mask);
 		show_mark_fhandle(m, inode);
 		seq_putc(m, '\n');
 		iput(inode);
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1229679

On Mon, Sep 21, 2015 at 11:45:01AM -0700, Dave Hansen wrote:
> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> There was a report that my patch:
> 
> 	inotify: actually check for invalid bits in sys_inotify_add_watch()
> 
> broke CRIU.
> 
> The reason is that CRIU looks up raw flags in /proc/$pid/fdinfo/*
> to figure out how to rebuild inotify watches and then passes those
> flags directly back in to the inotify API.  One of those flags
> (FS_EVENT_ON_CHILD) is set in mark->mask, but is not part of the
> inotify API.  It is used inside the kernel to _implement_ inotify
> but it is not and has never been part of the API.
> 
> My patch above ensured that we only allow bits which are part of
> the API (IN_ALL_EVENTS).  This broke CRIU.
> 
> FS_EVENT_ON_CHILD is really internal to the kernel.  It is set
> _anyway_ on all inotify marks.  So, CRIU was really just trying
> to set a bit that was already set.
> 
> This patch hides that bit from fdinfo.  CRIU will not see the
> bit, not try to set it, and should work as before.  We should not
> have been exposing this bit in the first place, so this is a good
> patch independent of the CRIU problem.
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reported-by: Andrey Wagin <avagin@gmail.com>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>

Thank you!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1229683

Acked-by: Eric Paris <eparis@redhat.com>

On Mon, 2015-09-21 at 11:45 -0700, Dave Hansen wrote:
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> There was a report that my patch:
> 
> 	inotify: actually check for invalid bits in
> sys_inotify_add_watch()
> 
> broke CRIU.
> 
> The reason is that CRIU looks up raw flags in /proc/$pid/fdinfo/*
> to figure out how to rebuild inotify watches and then passes those
> flags directly back in to the inotify API.  One of those flags
> (FS_EVENT_ON_CHILD) is set in mark->mask, but is not part of the
> inotify API.  It is used inside the kernel to _implement_ inotify
> but it is not and has never been part of the API.
> 
> My patch above ensured that we only allow bits which are part of
> the API (IN_ALL_EVENTS).  This broke CRIU.
> 
> FS_EVENT_ON_CHILD is really internal to the kernel.  It is set
> _anyway_ on all inotify marks.  So, CRIU was really just trying
> to set a bit that was already set.
> 
> This patch hides that bit from fdinfo.  CRIU will not see the
> bit, not try to set it, and should work as before.  We should not
> have been exposing this bit in the first place, so this is a good
> patch independent of the CRIU problem.
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reported-by: Andrey Wagin <avagin@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: xemul@parallels.com
> Cc: Eric Paris <eparis@redhat.com>
> Cc: john@johnmccutchan.com
> Cc: rlove@rlove.org
> Cc: linux-kernel@vger.kernel.org
> ---
> 
>  b/fs/notify/fdinfo.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff -puN fs/notify/fdinfo.c~fdinfo-mask fs/notify/fdinfo.c
> --- a/fs/notify/fdinfo.c~fdinfo-mask	2015-09-21
> 10:24:01.031864268 -0700
> +++ b/fs/notify/fdinfo.c	2015-09-21 10:25:04.335723826 -0700
> @@ -82,9 +82,16 @@ static void inotify_fdinfo(struct seq_fi
>  	inode_mark = container_of(mark, struct inotify_inode_mark,
> fsn_mark);
>  	inode = igrab(mark->inode);
>  	if (inode) {
> +		/*
> +		 * IN_ALL_EVENTS represents all of the mask bits
> +		 * that we expose to userspace.  There is at
> +		 * least one bit (FS_EVENT_ON_CHILD) which is
> +		 * used only internally to the kernel.
> +		 */
> +		u32 mask = mark->mask & IN_ALL_EVENTS;
>  		seq_printf(m, "inotify wd:%x ino:%lx sdev:%x mask:%x
> ignored_mask:%x ",
>  			   inode_mark->wd, inode->i_ino, inode->i_sb
> ->s_dev,
> -			   mark->mask, mark->ignored_mask);
> +			   mask, mark->ignored_mask);
>  		show_mark_fhandle(m, inode);
>  		seq_putc(m, '\n');
>  		iput(inode);
> _
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1229704

2015-09-21 21:45 GMT+03:00 Dave Hansen <dave@sr71.net>:
>
> From: Dave Hansen <dave.hansen@linux.intel.com>
>
> There was a report that my patch:
>
>         inotify: actually check for invalid bits in sys_inotify_add_watch()
>
> broke CRIU.
>
> The reason is that CRIU looks up raw flags in /proc/$pid/fdinfo/*
> to figure out how to rebuild inotify watches and then passes those
> flags directly back in to the inotify API.  One of those flags
> (FS_EVENT_ON_CHILD) is set in mark->mask, but is not part of the
> inotify API.  It is used inside the kernel to _implement_ inotify
> but it is not and has never been part of the API.
>
> My patch above ensured that we only allow bits which are part of
> the API (IN_ALL_EVENTS).  This broke CRIU.
>
> FS_EVENT_ON_CHILD is really internal to the kernel.  It is set
> _anyway_ on all inotify marks.  So, CRIU was really just trying
> to set a bit that was already set.
>
> This patch hides that bit from fdinfo.  CRIU will not see the
> bit, not try to set it, and should work as before.  We should not
> have been exposing this bit in the first place, so this is a good
> patch independent of the CRIU problem.
>
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reported-by: Andrey Wagin <avagin@gmail.com>

Acked-by: Andrey Vagin <avagin@openvz.org>

Thanks,
Andrey

> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: xemul@parallels.com
> Cc: Eric Paris <eparis@redhat.com>
> Cc: john@johnmccutchan.com
> Cc: rlove@rlove.org
> Cc: linux-kernel@vger.kernel.org
> ---
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web