ligang/vpp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

api: Avoid the usage of the freed registration by the API calls

Browse Source 
This issue happens if:
- the API client connects via Unix socket
- the client issues the *_dump API call and immediately disconnects

What happens after is that the API handler keeps sending the *_details
messages, however at some point the write fails, and the socket is
deleted.

The attempt of a use of the registration pointer results in interpreting
the socket as a shared memory socket. This results in a crash, because
the data in this structure then does not make sense, like the below:

|
|Thread 1 "vpp_main" received signal SIGSEGV, Segmentation fault.
|__GI___pthread_mutex_lock (mutex=0x0) at ../nptl/pthread_mutex_lock.c:67
|67      ../nptl/pthread_mutex_lock.c: No such file or directory.
|(gdb) bt
|#0  __GI___pthread_mutex_lock (mutex=0x0) at ../nptl/pthread_mutex_lock.c:67
|#1  0x00007ffff500f957 in svm_queue_lock (q=0x0) at /home/ubuntu/vpp/src/svm/queue.c:101
|#2  svm_queue_add (q=0x0, elem=0x7fffa76c2de0 "\210\365\006\060\001", nowait=0) at /home/ubuntu/vpp/src/svm/queue.c:274
|#3  0x00007ffff6e131e3 in vl_api_send_msg (rp=<optimized out>, elem=<optimized out>) at /home/ubuntu/vpp/src/vlibmemory/api.h:43
|#4  send_sw_interface_details (am=<optimized out>, rp=<optimized out>, swif=0x7fffb957a0bc, interface_name=<optimized out>, context=<optimized out>)
|    at /home/ubuntu/vpp/src/vnet/interface_api.c:353
|#5  0x00007ffff6e0edeb in vl_api_sw_interface_dump_t_handler (mp=<optimized out>) at /home/ubuntu/vpp/src/vnet/interface_api.c:412
|#6  0x00007ffff7daeb48 in msg_handler_internal (am=<optimized out>, the_msg=0x7fffb839a5e0, trace_it=<optimized out>, do_it=1, free_it=0)
|    at /home/ubuntu/vpp/src/vlibapi/api_shared.c:501
|#7  vl_msg_api_socket_handler (the_msg=0x7fffb839a5e0) at /home/ubuntu/vpp/src/vlibapi/api_shared.c:790
|#8  0x00007ffff7d7c608 in vl_socket_process_api_msg (rp=<optimized out>, input_v=0x7fffa76c2de0 "\210\365\006\060\001") at /home/ubuntu/vpp/src/vlibmemory/socket_api.c:212
|#9  0x00007ffff7d89ff1 in vl_api_clnt_process (vm=<optimized out>, node=<optimized out>, f=<optimized out>) at /home/ubuntu/vpp/src/vlibmemory/vlib_api.c:405
|#10 0x00007ffff53bf9a7 in vlib_process_bootstrap (_a=<optimized out>) at /home/ubuntu/vpp/src/vlib/main.c:1490
|#11 0x00007ffff4da0b2c in clib_calljmp () from /home/ayourtch/vpp/build-root/install-vpp-native/vpp/lib/libvppinfra.so.21.06
|#12 0x00007fffa99a4d90 in ?? ()
|#13 0x00007ffff53b6cb2 in vlib_process_startup (vm=0x7ffff56a9880 <vlib_global_main>, p=0x7fffb5d41380, f=0x0) at /home/ubuntu/vpp/src/vlib/main.c:1515
|#14 dispatch_process (vm=0x7ffff56a9880 <vlib_global_main>, p=0x7fffb5d41380, f=0x0, last_time_stamp=<optimized out>) at /home/ubuntu/vpp/src/vlib/main.c:1571
|#15 0x0000000000000000 in ?? ()
|(gdb) frame 3
|#3  0x00007ffff6e131e3 in vl_api_send_msg (rp=<optimized out>, elem=<optimized out>) at /home/ubuntu/vpp/src/vlibmemory/api.h:43
|43            vl_msg_api_send_shmem (rp->vl_input_queue, (u8 *) & elem);
|(gdb) l
|38          {
|39            vl_socket_api_send (rp, elem);
|40          }
|41        else
|42          {
|43            vl_msg_api_send_shmem (rp->vl_input_queue, (u8 *) & elem);
|44          }
|45      }
|46
|47      always_inline int
|(gdb)
|

The approach in this change is to avoid the closing operations "here and
now", but instead mark the the registration as a zombie and place
a forced RPC towards a callback that does the actual cleanup work.

Forced RPC is handled via the API processing loop with barrier sync,
so we are guaranteed not to have any API processing in-process.

Type: fix
Change-Id: I1972d42da620bdb4fd773c83262863c2781d9005
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
This commit is contained in:
Andrew Yourtchenko
2021-03-11 12:54:11 +00:00
committed by Florin Coras
parent 415cd67c29
commit 162b70d50a
2 changed files with 52 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/vlibapi/api_common.h
View File

