svn commit: r367927 - in head: sys/kern tests/sys/kern

[Kyle Evans]

On Sun, Nov 22, 2020 at 9:54 AM Guy Yur <guyyur at 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 descriptor table
> >    can be freed when the current process has a single-thread and the 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 changed)
> > 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 != NULL, but fc_nioctls=-16162
> ...
> #10 0xffffffff80655c72 in vpanic (fmt=<optimized out>, ap=<optimized out>)
>      at /usr/src/sys/kern/kern_shutdown.c:907
> #11 0xffffffff80655a03 in panic (
>      fmt=0xffffffff80eb2b78 <cnputs_mtx> "헝\200\377\377\377\377")
>      at /usr/src/sys/kern/kern_shutdown.c:843
> #12 0xffffffff805fff9a in filecaps_copy_prep (src=<optimized out>)
>      at /usr/src/sys/kern/kern_descrip.c:1629
> #13 kern_dup (td=<optimized out>, mode=<optimized out>, flags=0,
>      old=<optimized out>, new=256) at /usr/src/sys/kern/kern_descrip.c:970
> #14 0xffffffff8094a5de in syscallenter (td=<optimized out>)
>      at /usr/src/sys/amd64/amd64/../../kern/subr_syscall.c:189
> #15 amd64_syscall (td=0xfffffe00513f8500, traced=0)
>      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 = 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. =\

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.

Thanks,

Kyle Evans