Tobias Heider <tobias.hei...@stusta.de> wrote:
> On Thu, Nov 14, 2019 at 05:25:48PM +0100, Mike Belopuhov wrote:
> >
> > Tobias Heider writes:
> >
> > > Hi,
> > >
> > > in the error case ikev2_msg_send the accesses the sa before checking for
> > > NULL. The diff adds explicit checks in those cases.
> > > If sendtofrom fails for any other reason than EADDRNOTAVAIL and sa is not
> > > NULL
> > > we should continue instead of returning (-1) so that the error is handled
> > > with
> > > retransmission.
> > >
> > > ok?
> > >
> >
> > Hi Tobias,
> >
> > you can write a simpler diff w/o repeating log_warn:
> >
> > diff --git a/sbin/iked/ikev2_msg.c b/sbin/iked/ikev2_msg.c
> > index 2baea5f5508..396fea88c16 100644
> > --- a/sbin/iked/ikev2_msg.c
> > +++ b/sbin/iked/ikev2_msg.c
> > @@ -338,7 +338,8 @@ ikev2_msg_send(struct iked *env, struct iked_message
> > *msg)
> > if (sendtofrom(msg->msg_fd, ibuf_data(buf), ibuf_size(buf), 0,
> > (struct sockaddr *)&msg->msg_peer, msg->msg_peerlen,
> > (struct sockaddr *)&msg->msg_local, msg->msg_locallen) == -1) {
> > - if (errno == EADDRNOTAVAIL) {
> > + log_warn("%s: sendtofrom", __func__);
> > + if (msg->msg_sa != NULL && errno == EADDRNOTAVAIL) {
> > sa_state(env, msg->msg_sa, IKEV2_STATE_CLOSING);
> > timer_del(env, &msg->msg_sa->sa_timer);
> > timer_set(env, &msg->msg_sa->sa_timer,
> > @@ -346,8 +347,8 @@ ikev2_msg_send(struct iked *env, struct iked_message
> > *msg)
> > timer_add(env, &msg->msg_sa->sa_timer,
> > IKED_IKE_SA_DELETE_TIMEOUT);
> > }
> > - log_warn("%s: sendtofrom", __func__);
> > - return (-1);
> > + if (msg->msg_sa != NULL)
> > + return (-1);
> > }
> >
> > if (!sa)
> >
> >
> > Regards,
> > Mike
>
> The problem here is that log_warn can change errno,
No, it specifically avoids touching errno.
log_warn(const char *emsg, ...)
{
char *nfmt;
va_list ap;
int saved_errno = errno;
...
errno = saved_errno;
}
