I'm not sure. The last sigprocmask call restores the original signal mask of 
the thread. 
It looks kind of pointless since the two blocked signals are unblocked by the 
sigprocmask(SIG_UNBLOCK) call in the next loop iteration.

The comment added in the iputils code talks about signal mask inheritance. In 
my case arping was called via system() from a boost io_service thread for which 
most signals were blocked. At least the SIG_UNBLOCK before recvfrom makes sense 
in this case.

Since I don't know the consequences of removing that last sigprocmask() call I 
would let it stay like in the iputils arping at GitHub. 

-----Original Message-----
From: Denys Vlasenko [mailto:vda.li...@googlemail.com] 
Sent: Sunday, February 11, 2018 1:49 PM
To: Ortmann, Michael <m.ortm...@lxinstruments.com>
Cc: busybox@busybox.net
Subject: Re: [PATCH] Prevent arping from freezing when no packet is received

On Fri, Feb 9, 2018 at 12:52 PM, Ortmann, Michael <m.ortm...@lxinstruments.com> 
wrote:
> Hello,
>
>
>
> I ran into a case where a call like “arping -D -c 2 -I eth0 10.0.0.64” 
> would freeze forever. It is caused by a blocking recfrom call at the 
> end of the code. Turns out this was already fixed in the original 
> iputils code. The following patch is just a port from iputils which can be 
> found here:
> https://github.com/iputils/iputils/blob/master/arping.c
>
> It also fixes another problem that can occur when the Ethernet 
> connection is down.
>
>
>
> I successfully tested the code change with busybox 1.24.1. It should 
> also work with the current version since this part of the code has not 
> changed since then. The following patch is for the current version.
>
>
>
> Regards,
>
> Michael
>
>
>
> --- a/networking/arping.c            2018-02-09 11:50:56.551059773 +0100
>
> +++ b/networking/arping.c         2018-02-09 11:54:37.494598160 +0100
>
> @@ -423,16 +423,25 @@
>
>                                struct sockaddr_ll from;
>
>                                socklen_t alen = sizeof(from);
>
>                                int cc;
>
> +
>
> +                             sigemptyset(&sset);
>
> +                             sigaddset(&sset, SIGALRM);
>
> +                             sigaddset(&sset, SIGINT);
>
> +                             /* Unblock SIGALRM so that the 
> + previously
> called alarm()
>
> +                             * can prevent recvfrom from blocking 
> + forever
> in case the
>
> +                             * inherited procmask is blocking SIGALRM 
> + and
> no packet
>
> +                             * is received. */
>
> +                             sigprocmask(SIG_UNBLOCK, &sset, &osset);
>
>                                 cc = recvfrom(sock_fd, packet, 4096, 
> 0, (struct sockaddr *) &from, &alen);
>
>                                if (cc < 0) {
>
>                                                
> bb_perror_msg("recvfrom");
>
> +                                             if (errno == ENETDOWN)
>
> +                                                             exit(2);
>
>                                                continue;
>
>                                }
>
> -                              sigemptyset(&sset);
>
> -                              sigaddset(&sset, SIGALRM);
>
> -                              sigaddset(&sset, SIGINT);
>
> -                              sigprocmask(SIG_BLOCK, &sset, &osset);
>
> +
>
> +                             sigprocmask(SIG_BLOCK, &sset, NULL);
>
>                                recv_pack(packet, cc, &from);
>
>                                sigprocmask(SIG_SETMASK, &osset, NULL);
>
>                }

The last sigprocmask(SIG_SETMASK) is now pointless, right?



_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to