On Tue, Aug 28, 2018 at 02:49:38PM +0900, Bryan Linton wrote:
> [CCing visa@ in]
>
> On 2018-08-25 21:40:57, Tom Murphy <open...@pertho.net> wrote:
> > On Thu, Aug 23, 2018 at 08:45:54PM +0900, Tom Murphy wrote:
> > > I've narrowed it down.
> > >
> > >Last kernel where adb works: June 24 09:59:46 MDT 2018
> > >1st Kernel where adb panics: June 25 13:10:32 MDT 2018
> > >
> > > [...]
> > >
> > > I'm going to look at the commits next.
> > >
> > >-Tom
> >
> > I can verify that this commit is what makes the kernel panic when adb is
> > run and an Android device is connected to the machine with ADB enabled:
> >
> > https://marc.info/?l=openbsd-cvs&m=152996258723362&w=2
> >
> > CVSROOT: /cvs
> > Module name: src
> > Changes by: v...@cvs.openbsd.org 2018/06/25 10:06:27
> >
> > Modified files:
> > sys/kern : vfs_syscalls.c
> > lib/libc/sys : dup.2
> >
> > Log message:
> > During open(2), release the fdp lock before calling vn_open(9).
> > This lets other threads of the process modify the file descriptor
> > table even if the vn_open(9) call blocks.
> >
> > The change has an effect on dup2(2) and dup3(2). If the new descriptor
> > is the same as the one reserved by an unfinished open(2), the system
> > call will fail with error EBUSY. The accept(2) system call already
> > behaves like this.
> >
> > Issue pointed out by art@ via mpi@
> >
> > Tested in a bulk build by ajacoutot@
> > OK mpi@
> >
> > * * *
> >
> > I tested kernels compiled just before that commit and right after, and that
> > commit makes the kernel panic.
> >
>
> I can also confirm that reverting this patch fixes the kernel
> panics when launching ADB for me as well. I'm currently syncing
> my phone to my HDD as I type this.
>
> I'm still building against kernel sources from here:
> OpenBSD 6.3-current (GENERIC.MP) #163: Mon Jul 30 12:45:31 MDT 2018
> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> So fair warning, my tree is still a little bit out of date (I'm
> planning on upgrading to a newer snap this weekend if I have the
> time) but, as stated above, I can at least confirm that reverting
> this patch fixes the panics for me.
>
> --
> Bryan
Hi,
I've checked out the src from -current, and reverted the 25th June 2018 commit
(just to src/sys/kern/vfs_syscalls.c to test) and I'm able to transfer files to
and from my Android phone over USB now without any panics. This is the diff to
revert. I don't know what kind of an effect this will have overall on the kernel
as I see in the commit messages that there is some refactoring going on (if I
read
it correctly)?
Index: sys/kern/vfs_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.304
diff -u -p -u -p -r1.304 vfs_syscalls.c
--- sys/kern/vfs_syscalls.c 20 Aug 2018 16:00:22 -0000 1.304
+++ sys/kern/vfs_syscalls.c 28 Aug 2018 10:07:56 -0000
@@ -1007,8 +1007,6 @@ doopenat(struct proc *p, int fd, const c
fdplock(fdp);
if ((error = falloc(p, &fp, &indx)) != 0)
goto out;
- fdpunlock(fdp);
-
flags = FFLAGS(oflags);
if (flags & FREAD) {
ni_pledge |= PLEDGE_RPATH;
@@ -1035,7 +1033,6 @@ doopenat(struct proc *p, int fd, const c
flags &= ~O_TRUNC; /* Must do truncate ourselves */
}
if ((error = vn_open(&nd, flags, cmode)) != 0) {
- fdplock(fdp);
if (error == ENODEV &&
p->p_dupfd >= 0 && /* XXX from fdopen */
(error =
@@ -1070,7 +1067,6 @@ doopenat(struct proc *p, int fd, const c
VOP_UNLOCK(vp);
error = VOP_ADVLOCK(vp, (caddr_t)fp, F_SETLK, &lf, type);
if (error) {
- fdplock(fdp);
/* closef will vn_close the file for us. */
fdremove(fdp, indx);
closef(fp, p);
@@ -1093,7 +1089,6 @@ doopenat(struct proc *p, int fd, const c
}
if (error) {
VOP_UNLOCK(vp);
- fdplock(fdp);
/* closef will close the file for us. */
fdremove(fdp, indx);
closef(fp, p);
@@ -1102,7 +1097,6 @@ doopenat(struct proc *p, int fd, const c
}
VOP_UNLOCK(vp);
*retval = indx;
- fdplock(fdp);
fdinsert(fdp, indx, cloexec, fp);
FRELE(fp, p);
out:
Kind regards,
Tom
