[Patch 1/1] fsnotify,fanotify: adding flag for execution

[Patch 1/1] fsnotify,fanotify: adding flag for execution

[jozef.kralik]

[-- Attachment #1: Type: text/plain, Size: 4641 bytes --]

From: Jozef Kralik <jozef.kralik@eset.sk>

This patch add flag FS_OPENEXEC[FAN_OPENEXEC] to event
FS_OPEN_PERM[FAN_OPEN_PERM], when file is opened with flag FMODE_EXEC. 

Signed-off-by: Jozef Kralik <jozef.kralik@eset.sk>
---
Example:
if (metadata->mask & FAN_OPEN_PERM)
	if (metadata->mask & FAN_OPENEXEC)
		printf("file was executed");
	else
	printf("file was opened");

Patch for kernel: 2.6.38
Developed kernel: 2.6.37-rc4 with patch-v2.6.37-rc4-next-20101201 
Tested kernel: 2.6.38

diffstat -p1 ./patch_exec_2.6.38.diff 
fs/notify/fanotify/fanotify.c    |    1 +
fs/notify/fsnotify.c             |    2 +-
include/linux/fanotify.h         |    4 +++-
include/linux/fsnotify.h         |    9 ++++++---
include/linux/fsnotify_backend.h |    4 +++-
5 files changed, 14 insertions(+), 6 deletions(-)

diff -uprN -X linux-2.6.38/Documentation/dontdiff linux-2.6.38/fs/notify/fanotify/fanotify.c linux-2.6.38-dev/fs/notify/fanotify/fanotify.c
--- linux-2.6.38/fs/notify/fanotify/fanotify.c	2011-03-15 02:20:32.000000000 +0100
+++ linux-2.6.38-dev/fs/notify/fanotify/fanotify.c	2011-03-24 12:34:40.182283000 +0100
@@ -131,6 +131,7 @@ static int fanotify_handle_event(struct
 	BUILD_BUG_ON(FAN_CLOSE_NOWRITE != FS_CLOSE_NOWRITE);
 	BUILD_BUG_ON(FAN_CLOSE_WRITE != FS_CLOSE_WRITE);
 	BUILD_BUG_ON(FAN_OPEN != FS_OPEN);
+	BUILD_BUG_ON(FAN_OPENEXEC != FS_OPENEXEC);
 	BUILD_BUG_ON(FAN_EVENT_ON_CHILD != FS_EVENT_ON_CHILD);
 	BUILD_BUG_ON(FAN_Q_OVERFLOW != FS_Q_OVERFLOW);
 	BUILD_BUG_ON(FAN_OPEN_PERM != FS_OPEN_PERM);
diff -uprN -X linux-2.6.38/Documentation/dontdiff linux-2.6.38/fs/notify/fsnotify.c linux-2.6.38-dev/fs/notify/fsnotify.c
--- linux-2.6.38/fs/notify/fsnotify.c	2011-03-15 02:20:32.000000000 +0100
+++ linux-2.6.38-dev/fs/notify/fsnotify.c	2011-03-24 12:34:40.186283000 +0100
@@ -299,7 +299,7 @@ static __init int fsnotify_init(void)
 {
 	int ret;
 
-	BUG_ON(hweight32(ALL_FSNOTIFY_EVENTS) != 23);
+	BUG_ON(hweight32(ALL_FSNOTIFY_EVENTS) != 24);
 
 	ret = init_srcu_struct(&fsnotify_mark_srcu);
 	if (ret)
diff -uprN -X linux-2.6.38/Documentation/dontdiff linux-2.6.38/include/linux/fanotify.h linux-2.6.38-dev/include/linux/fanotify.h
--- linux-2.6.38/include/linux/fanotify.h	2011-03-15 02:20:32.000000000 +0100
+++ linux-2.6.38-dev/include/linux/fanotify.h	2011-03-24 12:34:40.190283001 +0100
@@ -9,6 +9,7 @@
 #define FAN_CLOSE_WRITE		0x00000008	/* Writtable file closed */
 #define FAN_CLOSE_NOWRITE	0x00000010	/* Unwrittable file closed */
 #define FAN_OPEN		0x00000020	/* File was opened */
+#define FAN_OPENEXEC	0x00001000	/* File had exec flag */
 
 #define FAN_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
 
@@ -81,7 +82,8 @@
 
 #define FAN_ALL_OUTGOING_EVENTS	(FAN_ALL_EVENTS |\
 				 FAN_ALL_PERM_EVENTS |\
-				 FAN_Q_OVERFLOW)
+				FAN_Q_OVERFLOW |\
+				FAN_OPENEXEC)
 
 #define FANOTIFY_METADATA_VERSION	3
 
diff -uprN -X linux-2.6.38/Documentation/dontdiff linux-2.6.38/include/linux/fsnotify_backend.h linux-2.6.38-dev/include/linux/fsnotify_backend.h
--- linux-2.6.38/include/linux/fsnotify_backend.h	2011-03-15 02:20:32.000000000 +0100
+++ linux-2.6.38-dev/include/linux/fsnotify_backend.h	2011-03-24 12:34:40.262283001 +0100
@@ -36,6 +36,7 @@
 #define FS_DELETE		0x00000200	/* Subfile was deleted */
 #define FS_DELETE_SELF		0x00000400	/* Self was deleted */
 #define FS_MOVE_SELF		0x00000800	/* Self was moved */
+#define FS_OPENEXEC		0x00001000	/* File had exec flag */
 
 #define FS_UNMOUNT		0x00002000	/* inode on umount fs */
 #define FS_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
@@ -73,7 +74,8 @@
 			     FS_UNMOUNT | FS_Q_OVERFLOW | FS_IN_IGNORED | \
 			     FS_OPEN_PERM | FS_ACCESS_PERM | FS_EXCL_UNLINK | \
 			     FS_ISDIR | FS_IN_ONESHOT | FS_DN_RENAME | \
-			     FS_DN_MULTISHOT | FS_EVENT_ON_CHILD)
+				FS_DN_MULTISHOT | FS_EVENT_ON_CHILD | \
+				FS_OPENEXEC)
 
 struct fsnotify_group;
 struct fsnotify_event;
