Bug#102437: dealloc analysis for each server function, more bugs!

[Marcus Brinkmann]

On Wed, Jun 27, 2001 at 02:52:17PM +0200, Marcus Brinkmann wrote:

[auth_getids]
 
> This one is questionable.  auth_getids uses idvec_copyout, which provides
> the internal buffer:
> 
> inline void idvec_copyout (struct idvec *idvec, uid_t **ids, uid_t *nids)
> {
>   if (idvec->num > *nids)
>     *ids = idvec->ids;
>   else
>     memcpy (*ids, idvec->ids, idvec->num * sizeof *ids);
>   *nids = idvec->num;
> }
> 
> So, this should not use dealloc.  However, idvec->ids is NOT a mmap'ed
> buffer!  It is a malloced/realloced buffer (coming from
> libshouldbeinlibc/idvec.c).  Also, the code should not use
> sizeof *ids, but sizeof (uid_t) = sizeof **ids for the calculation!
> 
> Is it correct that this should better use iohelp_return_malloced_buffer
> and dealloc or mmap in idvec.h for the performance?

I would suggest dealloc + iohelp_return_malloced_buffer, as usually those
uid vectors are small, and keeping them on one page each is a waste of
space.
 
> >   routine auth_server_authenticate (

[...]

> Again, I am not sure.  The server routine has:
> 
>   /* Extract the ids.  We must use a separate reply stub so
>      we can deref the user auth handle after the reply uses its
>      contents.  */
>   auth_server_authenticate_reply (reply, reply_type, 0,
>                                   user->euids.ids, user->euids.num,
>                                   user->auids.ids, user->auids.num,
>                                   user->egids.ids, user->egids.num,
>                                   user->agids.ids, user->agids.num);
>   ports_port_deref (user);
> 
> Because those arrays are not marked as dealloc, the pointer is copied by
> the reply stub.  Again, we are returning malloc'ed buffers.  But this time
> we are returning buffers without keeping a reference to them!  The comment
> is actually useless, as the buffers are not copied.  Am I incorrect, or is
> this a blatant error?

I was incorrect, I think.  Small uid vectors are copied in the message
inlined, while out-of-band memory is logically copied.  Although the
physical page is shared, it will not be removed unless the server and client
both called vm_deallocate on it (if I understood this correctly).
 
> It seems that using return-buffer and dealloc is the only chance, if we
> don't want to giver the server a reference to the user.

Not in this case.  But again, because we use malloc for the uids, I'd
suggest to use return-buffer and dealloc anyway.

I will make an appropriate patch and post it here.

Thanks,
Marcus



-- 
`Rhubarb is no Egyptian god.' Debian http://www.debian.org brinkmd@debian.org
Marcus Brinkmann              GNU    http://www.gnu.org    marcus@gnu.org
Marcus.Brinkmann@ruhr-uni-bochum.de
http://www.marcus-brinkmann.de