Date: Fri, 8 Jan 2016 16:52:28 +0800 (PHT)
   From: Paul Goyette <p...@whooppee.com>

   Robert Elz wrote:

   > That is, instead of a fd_getfile() without an (almost immediate)
   > fd_putfile() so keeping ff_refcount incremented, in filemon, just do
   >
   >        fp = fd_getfile(...);
   >        fp->f_count++;
   >        fd_putfile(...);
   >
   > so filemon has a proper reference to the file*.   When it is done, it
   > just closes it, like any other file would be closed (which decrements
   > f_count and frees the struct file if it goes to 0).

   Option 4 works!  See attached diffs.  Thanks, kre!

This is essentially the same as fd_getfile2, except with a lot more
logic involved, including fiddly details that you have to take care
of.  And...

   I'll commit this in a day or two unless I receive some extremely 
   negative feedback.

   @@ -301,14 +301,16 @@ filemon_ioctl(struct file * fp, u_long c
   [...]
   -               filemon->fm_fd = *((int *) data);
   -               if ((filemon->fm_fp = fd_getfile(filemon->fm_fd)) == NULL) {
   +               fd = *((int *) data);
   +               if ((filemon->fm_fp = fd_getfile(fd)) == NULL) {
                           error = EBADF;
                           break;
                   }
   +               filemon->fm_fp->f_count++;
   +               fd_putfile(fd);

...you missed one: you can't touch fp->f_count without holding
fp->f_lock.

So I reiterate my suggestion to use fd_getfile2:

        fd = *((int *)data);
        if ((filemon->fm_fp = fd_getfile2(curproc, fd)) == NULL) {
                error = EBADF;
                break;
        }

Reply via email to