diff -uprN -X linux-2.6.38/Documentation/dontdiff linux-2.6.38/include/linux/fsnotify.h linux-2.6.38-dev/include/linux/fsnotify.h
--- linux-2.6.38/include/linux/fsnotify.h	2011-03-15 02:20:32.000000000 +0100
+++ linux-2.6.38-dev/include/linux/fsnotify.h	2011-03-24 12:34:40.274283001 +0100
@@ -45,12 +45,15 @@ static inline int fsnotify_perm(struct f
 		return 0;
 	if (!(mask & (MAY_READ | MAY_OPEN)))
 		return 0;
-	if (mask & MAY_OPEN)
+	if (mask & MAY_OPEN) {
 		fsnotify_mask = FS_OPEN_PERM;
-	else if (mask & MAY_READ)
+		if (file->f_flags & FMODE_EXEC)
+			fsnotify_mask |= FS_OPENEXEC;
+	} else if (mask & MAY_READ) {
 		fsnotify_mask = FS_ACCESS_PERM;
-	else
+	} else {
 		BUG();
+	}
 
 	ret = fsnotify_parent(path, NULL, fsnotify_mask);
 	if (ret)

Re: [Patch 1/1] fsnotify,fanotify: adding flag for execution

[Eric Paris]

On Thu, 2011-03-24 at 12:49 +0000, jozef.kralik@eset.sk wrote:
> From: Jozef Kralik <jozef.kralik@eset.sk>
> 
> This patch add flag FS_OPENEXEC[FAN_OPENEXEC] to event
> FS_OPEN_PERM[FAN_OPEN_PERM], when file is opened with flag FMODE_EXEC. 
> 
> Signed-off-by: Jozef Kralik <jozef.kralik@eset.sk>

I keep waffling back and forth on this patch (for months now) mostly
because of the fact that I'm scared it will give people a false sense
that they will get notification of all files that might be executed.  I
don't understand the use case at all so I don't know if it good idea to
expose such notifications....

I'm just having a hard time deciding if I'm comfortable solving half of
a problem...  What exactly is your goal here and how do you see others
using it, usefully.

-Eric

> ---
> Example:
> if (metadata->mask & FAN_OPEN_PERM)
> 	if (metadata->mask & FAN_OPENEXEC)
> 		printf("file was executed");
> 	else
> 	printf("file was opened");
> 
> Patch for kernel: 2.6.38
> Developed kernel: 2.6.37-rc4 with patch-v2.6.37-rc4-next-20101201 
> Tested kernel: 2.6.38
> 
> diffstat -p1 ./patch_exec_2.6.38.diff 
> fs/notify/fanotify/fanotify.c    |    1 +
> fs/notify/fsnotify.c             |    2 +-
> include/linux/fanotify.h         |    4 +++-
> include/linux/fsnotify.h         |    9 ++++++---
> include/linux/fsnotify_backend.h |    4 +++-
> 5 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff -uprN -X linux-2.6.38/Documentation/dontdiff linux-2.6.38/fs/notify/fanotify/fanotify.c linux-2.6.38-dev/fs/notify/fanotify/fanotify.c
> --- linux-2.6.38/fs/notify/fanotify/fanotify.c	2011-03-15 02:20:32.000000000 +0100
> +++ linux-2.6.38-dev/fs/notify/fanotify/fanotify.c	2011-03-24 12:34:40.182283000 +0100
> @@ -131,6 +131,7 @@ static int fanotify_handle_event(struct
>  	BUILD_BUG_ON(FAN_CLOSE_NOWRITE != FS_CLOSE_NOWRITE);
>  	BUILD_BUG_ON(FAN_CLOSE_WRITE != FS_CLOSE_WRITE);
>  	BUILD_BUG_ON(FAN_OPEN != FS_OPEN);
> +	BUILD_BUG_ON(FAN_OPENEXEC != FS_OPENEXEC);
>  	BUILD_BUG_ON(FAN_EVENT_ON_CHILD != FS_EVENT_ON_CHILD);
>  	BUILD_BUG_ON(FAN_Q_OVERFLOW != FS_Q_OVERFLOW);
>  	BUILD_BUG_ON(FAN_OPEN_PERM != FS_OPEN_PERM);
> diff -uprN -X linux-2.6.38/Documentation/dontdiff linux-2.6.38/fs/notify/fsnotify.c linux-2.6.38-dev/fs/notify/fsnotify.c
> --- linux-2.6.38/fs/notify/fsnotify.c	2011-03-15 02:20:32.000000000 +0100
> +++ linux-2.6.38-dev/fs/notify/fsnotify.c	2011-03-24 12:34:40.186283000 +0100
> @@ -299,7 +299,7 @@ static __init int fsnotify_init(void)
>  {
>  	int ret;
>  
> -	BUG_ON(hweight32(ALL_FSNOTIFY_EVENTS) != 23);
> +	BUG_ON(hweight32(ALL_FSNOTIFY_EVENTS) != 24);
>  
>  	ret = init_srcu_struct(&fsnotify_mark_srcu);
>  	if (ret)
> diff -uprN -X linux-2.6.38/Documentation/dontdiff linux-2.6.38/include/linux/fanotify.h linux-2.6.38-dev/include/linux/fanotify.h
> --- linux-2.6.38/include/linux/fanotify.h	2011-03-15 02:20:32.000000000 +0100
> +++ linux-2.6.38-dev/include/linux/fanotify.h	2011-03-24 12:34:40.190283001 +0100
> @@ -9,6 +9,7 @@
>  #define FAN_CLOSE_WRITE		0x00000008	/* Writtable file closed */
>  #define FAN_CLOSE_NOWRITE	0x00000010	/* Unwrittable file closed */
>  #define FAN_OPEN		0x00000020	/* File was opened */
> +#define FAN_OPENEXEC	0x00001000	/* File had exec flag */
>  
>  #define FAN_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
>  
> @@ -81,7 +82,8 @@
>  
>  #define FAN_ALL_OUTGOING_EVENTS	(FAN_ALL_EVENTS |\
>  				 FAN_ALL_PERM_EVENTS |\
> -				 FAN_Q_OVERFLOW)
> +				FAN_Q_OVERFLOW |\
> +				FAN_OPENEXEC)
>  
>  #define FANOTIFY_METADATA_VERSION	3
>  
> diff -uprN -X linux-2.6.38/Documentation/dontdiff linux-2.6.38/include/linux/fsnotify_backend.h linux-2.6.38-dev/include/linux/fsnotify_backend.h
> --- linux-2.6.38/include/linux/fsnotify_backend.h	2011-03-15 02:20:32.000000000 +0100
> +++ linux-2.6.38-dev/include/linux/fsnotify_backend.h	2011-03-24 12:34:40.262283001 +0100
> @@ -36,6 +36,7 @@
>  #define FS_DELETE		0x00000200	/* Subfile was deleted */
>  #define FS_DELETE_SELF		0x00000400	/* Self was deleted */
>  #define FS_MOVE_SELF		0x00000800	/* Self was moved */
> +#define FS_OPENEXEC		0x00001000	/* File had exec flag */
>  
>  #define FS_UNMOUNT		0x00002000	/* inode on umount fs */
>  #define FS_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
> @@ -73,7 +74,8 @@
>  			     FS_UNMOUNT | FS_Q_OVERFLOW | FS_IN_IGNORED | \
>  			     FS_OPEN_PERM | FS_ACCESS_PERM | FS_EXCL_UNLINK | \
>  			     FS_ISDIR | FS_IN_ONESHOT | FS_DN_RENAME | \
> -			     FS_DN_MULTISHOT | FS_EVENT_ON_CHILD)
> +				FS_DN_MULTISHOT | FS_EVENT_ON_CHILD | \
> +				FS_OPENEXEC)
>  
>  struct fsnotify_group;
>  struct fsnotify_event;
> diff -uprN -X linux-2.6.38/Documentation/dontdiff linux-2.6.38/include/linux/fsnotify.h linux-2.6.38-dev/include/linux/fsnotify.h
> --- linux-2.6.38/include/linux/fsnotify.h	2011-03-15 02:20:32.000000000 +0100
> +++ linux-2.6.38-dev/include/linux/fsnotify.h	2011-03-24 12:34:40.274283001 +0100
> @@ -45,12 +45,15 @@ static inline int fsnotify_perm(struct f
>  		return 0;
>  	if (!(mask & (MAY_READ | MAY_OPEN)))
>  		return 0;
> -	if (mask & MAY_OPEN)
> +	if (mask & MAY_OPEN) {
>  		fsnotify_mask = FS_OPEN_PERM;
> -	else if (mask & MAY_READ)
> +		if (file->f_flags & FMODE_EXEC)
> +			fsnotify_mask |= FS_OPENEXEC;
> +	} else if (mask & MAY_READ) {
>  		fsnotify_mask = FS_ACCESS_PERM;
> -	else
> +	} else {
>  		BUG();
> +	}
>  
>  	ret = fsnotify_parent(path, NULL, fsnotify_mask);
>  	if (ret)
> 

Re: [Patch 1/1] fsnotify,fanotify: adding flag for execution

[Linus Torvalds]

On Thu, Mar 24, 2011 at 8:30 AM, Eric Paris <eparis@redhat.com> wrote:
>
> I keep waffling back and forth on this patch (for months now) mostly
> because of the fact that I'm scared it will give people a false sense
> that they will get notification of all files that might be executed.  I
> don't understand the use case at all so I don't know if it good idea to
> expose such notifications....

We have indeed historically had those kinds of bugs. For example,
having 'noexec' disable execve() on files, but *not* disabling using
them as LD_PRELOAD=xyz things where they aren't the target of an
execve(), but the code in them is run anyway (thanks to just an
executable mmap(), or even a "read()" into a data segment that is
executable)

So I don't know what makes "executed" different from "read". Because
at some point we really cannot tell the difference.

The one special thing about execve() is that it can execute something
even when it's not readable. But people who have depended on that as a
security feature have always been disappointed (ie just execve it and
then use ptrace to read the contents of the file _anyway_). So once
again it's not at all clear that "execute" should ever be considered
to be anything but "read".

                  Linus

Re: [Patch 1/1] fsnotify,fanotify: adding flag for execution

[Alan Cox]

> We have indeed historically had those kinds of bugs. For example,
> having 'noexec' disable execve() on files, but *not* disabling using
> them as LD_PRELOAD=xyz things where they aren't the target of an
> execve(), but the code in them is run anyway (thanks to just an
> executable mmap(), or even a "read()" into a data segment that is
> executable)

Actually there is a difference

> So I don't know what makes "executed" different from "read". Because
> at some point we really cannot tell the difference.

And this isn't quite true

> The one special thing about execve() is that it can execute something
> even when it's not readable. But people who have depended on that as a
> security feature have always been disappointed (ie just execve it and
> then use ptrace to read the contents of the file _anyway_). So once

And ptrace covers this

The specific case that has always been the one Unix has cared about is
'exec but no read bit', and execve() covers that correctly, including
stopping it being dumpable thus blocking ptrace.

Various standard Unix tricks rely upon this behaviour, notably the cd
into a directory whose name you can't determine to access "protected" by
writable stuff. All of the latter is really about suid apps though.

Alan

[Patch 1/1] fsnotify,fanotify: adding flag for execution

[Jozef Kralik]

From: Jozef Kralik <jozef.kralik@eset.sk>

This patch add flag FS_OPENEXEC[FAN_OPENEXEC] to event
FS_OPEN_PERM[FAN_OPEN_PERM], when file is opened with flag FMODE_EXEC. 

Signed-off-by: Jozef Kralik <jozef.kralik@eset.sk>
---
Example:
if (metadata->mask & FAN_OPEN_PERM)
	if (metadata->mask & FAN_OPENEXEC)
		printf("file was executed");
	else
		printf("file was opened");
Developed & Tested kernel: 2.6.37-rc4 with patch: 
patch-v2.6.37-rc4-next-20101201

diffstat -p1 patch
 fs/notify/fanotify/fanotify.c    |    1 +
 fs/notify/fsnotify.c             |    2 +-
 include/linux/fanotify.h         |    4 +++-
 include/linux/fsnotify.h         |    9 ++++++---
 include/linux/fsnotify_backend.h |    4 +++-
 5 files changed, 14 insertions(+), 6 deletions(-)

diff -uprN -X linux-2.6.37-rc4/Documentation/dontdiff
linux-2.6.37-rc4/fs/notify/fanotify/fanotify.c
linux-2.6.37-rc4-dev/fs/notify/fanotify/fanotify.c
--- linux-2.6.37-rc4/fs/notify/fanotify/fanotify.c	2010-12-08
10:07:17.605885678 +0100
+++ linux-2.6.37-rc4-dev/fs/notify/fanotify/fanotify.c	2010-12-08
10:08:06.729268116 +0100
@@ -131,6 +131,7 @@ static int fanotify_handle_event(struct
 	BUILD_BUG_ON(FAN_CLOSE_NOWRITE != FS_CLOSE_NOWRITE);
 	BUILD_BUG_ON(FAN_CLOSE_WRITE != FS_CLOSE_WRITE);
 	BUILD_BUG_ON(FAN_OPEN != FS_OPEN);
+	BUILD_BUG_ON(FAN_OPENEXEC != FS_OPENEXEC);
 	BUILD_BUG_ON(FAN_EVENT_ON_CHILD != FS_EVENT_ON_CHILD);
 	BUILD_BUG_ON(FAN_Q_OVERFLOW != FS_Q_OVERFLOW);
 	BUILD_BUG_ON(FAN_OPEN_PERM != FS_OPEN_PERM);
diff -uprN -X linux-2.6.37-rc4/Documentation/dontdiff
linux-2.6.37-rc4/fs/notify/fsnotify.c
linux-2.6.37-rc4-dev/fs/notify/fsnotify.c
--- linux-2.6.37-rc4/fs/notify/fsnotify.c	2010-11-30 05:42:04.000000000
+0100
+++ linux-2.6.37-rc4-dev/fs/notify/fsnotify.c	2010-12-08
10:08:06.729268116 +0100
@@ -297,7 +297,7 @@ static __init int fsnotify_init(void)
 {
 	int ret;
 
-	BUG_ON(hweight32(ALL_FSNOTIFY_EVENTS) != 23);
+	BUG_ON(hweight32(ALL_FSNOTIFY_EVENTS) != 24);
 
 	ret = init_srcu_struct(&fsnotify_mark_srcu);
 	if (ret)
diff -uprN -X linux-2.6.37-rc4/Documentation/dontdiff
linux-2.6.37-rc4/include/linux/fanotify.h
linux-2.6.37-rc4-dev/include/linux/fanotify.h
--- linux-2.6.37-rc4/include/linux/fanotify.h	2010-12-08
10:07:17.649885125 +0100
+++ linux-2.6.37-rc4-dev/include/linux/fanotify.h	2010-12-08
10:08:06.733268063 +0100
@@ -9,6 +9,7 @@
 #define FAN_CLOSE_WRITE		0x00000008	/* Writtable file closed */
 #define FAN_CLOSE_NOWRITE	0x00000010	/* Unwrittable file closed */
 #define FAN_OPEN		0x00000020	/* File was opened */
+#define FAN_OPENEXEC	0x00001000	/* File had exec flag */
 
 #define FAN_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
 
@@ -84,7 +85,8 @@
 
 #define FAN_ALL_OUTGOING_EVENTS	(FAN_ALL_EVENTS |\
 				 FAN_ALL_PERM_EVENTS |\
-				 FAN_Q_OVERFLOW)
+				FAN_Q_OVERFLOW |\
+				FAN_OPENEXEC)
 
 #define FANOTIFY_METADATA_VERSION	2
 
diff -uprN -X linux-2.6.37-rc4/Documentation/dontdiff
linux-2.6.37-rc4/include/linux/fsnotify_backend.h
linux-2.6.37-rc4-dev/include/linux/fsnotify_backend.h
--- linux-2.6.37-rc4/include/linux/fsnotify_backend.h	2010-12-08
10:07:17.657885026 +0100
+++ linux-2.6.37-rc4-dev/include/linux/fsnotify_backend.h	2010-12-08
10:08:06.741267961 +0100
@@ -36,6 +36,7 @@
 #define FS_DELETE		0x00000200	/* Subfile was deleted */
 #define FS_DELETE_SELF		0x00000400	/* Self was deleted */
 #define FS_MOVE_SELF		0x00000800	/* Self was moved */
+#define FS_OPENEXEC		0x00001000	/* File had exec flag */
 
 #define FS_UNMOUNT		0x00002000	/* inode on umount fs */
 #define FS_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
@@ -73,7 +74,8 @@
 			     FS_UNMOUNT | FS_Q_OVERFLOW | FS_IN_IGNORED | \
 			     FS_OPEN_PERM | FS_ACCESS_PERM | FS_EXCL_UNLINK | \
 			     FS_ISDIR | FS_IN_ONESHOT | FS_DN_RENAME | \
-			     FS_DN_MULTISHOT | FS_EVENT_ON_CHILD)
+				FS_DN_MULTISHOT | FS_EVENT_ON_CHILD | \
+				FS_OPENEXEC)
 
 struct fsnotify_group;
 struct fsnotify_event;
diff -uprN -X linux-2.6.37-rc4/Documentation/dontdiff
linux-2.6.37-rc4/include/linux/fsnotify.h
linux-2.6.37-rc4-dev/include/linux/fsnotify.h
--- linux-2.6.37-rc4/include/linux/fsnotify.h	2010-12-08
10:07:17.653885076 +0100
+++ linux-2.6.37-rc4-dev/include/linux/fsnotify.h	2010-12-08
10:11:17.086874772 +0100
@@ -46,12 +46,15 @@ static inline int fsnotify_perm(struct f
 		return 0;
 	if (!(mask & (MAY_READ | MAY_OPEN)))
 		return 0;
-	if (mask & MAY_OPEN)
+	if (mask & MAY_OPEN) {
 		fsnotify_mask = FS_OPEN_PERM;
-	else if (mask & MAY_READ)
+		if (file->f_flags & FMODE_EXEC)
+			fsnotify_mask |= FS_OPENEXEC;
+	} else if (mask & MAY_READ) {
 		fsnotify_mask = FS_ACCESS_PERM;
-	else
+	} else {
 		BUG();
+	}
 
 	ret = fsnotify_parent(path, NULL, fsnotify_mask);
 	if (ret)