On Thu, Dec 3, 2009 at 11:34 PM, Vladimir Kirillov <pro...@uaoug.org.ua>
wrote:
> Here's a diff for tun to make it keep record and display the
> pid of tunnel owner process in ifconfig.
> I'm sure it would be a handy feature, hope you'll like it.
....
> @@ -88,16 +88,17 @@
>  #include <net/if_tun.h>
>
>  struct tun_softc {
> -       struct arpcom   arpcom;         /* ethernet common data */
> -       struct selinfo  tun_rsel;       /* read select */
> -       struct selinfo  tun_wsel;       /* write select (not used) */
> +       struct arpcom    arpcom;        /* ethernet common data */
> +       struct selinfo   tun_rsel;      /* read select */
> +       struct selinfo   tun_wsel;      /* write select (not used) */

Please avoid including whitespace changes along with substantive changes.


> @@ -372,6 +374,7 @@ tunopen(dev_t dev, int flag, int mode, s
>        s = splnet();
>        ifp->if_flags |= IFF_RUNNING;
>        tun_link_state(tp);
> +       tp->tun_proc = p;

Ick, holding a pointer to a struct proc from inside a driver?!  What
happens when the process forks and the parent exits?  The file will
still be open in the child, so the device close routine won't have
been called and you'll have a dangling pointer, no?  (Doesn't ppp do
exactly that open/fork/exit routine?)

I suspect this should instead follow the lead of fcntl(F_[SG]ETOWN)
and save the pid, though that still has fun if the process exits and
the pid is reused.  What do you think the correct semantics are if the
original process exits?


> +               tunr->tun_pid = tp->tun_proc->p_pid;

This is wrong if the process is using rthreads.  It should be
tp->tun_proc->p_p->ps_mainproc->p_pid


Philip Guenther

Reply via email to