Date: Sat, 17 Feb 2018 16:23:40 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Mariusz Zaborski <oshogbo@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: Where KASASERT fd < fdp->fd_nfiles should be? Message-ID: <CAGudoHHikxaGiB4NvADDGubkXuHLR8rOys2hUvcOd%2BZ0K22=Tw@mail.gmail.com> In-Reply-To: <20180217150224.GA61118@x-wing> References: <20180217150224.GA61118@x-wing>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Feb 17, 2018 at 04:02:24PM +0100, Mariusz Zaborski wrote:
>
> KASSERT(fd >= 0 && fd < fdp->fd_nfiles,
> ("%s: invalid fd=%d", __func__, fd));
>
>
> My question and problem is do we need this KASSERT?
> The fdget_locked checks if the fd is not larger then fd_lastfile.
> But the code from fdinit suggest that fd_lastfile can be larger then
fd_nfiles.
> pjd@ suggested that it can go over size of the table fd_ofiles array:
> while (fdp->fd_lastfile >= newfdp->fd_nfiles) {
> FILEDESC_SUNLOCK(fdp);
> fdgrowtable(newfdp, fdp->fd_lastfile + 1);
> FILEDESC_SLOCK(fdp);
> }
>
> So the question is do we need this assertion here or maybe should we move
it to
> the fget_locked()/fdget_locked() functions?
>
While the assertion arguably can be removed, it is most definitely
valid.
fd_nfiles signifies the size of the table, while fd_lastfile the highest
used slot. By definition it must fit the table.
The code sample is used on fork where the existing table is duplicated.
Allocation of the new table is performed with locks dropped, which means
the old one can grow in the meantime. The while loop ensures the new
table will have the right size no matter what.
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHHikxaGiB4NvADDGubkXuHLR8rOys2hUvcOd%2BZ0K22=Tw>