Changeset ebed638


Ignore:
Timestamp:
09/04/18 14:56:53 (2 years ago)
Author:
Shane Alcock <salcock@…>
Branches:
develop, master, ringdecrementfix, ringperformance
Children:
2201d8c
Parents:
37ee856
Message:

Don't munmap ring rx buffers until last possible moment.

As soon as we munmap the buffers, any packets contained within
those buffers become invalid -- however, the user application may
still have references to those packets and could try to operate
on them without knowing that the memory holding the packet payload
is invalid.

Previously, the buffers were munmapped when the trace was paused,
which was causing crashes and invalid memory accesses.

Instead, we now only munmap the buffer if either the trace is
restarted or destroyed, so the packets will remain valid for
longer. Also, the new "start" iteration counting added in an
earlier commit means we can now recognise when a packet belongs
to an unmapped buffer and therefore return appropriate errors if
a user tries to interact with the packet.

This also fixes the assertion failure when freeing a ring packet
after the ring trace has been paused, i.e. after a call to
trace_pstop().

Location:
lib
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • lib/format_linux_common.c

    r47d4f8c rebed638  
    232232                close(stream->fd);
    233233        stream->fd = -1;
    234         if (stream->rx_ring != MAP_FAILED)
    235                 munmap(stream->rx_ring,
    236                        stream->req.tp_block_size *
    237                        stream->req.tp_block_nr);
    238         stream->rx_ring = MAP_FAILED;
    239         stream->rxring_offset = 0;
    240234        FORMAT_DATA->dev_stats.if_name[0] = 0;
     235
     236        /* Don't munmap the rx_ring here -- keep it around as long as
     237         * possible to ensure that any packets that the user is still
     238         * holding references to remain valid.
     239         *
     240         * Don't worry, linuxring will munmap the rx_ring as soon as
     241         * someone either destroys or restarts the trace. At that point,
     242         * any remaining packets from the old ring will be recognisable
     243         * as invalid.
     244         */
    241245}
    242246
  • lib/format_linux_ring.c

    r8a63abd rebed638  
    252252        char error[2048];
    253253
     254        /* Unmap any previous ring buffers associated with this stream. */
     255        if (stream->rx_ring != MAP_FAILED) {
     256                munmap(stream->rx_ring, stream->req.tp_block_size *
     257                                stream->req.tp_block_nr);
     258                stream->rx_ring = MAP_FAILED;
     259                stream->rxring_offset = 0;
     260        }
     261
     262
    254263        /* We set the socket up the same and then convert it to PACKET_MMAP */
    255264        if (linuxcommon_start_input_stream(libtrace, stream) < 0)
     
    274283        return 0;
    275284}
     285
     286static int linuxring_fin_input(libtrace_t *libtrace) {
     287        size_t i;
     288
     289        if (libtrace->format_data) {
     290                for (i = 0; i < libtrace_list_get_size(FORMAT_DATA->per_stream); ++i) {
     291                        struct linux_per_stream_t *stream;
     292                        stream = libtrace_list_get_index(
     293                                FORMAT_DATA->per_stream, i)->data;
     294                        if (stream->rx_ring != MAP_FAILED) {
     295                                munmap(stream->rx_ring,
     296                                                stream->req.tp_block_size *
     297                                                stream->req.tp_block_nr);
     298                        }
     299                }
     300
     301                if (FORMAT_DATA->filter != NULL)
     302                        free(FORMAT_DATA->filter);
     303
     304                if (FORMAT_DATA->per_stream)
     305                        libtrace_list_deinit(FORMAT_DATA->per_stream);
     306
     307                free(libtrace->format_data);
     308        }
     309
     310        return 0;
     311}
     312
    276313
    277314static int linuxring_start_input(libtrace_t *libtrace)
     
    622659        /* If we own the packet (i.e. it's not a copy), we need to free it */
    623660        if (packet->buf_control == TRACE_CTRL_EXTERNAL) {
    624                 /* Started should always match the existence of the rx_ring
    625                  * in the parallel case still just check the first ring */
    626                 assert(!!FORMAT_DATA_FIRST->rx_ring ==
    627                        !!packet->trace->started);
    628661                /* If we don't have a ring its already been destroyed */
    629662                if (FORMAT_DATA_FIRST->rx_ring != MAP_FAILED)
     
    742775        NULL,                           /* config_output */
    743776        linuxring_start_output,         /* start_ouput */
    744         linuxcommon_fin_input,          /* fin_input */
     777        linuxring_fin_input,            /* fin_input */
    745778        linuxring_fin_output,           /* fin_output */
    746779        linuxring_read_packet,          /* read_packet */
Note: See TracChangeset for help on using the changeset viewer.