sysctl_file() has 2 cases: KERN_FILE_BY_PID and KERN_FILE_BYUID.
In these cases sysctl_file() can access file descriptor table from
other processes. File descriptor table of caller process can be
accessed by other threads too. The file instances from file
descriptor table will be accessed too. So file descriptor table
and the file instances within should be protected in these cases.
The patch below adds protection to file instances only. Really,
each foreign process should be locked here, not only file
descriptor table, but not in this patch. Races between
sysctl_file() and process destruction denied by kernel lock.

Locking the whole foreach-fdp-loop is bad idea. This lock will be
too long. Really, the only place where file instance must be 
proceted is file instance check and fill_file() call. While fdp
is locked, file instance which is in fdp can not be removed, so
within fdplock() boundaries f_count increment is not required.
Current version of fd_getfile() returns unreferenced fp, so I
used it here for fp check. In the future, something like
"fd_getfile_nonref()" can be used here, but not now.
fdp->fd_nfiles can be only increased and fexpand() increases it
by smp safe way.

Index: sys/kern/kern_sysctl.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.284
diff -u -p -r1.284 kern_sysctl.c
--- sys/kern/kern_sysctl.c      28 Mar 2015 23:50:55 -0000      1.284
+++ sys/kern/kern_sysctl.c      8 May 2015 15:06:41 -0000
@@ -1288,11 +1288,25 @@ sysctl_file(int *name, u_int namelen, ch
                        if (pr->ps_tracevp)
                                FILLIT(NULL, NULL, KERN_FILE_TRACE, 
pr->ps_tracevp, pr);
                        for (i = 0; i < fdp->fd_nfiles; i++) {
-                               if ((fp = fdp->fd_ofiles[i]) == NULL)
-                                       continue;
-                               if (!FILE_IS_USABLE(fp))
-                                       continue;
-                               FILLIT(fp, fdp, i, NULL, pr);
+                               if (buflen >= elem_size && elem_count > 0) {
+                                       fdplock(fdp);
+                                       fp = fd_getfile(fdp, i);
+                                       if (fp == NULL) {
+                                               fdpunlock(fdp);
+                                               continue;
+                                       }
+                                       fill_file(kf, fp, fdp, i, NULL, NULL,
+                                           p, show_pointers);
+                                       fdpunlock(fdp);
+
+                                       error = copyout(kf, dp, outsize);
+                                       if (error)
+                                               break;
+                                       dp += elem_size;
+                                       buflen -= elem_size;
+                                       elem_count--;
+                               }
+                               needed += elem_size;
                        }
                }
                break;
@@ -1316,11 +1330,25 @@ sysctl_file(int *name, u_int namelen, ch
                        if (pr->ps_tracevp)
                                FILLIT(NULL, NULL, KERN_FILE_TRACE, 
pr->ps_tracevp, pr);
                        for (i = 0; i < fdp->fd_nfiles; i++) {
-                               if ((fp = fdp->fd_ofiles[i]) == NULL)
-                                       continue;
-                               if (!FILE_IS_USABLE(fp))
-                                       continue;
-                               FILLIT(fp, fdp, i, NULL, pr);
+                               if (buflen >= elem_size && elem_count > 0) {
+                                       fdplock(fdp);
+                                       fp = fd_getfile(fdp, i);
+                                       if (fp == NULL) {
+                                               fdpunlock(fdp);
+                                               continue;
+                                       }
+                                       fill_file(kf, fp, fdp, i, NULL, NULL,
+                                           p, show_pointers);
+                                       fdpunlock(fdp);
+
+                                       error = copyout(kf, dp, outsize);
+                                       if (error)
+                                               break;
+                                       dp += elem_size;
+                                       buflen -= elem_size;
+                                       elem_count--;
+                               }
+                               needed += elem_size;
                        }
                }
                break;

Reply via email to