Ack, thanks... See https://gerrit.fd.io/r/c/vpp/+/32688; merged in 
master/latest. Cherry-picked into stable/2106, should cherry-pick into older 
releases if needed.

 

D.

 

From: vpp-dev@lists.fd.io <vpp-dev@lists.fd.io> On Behalf Of sontu mazumdar
Sent: Sunday, June 13, 2021 1:35 PM
To: Dave Barach <v...@barachs.net>
Cc: vpp-dev@lists.fd.io
Subject: Re: [vpp-dev] "unix-cli-local" node corruption in node_by_name hash 
#vpp #vpp-dev

 

Thanks for the patch Dave. 

With this I am not seeing the corruption
issue of node hash key in node_by_name hash. 

 

Regards, 

Sontu

 

On Sat, 12 Jun, 2021, 11:09 AM sontu mazumdar, <sont...@gmail.com 
<mailto:sont...@gmail.com> > wrote:

Thanks Dave.

I will try with your suggested code changes and will share the result. 

 

Regards,

Sontu

 

On Fri, 11 Jun, 2021, 9:34 PM Dave Barach, <v...@barachs.net 
<mailto:v...@barachs.net> > wrote:

Please try these diffs and report results.

 

diff --git a/src/vlib/unix/cli.c b/src/vlib/unix/cli.c

index 6c9886725..ce29e0723 100644

--- a/src/vlib/unix/cli.c

+++ b/src/vlib/unix/cli.c

@@ -2863,6 +2863,7 @@ unix_cli_file_add (unix_cli_main_t * cm, char *name, int 
fd)

{

   unix_main_t *um = &unix_main;

   clib_file_main_t *fm = &file_main;

+  vlib_node_main_t *nm = &vlib_get_main()->node_main;

   unix_cli_file_t *cf;

   clib_file_t template = { 0 };

   vlib_main_t *vm = um->vlib_main;

@@ -2896,10 +2897,12 @@ unix_cli_file_add (unix_cli_main_t * cm, char *name, 
int fd)

       old_name = n->name;

       n->name = (u8 *) name;

     }

+      ASSERT(old_name);

+      hash_unset (nm->node_by_name, old_name);

+      hash_set (nm->node_by_name, name, n->index);

       vec_free (old_name);

       vlib_node_set_state (vm, n->index, VLIB_NODE_STATE_POLLING);

-

       _vec_len (cm->unused_cli_process_node_indices) = l - 1;

     }

   else

 

From: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io>  <vpp-dev@lists.fd.io 
<mailto:vpp-dev@lists.fd.io> > On Behalf Of sontu mazumdar
Sent: Friday, June 11, 2021 11:34 AM
To: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> 
Subject: [vpp-dev] "unix-cli-local" node corruption in node_by_name hash #vpp 
#vpp-dev

 

Hi,

 

I observe that in node_by_name hash we store a node with name 
"unix-cli-local:0" and node index 720 (not sure the purpose of the node).

The node name is stored as key in the node_by_name hash.

But later at some time when I print the node_by_name hash's each entry I see 
the key of node i.e the node name is printing some junk value (I figured it out 
via checking it against the node index).

 

When I looked at code in unix_cli_file_add(), below we are first time adding 
the node with name "unix-cli-local:0".

 

      static vlib_node_registration_t r = {

        .function = unix_cli_process,

        .type = VLIB_NODE_TYPE_PROCESS,

        .process_log2_n_stack_bytes = 18,

      };

 

      r.name <http://r.name>  = name;

 

      vlib_worker_thread_barrier_sync (vm);

 

      vlib_register_node (vm, &r);   <<<<<<<<<<<<

      vec_free (name);

 

      n = vlib_get_node (vm, r.index);

      vlib_worker_thread_node_runtime_update ();

      vlib_worker_thread_barrier_release (vm);

 

 

Later it again calls unix_cli_file_add(), there we pass a different name 
"unix-cli-local:1".

In this case we are overwriting the already existing node name from 
"unix-cli-local:0" to "unix-cli-local:1".

 

      for (i = 0; i < vec_len (vlib_mains); i++)

        {

          this_vlib_main = vlib_mains[i];

          if (this_vlib_main == 0)

            continue;

          n = vlib_get_node (this_vlib_main,

                             cm->unused_cli_process_node_indices[l - 1]);

          old_name = n->name;          <<<<<<<<<<<

          n->name = (u8 *) name;       <<<<<<<<<<<

        }

      vec_free (old_name);      <<<<<<<<<<

 

But the node name is already present in node_by_name hash as a key and there we 
haven't updated it instead we have deleted the old name.

This is resulting in printing some corrupted node name for the above node in 
node_by_name hash, which I think can sometimes results in VPP crash also as the 
hash key points to some freed memory.

 

Regards,

Sontu





-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#19564): https://lists.fd.io/g/vpp-dev/message/19564
Mute This Topic: https://lists.fd.io/mt/83471274/21656
Mute #vpp:https://lists.fd.io/g/vpp-dev/mutehashtag/vpp
Mute #vpp-dev:https://lists.fd.io/g/vpp-dev/mutehashtag/vpp-dev
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to