Look great Visa!  My built kernel (with your updated patch:
https://marc.info/?l=openbsd-bugs&m=158575700107939&w=2) works without
panicing.

Thank you!

On Wed, Apr 1, 2020 at 5:02 PM Bryan Stenson <[email protected]> wrote:
>
> Gah...my script IS valid...but I only rebuilt usr.bin/ktrace/ktrace
> binary, not a new kernel...I'll do that now (with the suggested
> patch), and report back.
>
> On Wed, Apr 1, 2020 at 4:17 PM Bryan Stenson <[email protected]> wrote:
> >
> > Ok, please forgive my scripting abilities, but the following gives me
> > a kernel panic regularly (running both ktrace as shipped with my
> > install (-current; OpenBSD sv01.openbsd.amsterdam 6.6 GENERIC#85
> > amd64) and the suggested patch:
> >
> > #!/bin/ksh
> >
> > set -e
> >
> > if [[ $(id -u) -ne 0 ]]; then
> >         echo 'must be run as root'
> >         exit 1
> > fi
> >
> > LOGGER=/usr/bin/logger
> >
> > echo 'trying to cause panic with ktrace and chkquota' | logger -t $0
> >
> > # setup deadman timer
> > /bin/ksh -c "while echo 'alive' | logger -t $0 ; do sleep 1; done" &
> >
> > # trace pflogd...
> > PID_TO_TRACE=$(ps aux | grep _pflogd | awk '{print $2}')
> >
> > # only support one file-system for now (found in /etc/fstab, with userquota)
> > PATH=$(cat /etc/fstab | grep userquota | awk '{print $2}' | head -1)
> >
> > SLEEP='/bin/sleep 1'
> >
> > APP=ktrace
> > LOG_IT="${LOGGER} -t ${APP}.changer"
> > APP_PATH=/usr/obj/usr.bin/ktrace
> > APP_ON="${APP_PATH}/${APP} -p ${PID_TO_TRACE}"
> >
> > # cd to $PATH, and enable ktrace
> > /bin/ksh -c "cd ${PATH}; ${APP_ON}; echo tracing on | ${LOG_IT}"
> >
> > APP=quota
> > LOG_IT="${LOGGER} -t ${APP}.changer"
> > APP_PATH=/usr/sbin
> > APP_ON="${APP_PATH}/${APP}on -u -v ${PATH}"
> > APP_OFF="${APP_PATH}/${APP}off -v ${PATH}"
> >
> > # toggle quota
> > /bin/ksh -c "while ${SLEEP} ; do ${APP_OFF} | ${LOG_IT}; ${SLEEP} ;
> > ${APP_ON} | ${LOG_IT}; done" &
> >
> > /usr/bin/tail -f /var/log/messages
> >
> > On Wed, Apr 1, 2020 at 4:02 PM Visa Hankala <[email protected]> wrote:
> > >
> > > On Tue, Mar 31, 2020 at 10:19:36PM +0000, Bryan Stenson wrote:
> > > > I started "doas ktrace -p <pid_of_pflogd>" after boot.
> > > >
> > > > However, your clue about "...quotaon(8) could miss the trace file
> > > > vnode..." seems relevant here.  I did use "doas quotaoff /home" and
> > > > "doas quotaon -u /home" while proving to myself how the quota system
> > > > works.
> > > >
> > > > The kernel didn't immediately panic after turning quotas on/off, so I
> > > > didn't think it was related.
> > > >
> > > > I'd love to be able to help test/prove this patch, but I haven't been
> > > > able to reproduce it.  I've been running a script to
> > > > create/delete/modify files by the user with quota limits.  At the same
> > > > time, I've run another script to enable/disable quotas, hoping to
> > > > trigger the bug...but I haven't been able to reproduce at all.
> > >
> > > You should be able to reproduce the panic by starting a process with
> > > ktrace, followed by enabling of quotas on the file system where the
> > > trace file is being collected.
> > >
> > >     # start top(1) to generate system calls
> > >     ktrace -f $FS_WITH_QUOTAS/ktrace.out top
> > >
> > >     # on another console
> > >     doas quotaoff -av
> > >     doas quotaon -av
> > >
> > > Below is an updated diff that fixes a bug in the previous version.
> > >
> > > Index: kern/kern_ktrace.c
> > > ===================================================================
> > > RCS file: src/sys/kern/kern_ktrace.c,v
> > > retrieving revision 1.102
> > > diff -u -p -r1.102 kern_ktrace.c
> > > --- kern/kern_ktrace.c  23 Mar 2020 15:45:39 -0000      1.102
> > > +++ kern/kern_ktrace.c  1 Apr 2020 15:32:55 -0000
> > > @@ -83,6 +83,7 @@ ktrcleartrace(struct process *pr)
> > >                 pr->ps_tracevp = NULL;
> > >                 pr->ps_tracecred = NULL;
> > >
> > > +               vp->v_writecount--;
> > >                 vrele(vp);
> > >                 crfree(cred);
> > >         }
> > > @@ -109,6 +110,7 @@ ktrsettrace(struct process *pr, int facs
> > >
> > >         vref(newvp);
> > >         crhold(newcred);
> > > +       newvp->v_writecount++;
> > >
> > >         oldvp = pr->ps_tracevp;
> > >         oldcred = pr->ps_tracecred;
> > > @@ -117,6 +119,7 @@ ktrsettrace(struct process *pr, int facs
> > >         pr->ps_tracecred = newcred;
> > >
> > >         if (oldvp != NULL) {
> > > +               oldvp->v_writecount--;
> > >                 vrele(oldvp);
> > >                 crfree(oldcred);
> > >         }

Reply via email to