On 2025/3/27 17:32, Morten Brørup wrote: >> From: Dengdui Huang [mailto:huangdeng...@huawei.com] >> Sent: Thursday, 27 March 2025 10.01 >> >> The worker_id may come from user input. >> So it is necessary to verify it. >> >> Fixes: a95d70547c57 ("eal: factorize lcore main loop") >> Cc: sta...@dpdk.org >> >> Signed-off-by: Dengdui Huang <huangdeng...@huawei.com> >> --- >> lib/eal/common/eal_common_launch.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/lib/eal/common/eal_common_launch.c >> b/lib/eal/common/eal_common_launch.c >> index 5320c3bd3c..76313d5cdf 100644 >> --- a/lib/eal/common/eal_common_launch.c >> +++ b/lib/eal/common/eal_common_launch.c >> @@ -35,6 +35,9 @@ rte_eal_remote_launch(lcore_function_t *f, void *arg, >> unsigned int worker_id) >> { >> int rc = -EBUSY; >> > > Also check the lcore_id: > + if (unlikely(lcore_id >= RTE_MAX_LCORE)) > + return -EINVAL; > + > >> + if (!rte_lcore_has_role(worker_id, ROLE_RTE)) >> + return -EINVAL; >> + > > Although rte_lcore_has_role() checks the lcore_id and returns -EINVAL if > invalid, "if (!-EINVAL))" will not take the branch to return -EINVAL here. So > this check alone does not suffice. You could change it to "if > (rte_lcore_has_role(worker_id, ROLE_RTE) <= 0) return -EINVAL;". > However, I prefer the separate lcore_id check, as mentioned above, rather > than changing this check. > >> /* Check if the worker is in 'WAIT' state. Use acquire order >> * since 'state' variable is used as the guard variable. >> */ >> -- >> 2.33.0 > > <feature creep> > While you are at it, please also add "if (unlikely(lcore_id >= > RTE_MAX_LCORE)) return -EINVAL;" checks in the other functions in > eal_common_lcore.c where lcore_id comes from user input and the check is > missing. > </feature creep> > >
I'll modify it in v3 based on your suggestions above.
