Why do I get a seg fault when copying to data buffer with PACKET_TX_RING?

Question

The following code generates a segmentation fault on iteration 33. The offending line of code is memcpy(data, datagram, 4096);. How could a simple memcpy generate a segmentation fault?

The output of my program in gdb is as follows:

(gdb) r
Starting program: /home/yaakov/mmap/minimal 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Iteration 0
Iteration 1
Iteration 2
Iteration 3
Iteration 4
Iteration 5
Iteration 6
Iteration 7
Iteration 8
Iteration 9
Iteration 10
Iteration 11
Iteration 12
Iteration 13
Iteration 14
Iteration 15
Iteration 16
Iteration 17
Iteration 18
Iteration 19
Iteration 20
Iteration 21
Iteration 22
Iteration 23
Iteration 24
Iteration 25
Iteration 26
Iteration 27
Iteration 28
Iteration 29
Iteration 30
Iteration 31
Iteration 32
Iteration 33

Program received signal SIGSEGV, Segmentation fault.
0x00005555555556e0 in main (argc=1, argv=0x7fffffffe618) at mmap/minimal.c:151
151         memcpy(data, datagram, 4096);
(gdb) bt
#0  0x00005555555556e0 in main (argc=1, argv=0x7fffffffe618) at mmap/minimal.c:151

The minimal reproducer which produces this segmentation fault is as follows:

#include <string.h>
#include <sys/mman.h>
#include <stdint.h>
#include <sys/socket.h>
#include <features.h>    /* for the glibc version number */
#if __GLIBC__ >= 2 && __GLIBC_MINOR >= 1
#include <netpacket/packet.h>
#include <net/ethernet.h>     /* the L2 protocols */
#else
#include <asm/types.h>
#include <linux/if_packet.h>
#include <linux/if_ether.h>   /* The L2 protocols */
#endif
#include <arpa/inet.h> 
#include <stdio.h>
#include <errno.h>
#include <sys/ioctl.h>
#include <net/if.h>

volatile struct tpacket_hdr * ps_header_start;
volatile int fd_socket;
volatile int data_offset = 0;
volatile struct sockaddr_ll *ps_sockaddr = NULL;
static int mode_loss     = 0;
static char * str_devname= NULL;
static int c_buffer_sz   = 1024*8;
static int c_buffer_nb   = 1024;
static int c_sndbuf_sz   = 0;
struct tpacket_req s_packet_req;

