svn commit: r184762 - head/sys/netgraph

[Kostik Belousov]

On Sat, Nov 08, 2008 at 02:13:46PM +0100, Attilio Rao wrote:
> 2008/11/8, Alexander Motin <mav at freebsd.org>:
> > Author: mav
> >  Date: Sat Nov  8 06:25:57 2008
> >  New Revision: 184762
> >  URL: http://svn.freebsd.org/changeset/base/184762
> >
> >  Log:
> >   Don't use curthread to resolve file descriptor. Request may be queued, so
> >   thread will be different. Instead require sender to send process ID
> >   together with file descriptor.
> >
> >  Modified:
> >   head/sys/netgraph/ng_tty.c
> >
> >  Modified: head/sys/netgraph/ng_tty.c
> >  ==============================================================================
> >  --- head/sys/netgraph/ng_tty.c  Sat Nov  8 04:43:24 2008        (r184761)
> >  +++ head/sys/netgraph/ng_tty.c  Sat Nov  8 06:25:57 2008        (r184762)
> >  @@ -73,6 +73,7 @@
> >   #include <sys/syslog.h>
> >   #include <sys/tty.h>
> >   #include <sys/ttycom.h>
> >  +#include <sys/proc.h>
> >
> >   #include <net/if.h>
> >   #include <net/if_var.h>
> >  @@ -250,7 +251,8 @@ ngt_shutdown(node_p node)
> >   static int
> >   ngt_rcvmsg(node_p node, item_p item, hook_p lasthook)
> >   {
> >  -       struct thread *td = curthread;  /* XXX */
> >  +       struct proc *p;
> >  +       struct thread *td;
> >         const sc_p sc = NG_NODE_PRIVATE(node);
> >         struct ng_mesg *msg, *resp = NULL;
> >         int error = 0;
> >  @@ -262,8 +264,14 @@ ngt_rcvmsg(node_p node, item_p item, hoo
> >                 case NGM_TTY_SET_TTY:
> >                         if (sc->tp != NULL)
> >                                 return (EBUSY);
> >  -                       error = ttyhook_register(&sc->tp, td, *(int *)msg->data,
> >  +
> >  +                       p = pfind(((int *)msg->data)[0]);
> >  +                       if (p == NULL)
> >  +                               return (ESRCH);
> >  +                       td = FIRST_THREAD_IN_PROC(p);
> >  +                       error = ttyhook_register(&sc->tp, td, ((int *)msg->data)[1],
> >                             &ngt_hook, sc);
> >  +                       PROC_UNLOCK(p);
> >                         if (error != 0)
> >                                 return (error);
> >                         break;
> 
> The threads iterator in strcut proc should be proc_slock protected, so
> you need to grab/release it around FIRST_THREAD_IN_PROC().

I do not believe that process slock is needed there, since code
just dereference a pointer. The process lock seems to be taken to
guarantee that td is alive.

On the other hand, ttyhook_register locks filedesc sx, that causes
sleepable lock acquition after non-sleepable. I believe this is
an actual problem with the change.

Probably, this is even the problem with KPI of ttyhook_register, since
caller need to somehow protect td from going away, and proc lock is
a right lock to held.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-all/attachments/20081108/7abf2c7f/attachment.pgp