On Tue, Jun 01, 2021 at 09:20:04AM -0400, Mark Johnston wrote:
> On Tue, Jun 01, 2021 at 12:44:30PM +0000, Dmitry Chagin wrote:
> > On Thu, May 27, 2021 at 07:53:23PM +0000, Mark Johnston wrote:
> > > The branch main has been updated by markj:
> > > 
> > > URL: 
> > > https://cgit.FreeBSD.org/src/commit/?id=f3851b235b23d9220ace31bbc89b1fe0a78fc75c
> > > 
> > > commit f3851b235b23d9220ace31bbc89b1fe0a78fc75c
> > > Author:     Mark Johnston <[email protected]>
> > > AuthorDate: 2021-05-27 19:49:59 +0000
> > > Commit:     Mark Johnston <[email protected]>
> > > CommitDate: 2021-05-27 19:52:20 +0000
> > > 
> > >     ktrace: Fix a race with fork()
> > >     
> > >     ktrace(2) may toggle trace points in any of
> > >     1. a single process
> > >     2. all members of a process group
> > >     3. all descendents of the processes in 1 or 2
> > >     
> > >     In the first two cases, we do not permit the operation if the process 
> > > is
> > >     being forked or not visible. However, in case 3 we did not enforce 
> > > this
> > >     restriction for descendents. As a result, the assertions about the 
> > > child
> > >     in ktrprocfork() may be violated.
> > >     
> > >     Move these checks into ktrops() so that they are applied consistently.
> > >     
> > >     Allow KTROP_CLEAR for nascent processes. Otherwise, there is a window
> > >     where we cannot clear trace points for a nascent child if they are
> > >     inherited from the parent.
> > >     
> > >     Reported by:    [email protected]
> > >     Reported by:    [email protected]
> > >     Reviewed by:    kib
> > >     MFC after:      1 week
> > >     Sponsored by:   The FreeBSD Foundation
> > >     Differential Revision:  https://reviews.freebsd.org/D30481
> > > ---
> > >  sys/kern/kern_ktrace.c | 43 ++++++++++++++++++++++---------------------
> > >  1 file changed, 22 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/sys/kern/kern_ktrace.c b/sys/kern/kern_ktrace.c
> > > index dc064d9ebd67..4a2dad20b035 100644
> > > --- a/sys/kern/kern_ktrace.c
> > > +++ b/sys/kern/kern_ktrace.c
> > > @@ -1006,7 +1006,7 @@ sys_ktrace(struct thread *td, struct ktrace_args 
> > > *uap)
> > >   int facs = uap->facs & ~KTRFAC_ROOT;
> > >   int ops = KTROP(uap->ops);
> > >   int descend = uap->ops & KTRFLAG_DESCEND;
> > > - int nfound, ret = 0;
> > > + int ret = 0;
> > >   int flags, error = 0;
> > >   struct nameidata nd;
> > >   struct ktr_io_params *kiop, *old_kiop;
> > > @@ -1080,42 +1080,31 @@ restart:
> > >                   error = ESRCH;
> > >                   goto done;
> > >           }
> > > +
> > >           /*
> > >            * ktrops() may call vrele(). Lock pg_members
> > >            * by the proctree_lock rather than pg_mtx.
> > >            */
> > >           PGRP_UNLOCK(pg);
> > > -         nfound = 0;
> > > +         if (LIST_EMPTY(&pg->pg_members)) {
> > > +                 sx_sunlock(&proctree_lock);
> > > +                 error = ESRCH;
> > > +                 goto done;
> > > +         }
> > >           LIST_FOREACH(p, &pg->pg_members, p_pglist) {
> > >                   PROC_LOCK(p);
> > > -                 if (p->p_state == PRS_NEW ||
> > > -                     p_cansee(td, p) != 0) {
> > > -                         PROC_UNLOCK(p); 
> > > -                         continue;
> > > -                 }
> > > -                 nfound++;
> > >                   if (descend)
> > >                           ret |= ktrsetchildren(td, p, ops, facs, kiop);
> > >                   else
> > >                           ret |= ktrops(td, p, ops, facs, kiop);
> > >           }
> > > -         if (nfound == 0) {
> > > -                 sx_sunlock(&proctree_lock);
> > > -                 error = ESRCH;
> > > -                 goto done;
> > > -         }
> > >   } else {
> > >           /*
> > >            * by pid
> > >            */
> > >           p = pfind(uap->pid);
> > > -         if (p == NULL)
> > > +         if (p == NULL) {
> > >                   error = ESRCH;
> > > -         else
> > > -                 error = p_cansee(td, p);
> > > -         if (error) {
> > > -                 if (p != NULL)
> > > -                         PROC_UNLOCK(p);
> > >                   sx_sunlock(&proctree_lock);
> > >                   goto done;
> > >           }
> > > @@ -1187,8 +1176,20 @@ ktrops(struct thread *td, struct proc *p, int ops, 
> > > int facs,
> > >           PROC_UNLOCK(p);
> > >           return (0);
> > >   }
> > > - if (p->p_flag & P_WEXIT) {
> > > -         /* If the process is exiting, just ignore it. */
> > > + if ((ops == KTROP_SET && p->p_state == PRS_NEW) || !p_cansee(td, p)) {
> > 
> >                     ^^^^^^^^^^^^^^ seems that it broke ktrace:
> >                     root@mordor:~ # ktrace ls
> >                     ktrace: ktrace.out: Operation not permitted
> 
> Indeed, I pushed a fix to main.  Sorry for the breakage.

works now, thank you!
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all
To unsubscribe, send any mail to "[email protected]"

Reply via email to