int main( int argc, char ** argv )
{
    uint32_t size;
    struct sockaddr_ll my_addr;
    struct ifreq s_ifr;
    int i_index = 0;
    int ec;
    int i_ifindex;
    int tmp;
    
    fd_socket = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
    if(fd_socket == -1)
    {
        perror("socket");
        return 1;
    }
    
    /* clear structure */
    memset(&my_addr, 0, sizeof(struct sockaddr_ll));
    my_addr.sll_family = PF_PACKET;
    my_addr.sll_protocol = htons(ETH_P_ALL);
    
    str_devname = "enp3s0";
    //strcpy (str_devname, ifname);
        
    /* initialize interface struct */
    strncpy (s_ifr.ifr_name, str_devname, sizeof(s_ifr.ifr_name));
    
    /* Get the broad cast address */
    ec = ioctl(fd_socket, SIOCGIFINDEX, &s_ifr);
    if(ec == -1)
    {
        perror("iotcl");
        return 1;
    }
    
    /* update with interface index */
    i_ifindex = s_ifr.ifr_ifindex;
    
    s_ifr.ifr_mtu = 7200;
    /* update the mtu through ioctl */
    ec = ioctl(fd_socket, SIOCSIFMTU, &s_ifr);
    if(ec == -1)
    {
        perror("iotcl");
        return 1;
    }
    
    /* set sockaddr info */
    memset(&my_addr, 0, sizeof(struct sockaddr_ll));
    my_addr.sll_family = AF_PACKET;
    my_addr.sll_protocol = ETH_P_ALL;
    my_addr.sll_ifindex = i_ifindex;
    
    /* bind port */
    if (bind(fd_socket, (struct sockaddr *)&my_addr, sizeof(struct sockaddr_ll)) == -1)
    {
        perror("bind");
        return 1;
    }
    
    /* prepare Tx ring request */
    s_packet_req.tp_block_size = c_buffer_sz;
    s_packet_req.tp_frame_size = c_buffer_sz;
    s_packet_req.tp_block_nr = c_buffer_nb;
    s_packet_req.tp_frame_nr = c_buffer_nb;
    
    /* calculate memory to mmap in the kernel */
    size = s_packet_req.tp_block_size * s_packet_req.tp_block_nr;
    
    /* set packet loss option */
    tmp = mode_loss;
    if (setsockopt(fd_socket, SOL_PACKET, PACKET_LOSS, (char *)&tmp, sizeof(tmp))<0)
    {
        perror("setsockopt: PACKET_LOSS");
        return 1;
    }
        
    /* send TX ring request */
    if (setsockopt(fd_socket, SOL_PACKET, PACKET_TX_RING, (char *)&s_packet_req, sizeof(s_packet_req))<0)
    {
        perror("setsockopt: PACKET_TX_RING");
        return 1;
    }
    
    /* change send buffer size */
    if(c_sndbuf_sz) {
        printf("send buff size = %d\n", c_sndbuf_sz);
        if (setsockopt(fd_socket, SOL_SOCKET, SO_SNDBUF, &c_sndbuf_sz, sizeof(c_sndbuf_sz))< 0)
        {
            perror("getsockopt: SO_SNDBUF");
            return 1;
        }
    }
    
    /* get data offset */
    data_offset = TPACKET_HDRLEN - sizeof(struct sockaddr_ll);
    
    /* mmap Tx ring buffers memory */
    ps_header_start = mmap(0, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd_socket, 0);
    if (ps_header_start == (void*)-1)
    {
        perror("mmap");
        return 1;
    }

    while (1)
    {
        char * data;
        char datagram[4096];
        struct tpacket_hdr * ps_header;
        int ec_send = 0;
        
        printf("Iteration %d\n", i_index);
        
        ps_header = ((struct tpacket_hdr *)((void *)ps_header_start) + (c_buffer_sz*i_index));
        data = ((void*) ps_header) + data_offset;
        
        memset (datagram, 0, 4096);
        
        memcpy(data, datagram, 4096);
        
        /* update packet len */
        ps_header->tp_len = c_buffer_sz;
        /* set header flag to USER (trigs xmit)*/
        ps_header->tp_status = TP_STATUS_SEND_REQUEST;
        
        //int ec_send;
        static int total=0;
        
        /* send all buffers with TP_STATUS_SEND_REQUEST */
        /* Wait end of transfer */
        //ec_send = sendto(fd_socket,NULL,0,MSG_DONTWAIT,(struct sockaddr *) ps_sockaddr,sizeof(struct sockaddr_ll));
        ec_send = sendto(fd_socket,NULL,0,0,(struct sockaddr *) ps_sockaddr,sizeof(struct sockaddr_ll));
        
        i_index++;
        if (i_index >= c_buffer_nb) {
            i_index = 0;
        }
    }
}

Note that the interface name is hard coded to enp3s0. You will likely have to change it for the code to run on your system.

EDIT:

Here is the debugging information requested:

(gdb) p data
$1 = 0x7ffff7dbf020 <__GI_tolower> "\363\017", <incomplete sequence \372\215\227\200>
(gdb) p ps_header_start
$2 = (volatile struct tpacket_hdr *) 0x7ffff757f000
(gdb) p size
$3 = 8388608
(gdb)

Converting decimal 8388608 to hexadecimal gives 800000. Adding 800000 to 7ffff757f000 gives 7FFFF7D7F000. The data pointer is indeed out of bounds.

