Commit 706f0e44 authored by Michael Cook's avatar Michael Cook

pdcp_fifo_flush_sdus: Fix message size calculation

-fsanitize=address detected that we were trying to access bytes
past the end of a block of malloc'ed memory.

Specifically, in this code:

```
    } else if (ENB_NAS_USE_TUN) {
      if( LOG_DEBUGFLAG(DEBUG_PDCP) )
        log_dump(PDCP, pdcpData, sizeToWrite, LOG_DUMP_CHAR,"PDCP
      output to be sent to TUN interface: \n");
      ret = write(nas_sock_fd[0], pdcpData, sizeToWrite);
```

-fsanitize=address said:

```
==80==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x61100004ffdc at pc 0x7f57c5f576a5 bp 0x7f57bb53c240 sp
0x7f57bb53b9e8
READ of size 108 at 0x61100004ffdc thread T7

0x61100004ffdc is located 0 bytes to the right of 220-byte region
[0x61100004ff00,0x61100004ffdc)
```

So, the code was trying to access the first byte after a block of
heap memory.

sizeToWrite was calculated like this:

```
    int sizeToWrite= sizeof (pdcp_data_ind_header_t) +
      pdcpHead->data_size;
```

There were a few other similar invocations of write() in the same
function used the wrong size.  That sizeToWrite calculation
should be used only when the header is being sent, too, which
happens in only one place in this function.

With this commit, our tests pass and -fsanitize=address is happy.
parent b6766ee8
......@@ -334,8 +334,8 @@ else (CUDA_FOUND)
endif ()
# Uncomment these lines to enable the address sanitizer
#set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address")
#set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address")
# Uncomment these lines to enable gprof profiling
#set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pg -DUSING_GPROF")
......
......@@ -113,8 +113,6 @@ int pdcp_fifo_flush_sdus(const protocol_ctxt_t *const ctxt_pP) {
* PDCP packet to pick the right socket below */
int rb_id = pdcpHead->rb_id;
int sizeToWrite= sizeof (pdcp_data_ind_header_t) +
pdcpHead->data_size;
void * pdcpData=(void*)(pdcpHead+1);
if (rb_id == 10) { //hardcoded for PC5-Signaling
if( LOG_DEBUGFLAG(DEBUG_PDCP) ) {
......@@ -128,25 +126,29 @@ int pdcp_fifo_flush_sdus(const protocol_ctxt_t *const ctxt_pP) {
} else if (UE_NAS_USE_TUN) {
//ret = write(nas_sock_fd[pdcpHead->inst], &(sdu_p->data[sizeof(pdcp_data_ind_header_t)]),sizeToWrite );
if(rb_id == mbms_rab_id){
ret = write(nas_sock_mbms_fd, pdcpData,sizeToWrite );
LOG_I(PDCP,"[PDCP_FIFOS] ret %d TRIED TO PUSH MBMS DATA TO rb_id %d handle %d sizeToWrite %d\n",
ret,rb_id,nas_sock_fd[pdcpHead->inst],sizeToWrite);
ret = write(nas_sock_mbms_fd, pdcpData, pdcpHead->data_size);
LOG_I(PDCP,"[PDCP_FIFOS] ret %d TRIED TO PUSH MBMS DATA TO rb_id %d handle %d size %d\n",
ret,rb_id,nas_sock_fd[pdcpHead->inst], pdcpHead->data_size);
}
else
{
if( LOG_DEBUGFLAG(DEBUG_PDCP) )
log_dump(PDCP, pdcpData, sizeToWrite, LOG_DUMP_CHAR,"PDCP output to be sent to TUN interface: \n");
ret = write(nas_sock_fd[pdcpHead->inst], pdcpData,sizeToWrite );
LOG_T(PDCP,"[UE PDCP_FIFOS] ret %d TRIED TO PUSH DATA TO rb_id %d handle %d sizeToWrite %d\n",
ret,rb_id,nas_sock_fd[pdcpHead->inst],sizeToWrite);
log_dump(PDCP, pdcpData, pdcpHead->data_size, LOG_DUMP_CHAR,
"PDCP output to be sent to TUN interface: \n");
ret = write(nas_sock_fd[pdcpHead->inst], pdcpData, pdcpHead->data_size);
LOG_T(PDCP,"[UE PDCP_FIFOS] ret %d TRIED TO PUSH DATA TO rb_id %d handle %d size %d\n",
ret,rb_id,nas_sock_fd[pdcpHead->inst], pdcpHead->data_size);
}
} else if (ENB_NAS_USE_TUN) {
if( LOG_DEBUGFLAG(DEBUG_PDCP) )
log_dump(PDCP, pdcpData, sizeToWrite, LOG_DUMP_CHAR,"PDCP output to be sent to TUN interface: \n");
ret = write(nas_sock_fd[0], pdcpData, sizeToWrite);
LOG_T(PDCP,"[NB PDCP_FIFOS] ret %d TRIED TO PUSH DATA TO rb_id %d handle %d sizeToWrite %d\n",ret,rb_id,nas_sock_fd[0],sizeToWrite);
log_dump(PDCP, pdcpData, pdcpHead->data_size, LOG_DUMP_CHAR,
"PDCP output to be sent to TUN interface: \n");
ret = write(nas_sock_fd[0], pdcpData, pdcpHead->data_size);
LOG_T(PDCP,"[NB PDCP_FIFOS] ret %d TRIED TO PUSH DATA TO rb_id %d handle %d size %d\n",
ret, rb_id, nas_sock_fd[0], pdcpHead->data_size);
} else if (PDCP_USE_NETLINK) {
memcpy(NLMSG_DATA(nas_nlh_tx), (uint8_t *) pdcpHead, sizeToWrite);
int sizeToWrite = sizeof(pdcp_data_ind_header_t) + pdcpHead->data_size;
memcpy(NLMSG_DATA(nas_nlh_tx), (uint8_t *) pdcpHead, sizeToWrite);
nas_nlh_tx->nlmsg_len = sizeToWrite;
ret = sendmsg(nas_sock_fd[0],&nas_msg_tx,0);
} // PDCP_USE_NETLINK
......@@ -154,7 +156,7 @@ int pdcp_fifo_flush_sdus(const protocol_ctxt_t *const ctxt_pP) {
AssertFatal(ret >= 0,"[PDCP_FIFOS] pdcp_fifo_flush_sdus (errno: %d %s), nas_sock_fd[0]: %d\n", errno, strerror(errno), nas_sock_fd[0]);
if( LOG_DEBUGFLAG(DEBUG_PDCP) )
log_dump(PDCP, pdcpData, min(sizeToWrite,30) , LOG_DUMP_CHAR,
log_dump(PDCP, pdcpData, min(pdcpHead->data_size, 30) , LOG_DUMP_CHAR,
"Printing first bytes of PDCP SDU before removing it from the list: \n");
delNotifiedFIFO_elt (sdu_p);
pdcp_nb_sdu_sent ++;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment