Skip to contentSkip to conversation

sys_execve and sys_uselib do not call into fsnotify

PATCH In review Eric Paris 4 participants 7 messages Dec 17, 2008 – Dec 18, 2008

Subsystems (2)
lore
Share & tools
lore

Eric Paris

author
#0 lore raw

PATCH 1/1 sys_execve and sys_uselib do not call into fsnotify

sys_execve and sys_uselib do not call into fsnotify so inotify does not get open events for these types of syscalls. This patch simply makes the requisite fsnotify calls.

---

 fs/exec.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
fs/exec.c +5 -0
index ec5df9a..cbd93b5 100644
@@ -51,6 +51,7 @@
 #include <linux/audit.h>
 #include <linux/tracehook.h>
 #include <linux/kmod.h>
+#include <linux/fsnotify.h>
 
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>
@@ -135,6 +136,8 @@ asmlinkage long sys_uselib(const char __user * library)
 	if (IS_ERR(file))
 		goto out;
 
+	fsnotify_open(file->f_path.dentry);
+
 	error = -ENOEXEC;
 	if(file->f_op) {
 		struct linux_binfmt * fmt;
@@ -687,6 +690,8 @@ struct file *open_exec(const char *name)
 	if (IS_ERR(file))
 		return file;
 
+	fsnotify_open(file->f_path.dentry);
+
 	err = deny_write_access(file);
 	if (err) {
 		fput(file);
#1 lore raw

Hello Eric,

Quoted 3 lines · Eric Paris

sys_execve and sys_uselib do not call into fsnotify so inotify does not get open events for these types of syscalls. This patch simply makes the requisite fsnotify calls.

Just curious, isn't the fact that the open still may fail a problem? For example, if the lib is not executable in sys_uselib or if the file was denied write access in open_exec.

Regards,
Frederik

Quoted 45 lines

---

 fs/exec.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
fs/exec.c +5 -1
index ec5df9a..cbd93b5 100644
@@ -51,6 +51,7 @@
 #include <linux/audit.h>
 #include <linux/tracehook.h>
 #include <linux/kmod.h>
+#include <linux/fsnotify.h>
 
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>
@@ -135,6 +136,8 @@ asmlinkage long sys_uselib(const char __user * library)
 	if (IS_ERR(file))
 		goto out;
 
+	fsnotify_open(file->f_path.dentry);
+
 	error = -ENOEXEC;
 	if(file->f_op) {
 		struct linux_binfmt * fmt;
@@ -687,6 +690,8 @@ struct file *open_exec(const char *name)
 	if (IS_ERR(file))
 		return file;
 
+	fsnotify_open(file->f_path.dentry);
+
 	err = deny_write_access(file);
 	if (err) {
 		fput(file);
 
 
--

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

author
#2 lore raw
Quoted 10 lines · Frederik Deweerdt

Hello Eric,

Quoted 3 lines · Eric Paris

sys_execve and sys_uselib do not call into fsnotify so inotify does not get open events for these types of syscalls. This patch simply makes the requisite fsnotify calls.

Just curious, isn't the fact that the open still may fail a problem? For example, if the lib is not executable in sys_uselib or if the file was denied write access in open_exec.

I could be convinced to change it but like what I have. At this point the file is open. If it fails after this we are going to call fput() which calls __fput() which calls fsnotify_close(). Seemed odd to leave a close without an open (although that's what we have today.)

This doesn't mean the syscall was successful, but the file was opened, and it will be closed....

-Eric

#3 lore raw
Quoted 5 lines · Eric Paris
Quoted 3 lines · Frederik Deweerdt

Just curious, isn't the fact that the open still may fail a problem? For example, if the lib is not executable in sys_uselib or if the file was denied write access in open_exec.

Quoted 7 lines

I could be convinced to change it but like what I have. At this point the file is open. If it fails after this we are going to call fput() which calls __fput() which calls fsnotify_close(). Seemed odd to leave a close without an open (although that's what we have today.)

This doesn't mean the syscall was successful, but the file was opened, and it will be closed....

OK, thanks for clarifying,

Frederik

#4 lore raw
In reply to Eric Paris
Quoted 3 lines · On Wed, 17 Dec 2008 13:53:20 -0500 Eric Paris <[email protected]> wrote:

sys_execve and sys_uselib do not call into fsnotify so inotify does not get open events for these types of syscalls. This patch simply makes the requisite fsnotify calls.

These two functions are grovelling around in pretty low-level fs operations. One wonders whether they could be converted (ie: cleaned up) to use do_sys_open() or some other such higher-level thing.

That way, this fsnotify bug would be magically fixed, too.

#5 lore raw
Quoted 7 lines
Quoted 3 lines · On Wed, 17 Dec 2008 13:53:20 -0500 Eric Paris <[email protected]> wrote:

sys_execve and sys_uselib do not call into fsnotify so inotify does not get open events for these types of syscalls. This patch simply makes the requisite fsnotify calls.

Quoted 5 lines

These two functions are grovelling around in pretty low-level fs operations. One wonders whether they could be converted (ie: cleaned up) to use do_sys_open() or some other such higher-level thing.

That way, this fsnotify bug would be magically fixed, too.

hmm..

In current implementaion, do_sys_open() call fd_install(). then, it cause /proc/{pid}/fd entry change. iow, compatibility break.

Yup, it is fixable. but I think Eric's implementation is simpler.

Reviewed-by on v1 1/1
#6 lore raw
In reply to Eric Paris
Quoted 5 lines

sys_execve and sys_uselib do not call into fsnotify so inotify does not get open events for these types of syscalls. This patch simply makes the requisite fsnotify calls.

I hope append your latter explain into the patch description.

Quoted 7 lines

I could be convinced to change it but like what I have. At this point the file is open. If it fails after this we are going to call fput() which calls __fput() which calls fsnotify_close(). Seemed odd to leave a close without an open (although that's what we have today.)

This doesn't mean the syscall was successful, but the file was opened, and it will be closed....

I think open/close pairing is important thing to this patch.

Otherthings, looks good to me. very thanks.