Skip to contentSkip to conversation

procfs: fix tid fdinfo

PATCH In review Jerome Marchand 4 participants 7 messages Apr 23, 2010 – Apr 26, 2010 Proc filesystem

lore
Share & tools
lore

Jerome Marchand

author
#0 lore raw

PATCH 1/1 procfs: fix tid fdinfo

Correct the file_operations struct in fdinfo entry of tid_base_stuff[].

Signed-off-by Jerome Marchand <[email protected]>
---
 base.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/proc/base.c +1 -1
index 7621db8..8418fcc 100644
@@ -2909,7 +2909,7 @@ out_no_task:
  */
 static const struct pid_entry tid_base_stuff[] = {
 	DIR("fd",        S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
-	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fd_operations),
+	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
 	REG("environ",   S_IRUSR, proc_environ_operations),
 	INF("auxv",      S_IRUSR, proc_pid_auxv),
 	ONE("status",    S_IRUGO, proc_pid_status),
#1 lore raw

fs/proc/base.c out_no_task: · line 2912

Quoted 2 lines · Jerome Marchand
fs/proc/base.c +1 -1
-	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fd_operations),
+	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),

Ick.

this is for stable@ as well

Cc [email protected]
#2 lore raw
In reply to Jerome Marchand
Quoted 12 lines · On Fri, 23 Apr 2010 17:40:07 +0200 Jerome Marchand <[email protected]> wrote:

Correct the file_operations struct in fdinfo entry of tid_base_stuff[].

Signed-off-by Jerome Marchand <[email protected]>
---
 base.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/proc/base.c +0 -0
index 7621db8..8418fcc 100644

fs/proc/base.c out_no_task: · line 2915

Quoted 8 lines
fs/proc/base.c +1 -1
@@ -2909,7 +2909,7 @@ out_no_task:
  */
 static const struct pid_entry tid_base_stuff[] = {
 	DIR("fd",        S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
-	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fd_operations),
+	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
 	REG("environ",   S_IRUSR, proc_environ_operations),
 	INF("auxv",      S_IRUSR, proc_pid_auxv),
 	ONE("status",    S_IRUGO, proc_pid_status),

hm, the code's been like that for over a year. What are the user-visible effects of the bug, and of this change?

#3 lore raw
Quoted 6 lines · Andrew Morton

On Fri, 23 Apr 2010 17:40:07 +0200

fs/proc/base.c out_no_task: · line 2912

Quoted 2 lines
fs/proc/base.c +1 -1
-	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fd_operations),
+	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),

hm, the code's been like that for over a year. What are the user-visible effects of the bug, and of this change?

/proc/*/task/*/fdinfo contains symlinks to opened files like /proc/*/fd/ which is cool bug :-)

#4 lore raw
Quoted 10 lines · Alexey Dobriyan
Quoted 6 lines · Andrew Morton

On Fri, 23 Apr 2010 17:40:07 +0200

fs/proc/base.c out_no_task: · line 2912

Quoted 2 lines
fs/proc/base.c +1 -1
-	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fd_operations),
+	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),

hm, the code's been like that for over a year. What are the user-visible effects of the bug, and of this change?

/proc/*/task/*/fdinfo contains symlinks to opened files like /proc/*/fd/ which is cool bug :-)

Yeah, at least now we can see that someone has started using this interface ;)

Jerome, thanks for spotting this!

Miklos

author
#5 lore raw
Quoted 6 lines · Miklos Szeredi
Quoted 5 lines · Alexey Dobriyan
Quoted 3 lines · Andrew Morton

On Fri, 23 Apr 2010 17:40:07 +0200

Quoted 2 lines
fs/proc/base.c +1 -1
-	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fd_operations),
+	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

hm, the code's been like that for over a year. What are the user-visible effects of the bug, and of this change?

/proc/*/task/*/fdinfo contains symlinks to opened files like /proc/*/fd/ which is cool bug :-)

Yeah, at least now we can see that someone has started using this interface ;)

AFAIK there is still nobody using it, but lsof should use it, and /proc/<pid>/task/<tid>/fd/ too. I haven't checked the code, but as it is now, it doesn't spot files open by a thread which does share its father's file descriptor table.

Quoted 6 lines

Jerome, thanks for spotting this!

Miklos

#6 lore raw
Quoted 13 lines · Jerome Marchand
Quoted 12 lines · Miklos Szeredi
Quoted 10 lines · Alexey Dobriyan
Quoted 6 lines · Andrew Morton

On Fri, 23 Apr 2010 17:40:07 +0200

fs/proc/base.c out_no_task: · line 2912

Quoted 2 lines
fs/proc/base.c +1 -1
-	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fd_operations),
+	DIR("fdinfo",    S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),

hm, the code's been like that for over a year. What are the user-visible effects of the bug, and of this change?

/proc/*/task/*/fdinfo contains symlinks to opened files like /proc/*/fd/ which is cool bug :-)

Quoted 7 lines
Quoted 2 lines

Yeah, at least now we can see that someone has started using this interface ;)

AFAIK there is still nobody using it, but lsof should use it, and /proc/<pid>/task/<tid>/fd/ too. I haven't checked the code, but as it is now, it doesn't spot files open by a thread which does share its father's file descriptor table.

Yes, apparently "lsof" doesn't care about contents of /proc/$PID/task. Well, tasks with separate file descriptor tables are rare, POSIX threads share their descriptor tables. But it would be nice if this were fixed.

Thanks,
Miklos