@ -57,6 +57,7 @@ typedef struct vl_api_registration_
f64 last_heard;
int last_queue_head;
int unanswered_pings;
int is_being_removed;
/** shared memory only: pointer to client input queue */
svm_queue_t *vl_input_queue;

74
src/vlibmemory/socket_api.c
View File

@ -148,15 +148,6 @@ vl_socket_api_send (vl_api_registration_t * rp, u8 * elem)
error = clib_file_write (cf);
unix_save_error (&unix_main, error);
/* Make sure cf not removed in clib_file_write */
cf = vl_api_registration_file (rp);
if (!cf)
{
clib_warning ("cf removed");
vl_msg_api_free ((void *) elem);
return;
}
/* If we didn't finish sending everything, wait for tx space */
if (vec_len (sock_rp->output_vector) > 0
&& !(cf->flags & UNIX_FILE_DATA_AVAILABLE_TO_WRITE))
@ -213,6 +204,42 @@ vl_socket_process_api_msg (vl_api_registration_t * rp, i8 * input_v)
socket_main.current_rp = 0;
}
int
is_being_removed_reg_index (u32 reg_index)
{
vl_api_registration_t *rp = vl_socket_get_registration (reg_index);
ALWAYS_ASSERT (rp != 0);
return (rp->is_being_removed);
}
static void
socket_cleanup_pending_remove_registration_cb (u32 *preg_index)
{
vl_api_registration_t *rp = vl_socket_get_registration (*preg_index);
clib_file_main_t *fm = &file_main;
u32 pending_remove_file_index = vl_api_registration_file_index (rp);
clib_file_t *zf = fm->file_pool + pending_remove_file_index;
clib_file_del (fm, zf);
vl_socket_free_registration_index (rp - socket_main.registration_pool);
}
static void
vl_socket_request_remove_reg_index (u32 reg_index)
{
vl_api_registration_t *rp = vl_socket_get_registration (reg_index);
ALWAYS_ASSERT (rp != 0);
if (rp->is_being_removed)
{
return;
}
rp->is_being_removed = 1;
vl_api_force_rpc_call_main_thread (
socket_cleanup_pending_remove_registration_cb, (void *) &reg_index,
sizeof (u32));
}
/*
* Read function for API socket.
*
@ -232,7 +259,6 @@ vl_socket_process_api_msg (vl_api_registration_t * rp, i8 * input_v)
clib_error_t *
vl_socket_read_ready (clib_file_t * uf)
{
clib_file_main_t *fm = &file_main;
vlib_main_t *vm = vlib_get_main ();
vl_api_registration_t *rp;
/* n is the size of data read to input_buffer */
@ -246,6 +272,10 @@ vl_socket_read_ready (clib_file_t * uf)
u32 save_input_buffer_length = vec_len (socket_main.input_buffer);
vl_socket_args_for_process_t *a;
u32 reg_index = uf->private_data;
if (is_being_removed_reg_index (reg_index))
{
return 0;
}
rp = vl_socket_get_registration (reg_index);
@ -258,8 +288,7 @@ vl_socket_read_ready (clib_file_t * uf)
if (errno != EAGAIN)
{
/* Severe error, close the file. */
clib_file_del (fm, uf);
vl_socket_free_registration_index (reg_index);
vl_socket_request_remove_reg_index (reg_index);
}
/* EAGAIN means we do not close the file, but no data to process anyway. */
return 0;
@ -354,7 +383,13 @@ vl_socket_write_ready (clib_file_t * uf)
vl_api_registration_t *rp;
int n;
rp = pool_elt_at_index (socket_main.registration_pool, uf->private_data);
u32 reg_index = uf->private_data;
if (is_being_removed_reg_index (reg_index))
{
return 0;
}
rp = pool_elt_at_index (socket_main.registration_pool, reg_index);
/* Flush output vector. */
size_t total_bytes = vec_len (rp->output_vector);
@ -373,9 +408,7 @@ vl_socket_write_ready (clib_file_t * uf)
#if DEBUG > 2
clib_warning ("write error, close the file...\n");
#endif
clib_file_del (fm, uf);
vl_socket_free_registration_index (rp -
socket_main.registration_pool);
vl_socket_request_remove_reg_index (reg_index);
return 0;
}
remaining_bytes -= bytes_to_send;
@ -396,13 +429,8 @@ vl_socket_write_ready (clib_file_t * uf)
clib_error_t *
vl_socket_error_ready (clib_file_t * uf)
{
vl_api_registration_t *rp;
clib_file_main_t *fm = &file_main;
rp = pool_elt_at_index (socket_main.registration_pool, uf->private_data);
clib_file_del (fm, uf);
vl_socket_free_registration_index (rp - socket_main.registration_pool);
u32 reg_index = uf->private_data;
vl_socket_request_remove_reg_index (reg_index);
return 0;
}