Re: 5.3-RELEASE TODO

On Fri, Sep 17, 2004 at 07:57:32PM +0200, Dag-Erling Smørgrav wrote:
> Brooks Davis <brooks_at_one-eyed-alien.net> writes:
> > I've got a patch for this one.  If someone will review it, I will commit
> > it to HEAD for further testing.
> 
> you shouldn't call malloc() with M_NOWAIT unless you absolutely must,
> and in this case you don't.

Oops, that's a bug.  I'll fix it before I commit.

> there's nothing to prevent the ifconf data from growing between the
> first and second pass, since you're not holding the lock.  you'd end
> up overflowing ifrbuf.

The space variable insures that you won't overflow just as it did before
when the user provided an undersized buffer.  There is a race that
might be worth fixing where the size can increase and more or more
addresses/interfaces at the end won't get written out, but there isn't
an overflow.  The only way I can see to fix it is to check at the end
of the loop and make sure that we added all items in the buffer or used
as much space as the user gave us.  If didn't do that, we need to start
over (zero buflen, reset space, free ifrbuf, and goto again).  Using
sbufs as you suggest would probably make things a bit more clear, but I
believe what I've got works.

> what I suggest you do is:
> 
>     struct sbuf *sb = sbuf_new(NULL, NULL, size, ifc->ifc_len + 1);

What are you trying to do here?  Unless my manpages are wrong, the
fourth arg is flags.  Do you mean to set SBUF_FIXEDLEN?  I think you
would have to to avoid a new LOR.  Also, it is not safe to trust
ifc->ifc_len for allocations because it is provided by potentially
unpriveleged users.  Thus, so you have to know how much space you will
need before doing any kind of allocation, hence the double loop and the
potential race.

Thanks,
Brooks

-- 
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529  9BF0 5D8E 8BE9 F238 1AD4