Date: Sun, 22 Nov 2020 11:26:11 -0600 From: Kyle Evans <kevans@freebsd.org> Cc: Guy Yur <guyyur@gmail.com>, Robert Wing <rew@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r367927 - in head: sys/kern tests/sys/kern Message-ID: <CACNAnaFdxzULk_n%2BOz81qUNXUMG7pHsyLNvYpP%2Bso7T136iVMg@mail.gmail.com> In-Reply-To: <CACNAnaEFXz69MVPfcfwabAWOaXeLbg2MBjHChj11-ABXOtbDxg@mail.gmail.com> References: <202011220500.0AM50Tml047660@repo.freebsd.org> <75a09790-79d8-f539-e118-acf32b6bc65e@gmail.com> <CACNAnaEM3e7Pu4hKm7aLB52E-STzju0A9gA7VA-RVoOjFnu0ew@mail.gmail.com> <CACNAnaEFXz69MVPfcfwabAWOaXeLbg2MBjHChj11-ABXOtbDxg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Nov 22, 2020 at 11:25 AM Kyle Evans <kevans@freebsd.org> wrote: > > On Sun, Nov 22, 2020 at 10:36 AM Kyle Evans <kevans@freebsd.org> wrote: > > > > On Sun, Nov 22, 2020 at 9:54 AM Guy Yur <guyyur@gmail.com> wrote: > > > > > > On 22/11/20 7:00 am, Robert Wing wrote: > > > > Author: rew > > > > Date: Sun Nov 22 05:00:28 2020 > > > > New Revision: 367927 > > > > URL: https://svnweb.freebsd.org/changeset/base/367927 > > > > > > > > Log: > > > > fd: free old file descriptor tables when not shared > > > > > > > > During the life of a process, new file descriptor tables may be = allocated. When > > > > a new table is allocated, the old table is placed in a free list= and held onto > > > > until all processes referencing them exit. > > > > > > > > When a new file descriptor table is allocated, the old file desc= riptor table > > > > can be freed when the current process has a single-thread and th= e file > > > > descriptor table is not being shared with any other processes. > > > > > > > > Reviewed by: kevans > > > > Approved by: kevans (mentor) > > > > Differential Revision: https://reviews.freebsd.org/D18617 > > > > > > > > Added: > > > > head/tests/sys/kern/fdgrowtable_test.c (contents, props change= d) > > > > Modified: > > > > head/sys/kern/kern_descrip.c > > > > head/tests/sys/kern/Makefile > > > > > > Hi, > > > > > > I am getting a kernel panic with this commit when building > > > devel/gmake port and it runs dup2 test in configure script. > > > > > > panic: fc_ioctls !=3D NULL, but fc_nioctls=3D-16162 > > > ... > > > #10 0xffffffff80655c72 in vpanic (fmt=3D<optimized out>, ap=3D<optimi= zed out>) > > > at /usr/src/sys/kern/kern_shutdown.c:907 > > > #11 0xffffffff80655a03 in panic ( > > > fmt=3D0xffffffff80eb2b78 <cnputs_mtx> "=ED=97=9D\200\377\377\377= \377") > > > at /usr/src/sys/kern/kern_shutdown.c:843 > > > #12 0xffffffff805fff9a in filecaps_copy_prep (src=3D<optimized out>) > > > at /usr/src/sys/kern/kern_descrip.c:1629 > > > #13 kern_dup (td=3D<optimized out>, mode=3D<optimized out>, flags=3D0= , > > > old=3D<optimized out>, new=3D256) at /usr/src/sys/kern/kern_desc= rip.c:970 > > > #14 0xffffffff8094a5de in syscallenter (td=3D<optimized out>) > > > at /usr/src/sys/amd64/amd64/../../kern/subr_syscall.c:189 > > > #15 amd64_syscall (td=3D0xfffffe00513f8500, traced=3D0) > > > at /usr/src/sys/amd64/amd64/trap.c:1156 > > > > > > > > > Simplified test program that causes panic: > > > #include <unistd.h> > > > #include <limits.h> > > > > > > int main () > > > { > > > int bad_fd =3D INT_MAX; > > > dup2 (1, 1); > > > close (0); > > > dup2 (0, 0); > > > dup2 (2, bad_fd); > > > dup2 (2, -1); > > > dup2 (2, 255); > > > dup2 (2, 256); > > > return 0; > > > } > > > > > > > Whoops. =3D\ > > > > It looks like kern_dup grows the file table but assumes that it can > > continue using oldfe that it fetched from the now-freed table. I > > suspect we just need to refetch oldfde after the grow operation, and > > it might be a good idea (under INVARIANTS) to grab the fp from oldfde > > before we grow the table and assert that the new entry we fetch is the > > same underlying file. > > > > I can confirm that the below fixes it and no other growth spots keep > pointers into the old table around, I'll give it a little bit for any > objections to be raised then commit. > Bah, sorry, this still isn't right. The other paths may grow the table via fdalloc(). I'll throw up a review for this shortly.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACNAnaFdxzULk_n%2BOz81qUNXUMG7pHsyLNvYpP%2Bso7T136iVMg>