osmo_pcap_lapd_write: Fix write of uninitialized byte(s)
The problem is that we don't zero-initialize the struct pcap_rechdr +
pcap_lapdhdr before memcpy'ing them to buf, before we call write:
==20097== Syscall param write(buf) points to uninitialised byte(s)
==20097== at 0x4E48471: write (write.c:26)
==20097== by 0x4DA8DE9: osmo_pcap_lapd_write (lapd_pcap.c:168)
==20097== by 0x4DA8433: send_ph_data_req (lapd.c:628)
==20097== by 0x4C94F5C: lapd_send_rej (lapd_core.c:536)
==20097== by 0x4C9A08A: lapd_rx_i (lapd_core.c:1574)
==20097== by 0x4C9AA8F: lapd_ph_data_ind (lapd_core.c:1708)
==20097== by 0x4DA7C55: lapd_receive (lapd.c:496)
==20097== by 0x4D96B2C: e1inp_rx_ts_lapd (e1_input.c:778)
==20097== by 0x4D9C97C: handle_ts_sign_read (e1d.c:78)
==20097== by 0x4D9D908: e1d_fd_cb (e1d.c:281)
==20097== by 0x4D1281B: poll_disp_fds (select.c:361)
==20097== by 0x4D12928: _osmo_select_main (select.c:399)
==20097== Address 0x1ffefffed7 is on thread 1's stack
==20097== in frame #1, created by osmo_pcap_lapd_write (lapd_pcap.c:129)
The whole idea of first filling the two structs on the stack, and then
copying them to another buffer on the stack is somehow weird. Let's
just create a combined struct on the stack and then fill that one
directly.
Change-Id: I358c71354cc6ddad1964cc4a988ad29b7ba617f1
Closes: OS#5592
diff --git a/src/input/lapd_pcap.c b/src/input/lapd_pcap.c
index 77d4971..6e1faf5 100644
--- a/src/input/lapd_pcap.c
+++ b/src/input/lapd_pcap.c
@@ -127,50 +127,45 @@
/* This currently only works for the D-Channel */
int osmo_pcap_lapd_write(int fd, int direction, struct msgb *msg)
{
- int numbytes = 0;
struct timeval tv;
- struct pcap_rechdr pcap_rechdr;
- struct pcap_lapdhdr header;
- char buf[sizeof(struct pcap_rechdr) +
- sizeof(struct pcap_lapdhdr) + msg->len];
+ struct {
+ struct pcap_rechdr pcap_rechdr;
+ struct pcap_lapdhdr header;
+ char buf[msg->len];
+ } __attribute__((packed)) s;
/* PCAP file has not been opened, skip. */
if (fd < 0)
return 0;
- pcap_rechdr.ts_sec = 0;
- pcap_rechdr.ts_usec = 0;
- pcap_rechdr.incl_len = msg->len + sizeof(struct pcap_lapdhdr);
- pcap_rechdr.orig_len = msg->len + sizeof(struct pcap_lapdhdr);
+ memset(&s, 0, sizeof(s));
+
+ s.pcap_rechdr.ts_sec = 0;
+ s.pcap_rechdr.ts_usec = 0;
+ s.pcap_rechdr.incl_len = msg->len + sizeof(struct pcap_lapdhdr);
+ s.pcap_rechdr.orig_len = msg->len + sizeof(struct pcap_lapdhdr);
if (direction == OSMO_LAPD_PCAP_OUTPUT)
- header.pkttype = htons(LINUX_SLL_OUTGOING);
+ s.header.pkttype = htons(LINUX_SLL_OUTGOING);
else
- header.pkttype = htons(LINUX_SLL_HOST);
- header.hatype = 0;
- header.halen = 0;
- header.addr[0] = 0x01; /* we are the network side */
- header.protocol = ntohs(48);
+ s.header.pkttype = htons(LINUX_SLL_HOST);
+ s.header.hatype = 0;
+ s.header.halen = 0;
+ s.header.addr[0] = 0x01; /* we are the network side */
+ s.header.protocol = ntohs(48);
gettimeofday(&tv, NULL);
- pcap_rechdr.ts_sec = tv.tv_sec;
- pcap_rechdr.ts_usec = tv.tv_usec;
+ s.pcap_rechdr.ts_sec = tv.tv_sec;
+ s.pcap_rechdr.ts_usec = tv.tv_usec;
- memcpy(buf + numbytes, &pcap_rechdr, sizeof(pcap_rechdr));
- numbytes += sizeof(pcap_rechdr);
+ memcpy(s.buf, msg->data, msg->len);
- memcpy(buf + numbytes, &header, sizeof(header));
- numbytes += sizeof(header);
-
- memcpy(buf + numbytes, msg->data, msg->len);
- numbytes += msg->len;
-
- if (write(fd, buf, numbytes) != numbytes) {
+ if (write(fd, &s, sizeof(s)) != sizeof(s)) {
LOGP(DLLAPD, LOGL_ERROR, "cannot write packet to PCAP: %s\n",
strerror(errno));
return -1;
}
- return numbytes;
+ return sizeof(s);
}
int osmo_pcap_lapd_close(int fd)