What is the value of `data` and have you confirmed that there are 4096 valid bytes from that pointer onwards? That is have you verified things like the `mmap` size and the pointer calculations? If so, please update the post with that info. — kaylum, Jan 12 '22 at 22:36
@kaylum My understanding is that ```mmap``` returns a pointer to memory mapped area in memory. ```data``` is just a pointer to a part of that memory mapped area. There must be something significant about the 33rd iteration. Perhaps it is going out of bounds somehow. I don't understand how because there is wrap around logic in the code. — James Read, Jan 12 '22 at 22:58
You didn't actually answer my questions. Have you verified the validity of the `data` pointer and all the memory up to 4096 bytes ? The `data` pointer depends on `i_index`. So it would not be surprising for it to overrun the memory after some number of iterations. That's why it is important to debug directly rather than speculating. — kaylum, Jan 12 '22 at 23:04
@kaylum There is no verification code for the ```data``` pointer. I'm not sure how I would do that. Regarding ```i_index``` there is the following wrap around logic ```if (i_index >= c_buffer_nb) { i_index = 0; }``` — James Read, Jan 12 '22 at 23:07
I understand that you think the wrapping code is working. And it may even be. But you need to verify that by debugging. Run the code in a debugger and look at the `data` pointer value at the point it crashes. Then compare it against `ps_header_start + size` to see whether it is within bounds. What I'm trying to say is that you need to actively debug the running program rather than just inspect the code. — kaylum, Jan 12 '22 at 23:09
@kaylum I've updated the question with the requested debugging information. I'm not sure if ```data``` variable is out of bounds or not. It looks like it could be. What's your take? — James Read, Jan 12 '22 at 23:14
It does look out of bounds. The mmap region ends at `ps_header_start+size`. Which is `0x7ffff757f000+8388608 == 0x7FFFF7D7F000`. Which is less than the `data` pointer value. That is, `data` is beyond the mmap region. So something in your logic or calculations is not correct. — kaylum, Jan 12 '22 at 23:20
I think your problem is here: `ps_header = ((struct tpacket_hdr *)((void *)ps_header_start) + (c_buffer_sz*i_index));` you are getting the offset but the base pointer you are doing the math on, should be a `char *` type not `(struct tpacket_hdr *)`. — hesham_EE, Jan 13 '22 at 01:09
@hesham_EE OK. That seems to work. But I have no idea why. Can you explain? — James Read, Jan 13 '22 at 02:24
Pointer arithmetic: if you declare `type * ptr;`, when you do pointer asthmatic like `(ptr + 4)` this means `ptr + 4 * sizeof(type)`. It's like this to handle accessing indexed elements, in the example, to get to the 4th element in the array pointed to by `ptr`. It's a basic property of pointers in C. — hesham_EE, Jan 13 '22 at 05:11
@JamesRead I didn't know about `mmap()` to use file descriptor `fd` of a socket. Do you have a reference for this sample code? — hesham_EE, Jan 13 '22 at 05:14
@hesham_EE Or maybe this is better https://www.kernel.org/doc/html/latest/networking/packet_mmap.html — James Read, Jan 13 '22 at 12:39

score 1 · Accepted Answer · answered Jan 13 '22 at 15:29

1

The issue is here:

ps_header = ((struct tpacket_hdr *)((void *)ps_header_start) + (c_buffer_sz*i_index));

You're taking ps_header_start, casting it to a struct tpacket_hdr * and then adding a buffer size. But when you add to a pointer in C, the value added is multiplied by the size of the pointed-to type. In other words, instead of adding c_buffer_size * i_index to the starting pointer, you're actually adding c_buffer_sz * i_index * sizeof(struct tpacket_hdr) to it.

The right way to calculate the address of the next frame slot is:

ps_header = (struct tpacket_hdr *)((char *)ps_header_start + (c_buffer_sz * i_index));

Cast the starting pointer to a char * (because the size of char is one), then add to it the product of frame size and index, then cast it back to struct tpacket_hdr *.

answered Jan 13 '22 at 15:29

Gil Hamilton

11,973
28
51

I tried this. It produces a segmentation fault with gdb pointing the finger of blame at ```memcpy(data, datagram, 4096);```. However, if I do ```ps_header = ((char *)((void *)ps_header_start) + (c_buffer_sz*i_index));``` it runs without error but produces a warning a compile time. I guess I can live with the compiler warning as long as it runs right. – James Read Jan 13 '22 at 15:37
Check your parentheses then. You must ensure the address arithmetic is done (with `char *`) *before* casting back to `struct tpacket_hdr *`. With the line I provided above, your code works fine for me. (The cast back to `struct tpacket_hdr *` really does nothing except to silence the warning.) – Gil Hamilton Jan 13 '22 at 17:22
You're right again. It was the parentheses. Thanks again. I'm marking this as the accepted answer. – James Read Jan 13 '22 at 17:33
Can you figure out why I am not receiving any packets with an extended version of the program https://stackoverflow.com/questions/70676086/why-is-recvfrom-blocking-forever-with-pf-packet-socket – James Read Jan 13 '22 at 19:01

Why do I get a seg fault when copying to data buffer with PACKET_TX_RING?

1 Answers1