From 0fa6a290246b0d4b980b9af418fbf0f9cffd7da5 Mon Sep 17 00:00:00 2001 From: XANTRONIX Development Date: Tue, 21 Jul 2020 01:38:23 -0400 Subject: [PATCH] Fix subtle sequence issues in src/sock.c Fix subtle sequence issues in src/sock.c wherein non-I frames which are sent may overwrite a previously sent I frame with the same sequence number, which would cause issues with the remote end expecting an I frame and possibly getting a non-I frame --- src/sock.c | 128 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 77 insertions(+), 51 deletions(-) diff --git a/src/sock.c b/src/sock.c index 217e527..754fb82 100644 --- a/src/sock.c +++ b/src/sock.c @@ -54,19 +54,40 @@ error_open: return -1; } -static inline size_t tx_slot_size(size_t len) { - return sizeof(size_t) - + sizeof(patty_ax25_addr) * 2 - + sizeof(patty_ax25_addr) * PATTY_AX25_MAX_HOPS - + len; +static inline size_t tx_bufsz(patty_ax25_sock *sock) { + return PATTY_AX25_FRAME_OVERHEAD + sock->n_maxlen_tx; +} + +static inline size_t tx_slot_size(patty_ax25_sock *sock) { + return sizeof(size_t) + PATTY_AX25_FRAME_OVERHEAD + sock->n_maxlen_tx; } static inline size_t tx_slots_size(patty_ax25_sock *sock) { - return sock->n_window_tx * tx_slot_size(sock->n_maxlen_tx); + return sock->n_window_tx * tx_slot_size(sock); } -static inline void *tx_slot(void *slots, size_t i, size_t len) { - return (void *)((uint8_t *)slots + (i * tx_slot_size(len))); +static inline void *tx_slot(patty_ax25_sock *sock, size_t i) { + return (void *)((uint8_t *)sock->tx_slots + (i * tx_slot_size(sock))); +} + +static int init_bufs(patty_ax25_sock *sock) { + if ((sock->buf = realloc(sock->buf, tx_bufsz(sock))) == NULL) { + goto error_realloc_buf; + } + + if ((sock->tx_slots = realloc(sock->tx_slots, tx_slots_size(sock))) == NULL) { + goto error_realloc_tx_slots; + } + + return 0; + +error_realloc_tx_slots: + free(sock->buf); + + sock->buf = NULL; + +error_realloc_buf: + return -1; } patty_ax25_sock *patty_ax25_sock_new(enum patty_ax25_proto proto, @@ -87,12 +108,8 @@ patty_ax25_sock *patty_ax25_sock_new(enum patty_ax25_proto proto, sock->flags_classes = PATTY_AX25_PARAM_DEFAULT_CLASSES; sock->flags_hdlc = PATTY_AX25_SOCK_DEFAULT_HDLC; - if ((sock->buf = malloc(PATTY_AX25_FRAME_OVERHEAD + sock->n_maxlen_rx)) == NULL) { - goto error_malloc_buf; - } - - if ((sock->tx_slots = malloc(tx_slots_size(sock))) == NULL) { - goto error_malloc_tx_slots; + if (init_bufs(sock) < 0) { + goto error_init_bufs; } if (bind_pty(sock) < 0) { @@ -102,12 +119,10 @@ patty_ax25_sock *patty_ax25_sock_new(enum patty_ax25_proto proto, return sock; error_bind_pty: - free(sock->tx_slots); +error_init_bufs: + if (sock->tx_slots) free(sock->tx_slots); + if (sock->buf) free(sock->buf); -error_malloc_tx_slots: - free(sock->buf); - -error_malloc_buf: free(sock); error_malloc_sock: @@ -277,9 +292,20 @@ void patty_ax25_sock_seq_recv_incr(patty_ax25_sock *sock) { } } -static size_t addr_copy_to_buf(void *dest, - patty_ax25_sock *sock, - enum patty_ax25_frame_cr cr) { +static inline size_t maxsz(patty_ax25_sock *sock, + size_t infolen) { + return infolen + PATTY_AX25_FRAME_OVERHEAD; +} + +static inline int toobig(patty_ax25_sock *sock, + size_t infolen) { + return infolen > PATTY_AX25_FRAME_OVERHEAD + sock->n_maxlen_tx; +} + +static ssize_t encode_address(patty_ax25_sock *sock, + enum patty_ax25_frame_cr cr, + void *dest, + size_t len) { uint8_t *buf = (uint8_t *)dest; size_t offset = 0; @@ -288,6 +314,12 @@ static size_t addr_copy_to_buf(void *dest, unsigned int i; + if ((2 + sock->hops) * sizeof(patty_ax25_addr) > len) { + errno = EOVERFLOW; + + goto error_toobig; + } + switch (cr) { case PATTY_AX25_FRAME_COMMAND: flags_remote = 0x80; break; case PATTY_AX25_FRAME_RESPONSE: flags_local = 0x80; break; @@ -304,12 +336,9 @@ static size_t addr_copy_to_buf(void *dest, ((uint8_t *)buf)[offset-1] |= 1; return offset; -} -static inline int toobig(patty_ax25_sock *sock, - uint16_t control, - size_t infolen) { - return infolen > PATTY_AX25_FRAME_OVERHEAD + sock->n_maxlen_tx; +error_toobig: + return -1; } ssize_t patty_ax25_sock_send(patty_ax25_sock *sock, @@ -317,50 +346,48 @@ ssize_t patty_ax25_sock_send(patty_ax25_sock *sock, uint16_t control, void *info, size_t infolen) { - size_t offset; + size_t offset = 0; + ssize_t encoded; - void *slot = tx_slot(sock->tx_slots, - sock->seq_send, - sock->n_maxlen_tx); + uint8_t *buf = PATTY_AX25_FRAME_CONTROL_U(control)? + sock->buf: tx_slot(sock, sock->seq_send); - uint8_t *buf = (uint8_t *)slot + sizeof(size_t); - - if (toobig(sock, control, infolen)) { + if (toobig(sock, infolen)) { goto error_toobig; } - offset = addr_copy_to_buf(buf, sock, cr); - - if (sock->mode == PATTY_AX25_SOCK_SABME) { - buf[offset++] = control & 0x00ff; - - if (!(PATTY_AX25_FRAME_CONTROL_U(control))) { - buf[offset++] = (control & 0xff00) >> 8; - } + if ((encoded = encode_address(sock, cr, buf, tx_bufsz(sock))) < 0) { + goto error_encode_address; } else { - buf[offset++] = control; + offset += encoded; } - if (info) { + buf[offset++] = control & 0x00ff; + + if (sock->mode == PATTY_AX25_SOCK_SABME && !PATTY_AX25_FRAME_CONTROL_U(control)) { + buf[offset++] = (control & 0xff00) >> 8; + } + + if (info && infolen) { if (PATTY_AX25_FRAME_CONTROL_I(control)) { buf[offset++] = (uint8_t)sock->proto; } memcpy(buf + offset, info, infolen); + offset += infolen; } - *((size_t *)slot) = offset; - return patty_ax25_if_send(sock->iface, buf, offset); +error_encode_address: error_toobig: return -1; } ssize_t patty_ax25_sock_resend(patty_ax25_sock *sock, unsigned int seq) { - void *slot = tx_slot(sock->tx_slots, seq, sock->n_maxlen_tx); + void *slot = tx_slot(sock, seq); void *buf = (uint8_t *)slot + sizeof(size_t); size_t len = *((size_t *)slot); @@ -472,6 +499,7 @@ ssize_t patty_ax25_sock_send_disc(patty_ax25_sock *sock, int pf) { ssize_t patty_ax25_sock_send_xid(patty_ax25_sock *sock) { patty_ax25_params params; + char buf[256]; ssize_t encoded; @@ -507,16 +535,14 @@ ssize_t patty_ax25_sock_send_xid(patty_ax25_sock *sock) { params.ack = sock->n_ack; params.retry = sock->n_retry; - if ((encoded = patty_ax25_frame_encode_xid(¶ms, - sock->buf, - sock->n_maxlen_tx)) < 0) { + if ((encoded = patty_ax25_frame_encode_xid(¶ms, buf, sizeof(buf))) < 0) { goto error_frame_encode_xid; } return patty_ax25_sock_send(sock, PATTY_AX25_FRAME_COMMAND, control_u(PATTY_AX25_FRAME_XID, 0), - sock->buf, + buf, encoded); error_nopeer: