telemetry: fix accessing callbacks list using lock

Message ID 20210505152248.253496-1-ciara.power@intel.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series telemetry: fix accessing callbacks list using lock |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Power, Ciara May 5, 2021, 3:22 p.m. UTC
  The list_commands() function accessed the callbacks list,
but did not take the lock. This may have caused inconsistencies if
callbacks were being registered at the same time.
This is now fixed to lock before iterating the list,
and unlock afterwards.

Fixes: f38748736eb2 ("telemetry: add default callback commands")
Cc: stable@dpdk.org

Signed-off-by: Ciara Power <ciara.power@intel.com>
Reported-by: David Marchand <david.marchand@redhat.com>
---
 lib/telemetry/telemetry.c | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Bruce Richardson May 5, 2021, 3:35 p.m. UTC | #1
On Wed, May 05, 2021 at 03:22:48PM +0000, Ciara Power wrote:
> The list_commands() function accessed the callbacks list,
> but did not take the lock. This may have caused inconsistencies if
> callbacks were being registered at the same time.
> This is now fixed to lock before iterating the list,
> and unlock afterwards.
> 
> Fixes: f38748736eb2 ("telemetry: add default callback commands")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> Reported-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  
David Marchand May 5, 2021, 4:15 p.m. UTC | #2
On Wed, May 5, 2021 at 5:36 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, May 05, 2021 at 03:22:48PM +0000, Ciara Power wrote:
> > The list_commands() function accessed the callbacks list,
> > but did not take the lock. This may have caused inconsistencies if
> > callbacks were being registered at the same time.
> > This is now fixed to lock before iterating the list,
> > and unlock afterwards.
> >
> > Fixes: f38748736eb2 ("telemetry: add default callback commands")
> > Cc: stable@dpdk.org
> >
> > Reported-by: David Marchand <david.marchand@redhat.com>
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks.
  

Patch

diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index 386d0080bc..68b479e0e4 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -104,8 +104,10 @@  list_commands(const char *cmd __rte_unused, const char *params __rte_unused,
 	int i;
 
 	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
+	rte_spinlock_lock(&callback_sl);
 	for (i = 0; i < num_callbacks; i++)
 		rte_tel_data_add_array_string(d, callbacks[i].cmd);
+	rte_spinlock_unlock(&callback_sl);
 	return 0;
 }