Where KASASERT fd < fdp->fd_nfiles should be?

[Mariusz Zaborski]

On Sat, Feb 17, 2018 at 04:23:40PM +0100, Mateusz Guzik wrote:
> 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.
So maybe we should go in more general approach and move this KASSERT to the
fget_locked and fdget_locked functions?

Thanks,
-- 
Mariusz Zaborski
oshogbo//vx		| http://oshogbo.vexillium.org
FreeBSD commiter	| https://freebsd.org
Software developer	| http://wheelsystems.com
If it's not broken, let's fix it till it is!!1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20180217/11c89caf/attachment.sig>