This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: getifaddrs returns -1 with errno == 0, fix


On Tue, Sep 17, 2013 at 07:52:04PM -0300, Alexandre Oliva wrote:
> On Feb  4, 2013, Alexandre Oliva <aoliva@redhat.com> wrote:
> 
> > postfix was misbehaving on my yeeloong (just upgraded to Parabola
> > GNU/Linux-blire) when I connected my freerunner to it.  It printed
> > âfatal: inet_addr_local[getifaddrs]: getifaddrs: Successâ and failed to
> > start daemons, to send messages, to print the mail queue, everything.
> > Without the freerunner, it worked just fine.
> 
> > It turned out that getifaddrs() was indeed returning -1 without setting
> > errno, because the buffer allocated by __netlink_request wasn't large
> > enough to hold the response.  The received message was marked as
> > truncated, and we bailed out(_fail) without setting any errors.
> 
> > Rather than just fail, I experimented with growing the buffer and
> > retrying.  I found out I had to consume the rest of the response to the
> > earlier request first, so I grew the buffer by just the right amount,
> > and that worked fine.
> 
> > As an afterthought, I wondered if it would make more sense to just paste
> > the subsequent msgs into a larger buffer and be done with it, rather
> > than re-issuing the request.  I don't know enough about netlink to tell
> > whether that's guaranteed to get the correct answer.  Indeed, I'm not
> > even sure it's safe to consume the pending message fragments like I did.
> 
> > Thoughts?  Comments?  Should I go with this, or grow the buffer
> > accummulating remaining fragments?

This should have a BZ.

>         * sysdeps/unix/sysv/linux/ifaddrs.c (__netlink_request): Grow
>         buffer and retry when message is fragmented.
> ---
>  sysdeps/unix/sysv/linux/ifaddrs.c |   23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/ifaddrs.c b/sysdeps/unix/sysv/linux/ifaddrs.c
> index 89fda15..c9decc3 100644
> --- a/sysdeps/unix/sysv/linux/ifaddrs.c
> +++ b/sysdeps/unix/sysv/linux/ifaddrs.c
> @@ -135,13 +135,14 @@ __netlink_request (struct netlink_handle *h, int type)
>  #ifdef PAGE_SIZE
>    /* Help the compiler optimize out the malloc call if PAGE_SIZE
>       is constant and smaller or equal to PTHREAD_STACK_MIN/4.  */
> -  const size_t buf_size = PAGE_SIZE;
> +  size_t buf_size = PAGE_SIZE;
>  #else
> -  const size_t buf_size = __getpagesize ();
> +  size_t buf_size = __getpagesize ();
>  #endif
>    bool use_malloc = false;
>    char *buf;
>  
> + retry:
>    if (__libc_use_alloca (buf_size))
>      buf = alloca (buf_size);
>    else
> @@ -176,7 +177,23 @@ __netlink_request (struct netlink_handle *h, int type)
>  	continue;
>  
>        if (__builtin_expect (msg.msg_flags & MSG_TRUNC, 0))
> -	goto out_fail;
> +	{
> +	  /* Consume the rest of the truncated response, then retry
> +	     with a larger buffer.  */
> +	  do
> +	    {
> +	      read_len = TEMP_FAILURE_RETRY (__recvmsg (h->fd, &msg, 0));
> +	      if (read_len < 0)
> +		break;
> +	      buf_size += read_len;

This probably doesn't work the way you think it does.  If MSG_TRUNC is
set, all that recvmsg is doing is returning buf_size.  So essentially,
you're adding buf_size for every packet in the queue and you're not
getting the actual size of the failed datagram since the remaining
portion of the datagram is supposed to have been discarded.

Instead, if we use recv() with MSG_TRUNC as an input flag instead of
recvmsg, I believe we can get the actual size of the packet and then
retry with a size larger than that to account for cases where
interfaces may have been added in the interim.  We're not really using
anything in the msg structure, so I don't see any reason to use it
over recv().

> +	    }
> +	  while ((msg.msg_flags & MSG_TRUNC) != 0);
> +
> +	  if (use_malloc)
> +	    free (buf);
> +	  goto retry;
> +	}
> +
>  
>        size_t count = 0;
>        size_t remaining_len = read_len;
> 
> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer

Attachment: pgp4bPHntxFRZ.pgp
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]