Tobias Heider writes:
> On Thu, Nov 14, 2019 at 09:57:27AM -0700, Theo de Raadt wrote:
>> >
>> > 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;
>> }
>>
>
> Good to know, thanks! In that case I really prefer Mike's diff.
> Here is an update with msg->msg_sa used consistently. We can also do it
> the other way around, but I would prefer to use either sa or msg_sa.
I'm sorry, it was a bit irresponsible of me to put msg_sa everywhere
in my diff. In fact, almost all of the code in this file uses 'sa'
so it would be consistent with other functions if 'sa' would be used
instead of 'msg_sa'. If you don't mind changing it to 'sa', I would
appreciate it. OK mikeb either way.
>
> Index: ikev2_msg.c
> ===================================================================
> RCS file: /cvs/src/sbin/iked/ikev2_msg.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 ikev2_msg.c
> --- ikev2_msg.c 13 Nov 2019 12:24:40 -0000 1.58
> +++ ikev2_msg.c 14 Nov 2019 17:15:42 -0000
> @@ -303,7 +303,6 @@ ikev2_msg_valid_ike_sa(struct iked *env,
> int
> ikev2_msg_send(struct iked *env, struct iked_message *msg)
> {
> - struct iked_sa *sa = msg->msg_sa;
> struct ibuf *buf = msg->msg_data;
> uint32_t natt = 0x00000000;
> int isnatt = 0;
> @@ -338,7 +337,8 @@ ikev2_msg_send(struct iked *env, struct
> 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,11 +346,11 @@ ikev2_msg_send(struct iked *env, struct
> 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)
> + if (msg->msg_sa == NULL)
> return (0);
>
> if ((m = ikev2_msg_copy(env, msg)) == NULL) {
> @@ -360,11 +360,11 @@ ikev2_msg_send(struct iked *env, struct
> m->msg_exchange = exchange;
>
> if (flags & IKEV2_FLAG_RESPONSE) {
> - TAILQ_INSERT_TAIL(&sa->sa_responses, m, msg_entry);
> + TAILQ_INSERT_TAIL(&msg->msg_sa->sa_responses, m, msg_entry);
> timer_set(env, &m->msg_timer, ikev2_msg_response_timeout, m);
> timer_add(env, &m->msg_timer, IKED_RESPONSE_TIMEOUT);
> } else {
> - TAILQ_INSERT_TAIL(&sa->sa_requests, m, msg_entry);
> + TAILQ_INSERT_TAIL(&msg->msg_sa->sa_requests, m, msg_entry);
> timer_set(env, &m->msg_timer, ikev2_msg_retransmit_timeout, m);
> timer_add(env, &m->msg_timer, IKED_RETRANSMIT_TIMEOUT);
> }
