Re: KAUTH_GENERIC_CANSEE

[Elad Efrat <elad%NetBSD.org@localhost>]

Hi,

Attached is a patch to do what David suggested for one of the two
instances. It's abusing the f_unused1 field to do the marking, and
uses a static "seq" variable... I'm sure someone will come up with
a better way of doing the same thing.

Once we've agreed on the approach I'll adjust the other instance
and post a complete diff.

Please review. FWIW, I've written a kernel module using both methods
to test the changes and they seem fine. I'm also running with this
change locally and pstat seems fine as well.

Thanks,

-e.

Index: init_sysctl.c
===================================================================
RCS file: /cvsroot/src/sys/kern/init_sysctl.c,v
retrieving revision 1.168
diff -u -p -r1.168 init_sysctl.c
--- init_sysctl.c       21 Oct 2009 21:12:06 -0000      1.168
+++ init_sysctl.c       15 Nov 2009 08:17:43 -0000
@@ -1877,7 +1877,7 @@ static int
 sysctl_kern_file2(SYSCTLFN_ARGS)
 {
        struct proc *p;
-       struct file *fp, *tp, *np;
+       struct file *fp;
        struct filedesc *fd;
        struct kinfo_file kf;
        char *dp;
@@ -1886,6 +1886,7 @@ sysctl_kern_file2(SYSCTLFN_ARGS)
        int error, arg, elem_count;
        fdfile_t *ff;
        fdtab_t *dt;
+       static u_int seq = 0;
 
        if (namelen == 1 && name[0] == CTL_QUERY)
                return (sysctl_query(SYSCTLFN_CALL(rnode)));
@@ -1908,65 +1909,21 @@ sysctl_kern_file2(SYSCTLFN_ARGS)
 
        switch (op) {
        case KERN_FILE_BYFILE:
+       case KERN_FILE_BYPID:
                /*
-                * doesn't use arg so it must be zero
-                */
-               if (arg != 0)
-                       return (EINVAL);
-               sysctl_unlock();
-               /*
-                * allocate dummy file descriptor to make position in list
+                * We're traversing the process list in both cases; the BYFILE
+                * case does additional work of keeping track of files already
+                * looked at.
                 */
-               if ((tp = fgetdummy()) == NULL) {
-                       sysctl_relock();
-                       return ENOMEM;
-               }
-               mutex_enter(&filelist_lock);
-               for (fp = LIST_FIRST(&filehead); fp != NULL; fp = np) {
-                       np = LIST_NEXT(fp, f_list);
-                       mutex_enter(&fp->f_lock);
-                       if (fp->f_count == 0) {
-                               mutex_exit(&fp->f_lock);
-                               continue;
-                       }
-                       /*
-                        * XXX Need to prevent that from being an alternative
-                        * XXX way for getting process information.
-                        */
-                       if (kauth_authorize_generic(l->l_cred,
-                           KAUTH_GENERIC_CANSEE, fp->f_cred) != 0) {
-                               mutex_exit(&fp->f_lock);
-                               continue;
-                       }
-                       if (len >= elem_size && elem_count > 0) {
-                               fill_file(&kf, fp, NULL, 0, 0);
-                               LIST_INSERT_AFTER(fp, tp, f_list);
-                               mutex_exit(&fp->f_lock);
-                               mutex_exit(&filelist_lock);
-                               error = dcopyout(l, &kf, dp, out_size);
-                               mutex_enter(&filelist_lock);
-                               np = LIST_NEXT(tp, f_list);
-                               LIST_REMOVE(tp, f_list);
-                               if (error) {
-                                       break;
-                               }
-                               dp += elem_size;
-                               len -= elem_size;
-                       } else {
-                               mutex_exit(&fp->f_lock);
-                       }
-                       needed += elem_size;
-                       if (elem_count > 0 && elem_count != INT_MAX)
-                               elem_count--;
-               }
-               mutex_exit(&filelist_lock);
-               fputdummy(tp);
-               sysctl_relock();
-               break;
-       case KERN_FILE_BYPID:
-               if (arg < -1)
+
+               /* doesn't use arg so it must be zero */
+               if ((op == KERN_FILE_BYFILE) && (arg != 0))
+                       return EINVAL;
+
+               if ((op == KERN_FILE_BYPID) && (arg < -1))
                        /* -1 means all processes */
                        return (EINVAL);
+
                sysctl_unlock();
                mutex_enter(proc_lock);
                PROCLIST_FOREACH(p, &allproc) {
@@ -2013,6 +1970,10 @@ sysctl_kern_file2(SYSCTLFN_ARGS)
                                if ((fp = ff->ff_file) == NULL) {
                                        continue;
                                }
+                               if ((op == KERN_FILE_BYFILE) &&
+                                   (fp->f_unused1 == seq)) {
+                                       continue;
+                               }
                                if (len >= elem_size && elem_count > 0) {
                                        mutex_enter(&fp->f_lock);
                                        fill_file(&kf, fp, ff, i, p->p_pid);
@@ -2025,6 +1986,8 @@ sysctl_kern_file2(SYSCTLFN_ARGS)
                                        dp += elem_size;
                                        len -= elem_size;
                                }
+                               if (op == KERN_FILE_BYFILE)
+                                       fp->f_unused1 = seq;
                                needed += elem_size;
                                if (elem_count > 0 && elem_count != INT_MAX)
                                        elem_count--;
@@ -2037,6 +2000,8 @@ sysctl_kern_file2(SYSCTLFN_ARGS)
                        mutex_enter(proc_lock);
                        rw_exit(&p->p_reflock);
                }
+               if (op == KERN_FILE_BYFILE)
+                       seq++;
                mutex_exit(proc_lock);
                sysctl_relock();
                break;