* [PATCH net v4 0/3] Bug fixes from XDP and perout series
@ 2025-04-15 9:05 Meghana Malladi
2025-04-15 9:05 ` [PATCH net v4 1/3] net: ti: icssg-prueth: Fix kernel warning while bringing down network interface Meghana Malladi
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Meghana Malladi @ 2025-04-15 9:05 UTC (permalink / raw)
To: dan.carpenter, javier.carrasco.cruz, diogo.ivo, horms,
jacob.e.keller, m-malladi, john.fastabend, hawk, daniel, ast,
richardcochran, pabeni, kuba, edumazet, davem, andrew+netdev
Cc: bpf, linux-kernel, netdev, linux-arm-kernel, srk,
Vignesh Raghavendra, Roger Quadros, danishanwar
This patch series consists of bug fixes from the XDP series:
1. Fixes a kernel warning that occurs when bringing down the
network interface.
2. Resolves a potential NULL pointer dereference in the
emac_xmit_xdp_frame() function.
3. Resolves a potential NULL pointer dereference in the
icss_iep_perout_enable() function
v3: https://lore.kernel.org/all/20250328102403.2626974-1-m-malladi@ti.com/
Meghana Malladi (3):
net: ti: icssg-prueth: Fix kernel warning while bringing down network
interface
net: ti: icssg-prueth: Fix possible NULL pointer dereference inside
emac_xmit_xdp_frame()
net: ti: icss-iep: Fix possible NULL pointer dereference for perout
request
drivers/net/ethernet/ti/icssg/icss_iep.c | 121 +++++++++----------
drivers/net/ethernet/ti/icssg/icssg_common.c | 9 +-
2 files changed, 62 insertions(+), 68 deletions(-)
base-commit: 8c941f14a694b40a91d381e77bcd334622aa7196
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net v4 1/3] net: ti: icssg-prueth: Fix kernel warning while bringing down network interface
2025-04-15 9:05 [PATCH net v4 0/3] Bug fixes from XDP and perout series Meghana Malladi
@ 2025-04-15 9:05 ` Meghana Malladi
2025-04-15 17:41 ` Jacob Keller
2025-04-15 9:05 ` [PATCH net v4 2/3] net: ti: icssg-prueth: Fix possible NULL pointer dereference inside emac_xmit_xdp_frame() Meghana Malladi
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Meghana Malladi @ 2025-04-15 9:05 UTC (permalink / raw)
To: dan.carpenter, javier.carrasco.cruz, diogo.ivo, horms,
jacob.e.keller, m-malladi, john.fastabend, hawk, daniel, ast,
richardcochran, pabeni, kuba, edumazet, davem, andrew+netdev
Cc: bpf, linux-kernel, netdev, linux-arm-kernel, srk,
Vignesh Raghavendra, Roger Quadros, danishanwar
During network interface initialization, the NIC driver needs to register
its Rx queue with the XDP, to ensure the incoming XDP buffer carries a
pointer reference to this info and is stored inside xdp_rxq_info.
While this struct isn't tied to XDP prog, if there are any changes in
Rx queue, the NIC driver needs to stop the Rx queue by unregistering
with XDP before purging and reallocating memory. Drop page_pool destroy
during Rx channel reset as this is already handled by XDP during
xdp_rxq_info_unreg (Rx queue unregister), failing to do will cause the
following warning:
warning logs: https://gist.github.com/MeghanaMalladiTI/eb627e5dc8de24e42d7d46572c13e576
Fixes: 46eeb90f03e0 ("net: ti: icssg-prueth: Use page_pool API for RX buffer allocation")
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Roger Quadros <rogerq@kernel.org>
---
Changes from v3 (v4-v3):
- Collected RB tag from Roger Quadros <rogerq@kernel.org>
drivers/net/ethernet/ti/icssg/icssg_common.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index 14002b026452..ec643fb69d30 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -1215,9 +1215,6 @@ void prueth_reset_rx_chan(struct prueth_rx_chn *chn,
prueth_rx_cleanup);
if (disable)
k3_udma_glue_disable_rx_chn(chn->rx_chn);
-
- page_pool_destroy(chn->pg_pool);
- chn->pg_pool = NULL;
}
EXPORT_SYMBOL_GPL(prueth_reset_rx_chan);
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net v4 2/3] net: ti: icssg-prueth: Fix possible NULL pointer dereference inside emac_xmit_xdp_frame()
2025-04-15 9:05 [PATCH net v4 0/3] Bug fixes from XDP and perout series Meghana Malladi
2025-04-15 9:05 ` [PATCH net v4 1/3] net: ti: icssg-prueth: Fix kernel warning while bringing down network interface Meghana Malladi
@ 2025-04-15 9:05 ` Meghana Malladi
2025-04-15 17:42 ` Jacob Keller
2025-04-15 9:05 ` [PATCH net v4 3/3] net: ti: icss-iep: Fix possible NULL pointer dereference for perout request Meghana Malladi
2025-04-17 10:30 ` [PATCH net v4 0/3] Bug fixes from XDP and perout series patchwork-bot+netdevbpf
3 siblings, 1 reply; 8+ messages in thread
From: Meghana Malladi @ 2025-04-15 9:05 UTC (permalink / raw)
To: dan.carpenter, javier.carrasco.cruz, diogo.ivo, horms,
jacob.e.keller, m-malladi, john.fastabend, hawk, daniel, ast,
richardcochran, pabeni, kuba, edumazet, davem, andrew+netdev
Cc: bpf, linux-kernel, netdev, linux-arm-kernel, srk,
Vignesh Raghavendra, Roger Quadros, danishanwar
There is an error check inside emac_xmit_xdp_frame() function which
is called when the driver wants to transmit XDP frame, to check if
the allocated tx descriptor is NULL, if true to exit and return
ICSSG_XDP_CONSUMED implying failure in transmission.
In this case trying to free a descriptor which is NULL will result
in kernel crash due to NULL pointer dereference. Fix this error handling
and increase netdev tx_dropped stats in the caller of this function
if the function returns ICSSG_XDP_CONSUMED.
Fixes: 62aa3246f462 ("net: ti: icssg-prueth: Add XDP support")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/70d8dd76-0c76-42fc-8611-9884937c82f5@stanley.mountain/
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Roger Quadros <rogerq@kernel.org>
---
Changes from v3 (v4-v3):
- Collected RB tag from Roger Quadros <rogerq@kernel.org>
drivers/net/ethernet/ti/icssg/icssg_common.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index ec643fb69d30..b4be76e13a2f 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -583,7 +583,7 @@ u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
first_desc = k3_cppi_desc_pool_alloc(tx_chn->desc_pool);
if (!first_desc) {
netdev_dbg(ndev, "xdp tx: failed to allocate descriptor\n");
- goto drop_free_descs; /* drop */
+ return ICSSG_XDP_CONSUMED; /* drop */
}
if (page) { /* already DMA mapped by page_pool */
@@ -671,8 +671,10 @@ static u32 emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp,
q_idx = smp_processor_id() % emac->tx_ch_num;
result = emac_xmit_xdp_frame(emac, xdpf, page, q_idx);
- if (result == ICSSG_XDP_CONSUMED)
+ if (result == ICSSG_XDP_CONSUMED) {
+ ndev->stats.tx_dropped++;
goto drop;
+ }
dev_sw_netstats_rx_add(ndev, xdpf->len);
return result;
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net v4 3/3] net: ti: icss-iep: Fix possible NULL pointer dereference for perout request
2025-04-15 9:05 [PATCH net v4 0/3] Bug fixes from XDP and perout series Meghana Malladi
2025-04-15 9:05 ` [PATCH net v4 1/3] net: ti: icssg-prueth: Fix kernel warning while bringing down network interface Meghana Malladi
2025-04-15 9:05 ` [PATCH net v4 2/3] net: ti: icssg-prueth: Fix possible NULL pointer dereference inside emac_xmit_xdp_frame() Meghana Malladi
@ 2025-04-15 9:05 ` Meghana Malladi
2025-04-15 17:45 ` Jacob Keller
2025-04-17 10:30 ` [PATCH net v4 0/3] Bug fixes from XDP and perout series patchwork-bot+netdevbpf
3 siblings, 1 reply; 8+ messages in thread
From: Meghana Malladi @ 2025-04-15 9:05 UTC (permalink / raw)
To: dan.carpenter, javier.carrasco.cruz, diogo.ivo, horms,
jacob.e.keller, m-malladi, john.fastabend, hawk, daniel, ast,
richardcochran, pabeni, kuba, edumazet, davem, andrew+netdev
Cc: bpf, linux-kernel, netdev, linux-arm-kernel, srk,
Vignesh Raghavendra, Roger Quadros, danishanwar
The ICSS IEP driver tracks perout and pps enable state with flags.
Currently when disabling pps and perout signals during icss_iep_exit(),
results in NULL pointer dereference for perout.
To fix the null pointer dereference issue, the icss_iep_perout_enable_hw
function can be modified to directly clear the IEP CMP registers when
disabling PPS or PEROUT, without referencing the ptp_perout_request
structure, as its contents are irrelevant in this case.
Fixes: 9b115361248d ("net: ti: icssg-prueth: Fix clearing of IEP_CMP_CFG registers during iep_init")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/7b1c7c36-363a-4085-b26c-4f210bee1df6@stanley.mountain/
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---
Changes from v3 (v4-v3):
- Fix the logic in icss_iep_perout_enable_hw() to clear IEP registers
when disabling periodic signal
drivers/net/ethernet/ti/icssg/icss_iep.c | 121 +++++++++++------------
1 file changed, 58 insertions(+), 63 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c
index b4a34c57b7b4..2a1c43316f46 100644
--- a/drivers/net/ethernet/ti/icssg/icss_iep.c
+++ b/drivers/net/ethernet/ti/icssg/icss_iep.c
@@ -412,6 +412,22 @@ static int icss_iep_perout_enable_hw(struct icss_iep *iep,
int ret;
u64 cmp;
+ if (!on) {
+ /* Disable CMP 1 */
+ regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
+ IEP_CMP_CFG_CMP_EN(1), 0);
+
+ /* clear CMP regs */
+ regmap_write(iep->map, ICSS_IEP_CMP1_REG0, 0);
+ if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT)
+ regmap_write(iep->map, ICSS_IEP_CMP1_REG1, 0);
+
+ /* Disable sync */
+ regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG, 0);
+
+ return 0;
+ }
+
/* Calculate width of the signal for PPS/PEROUT handling */
ts.tv_sec = req->on.sec;
ts.tv_nsec = req->on.nsec;
@@ -430,64 +446,39 @@ static int icss_iep_perout_enable_hw(struct icss_iep *iep,
if (ret)
return ret;
- if (on) {
- /* Configure CMP */
- regmap_write(iep->map, ICSS_IEP_CMP1_REG0, lower_32_bits(cmp));
- if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT)
- regmap_write(iep->map, ICSS_IEP_CMP1_REG1, upper_32_bits(cmp));
- /* Configure SYNC, based on req on width */
- regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG,
- div_u64(ns_width, iep->def_inc));
- regmap_write(iep->map, ICSS_IEP_SYNC0_PERIOD_REG, 0);
- regmap_write(iep->map, ICSS_IEP_SYNC_START_REG,
- div_u64(ns_start, iep->def_inc));
- regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG, 0); /* one-shot mode */
- /* Enable CMP 1 */
- regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
- IEP_CMP_CFG_CMP_EN(1), IEP_CMP_CFG_CMP_EN(1));
- } else {
- /* Disable CMP 1 */
- regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
- IEP_CMP_CFG_CMP_EN(1), 0);
-
- /* clear regs */
- regmap_write(iep->map, ICSS_IEP_CMP1_REG0, 0);
- if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT)
- regmap_write(iep->map, ICSS_IEP_CMP1_REG1, 0);
- }
+ /* Configure CMP */
+ regmap_write(iep->map, ICSS_IEP_CMP1_REG0, lower_32_bits(cmp));
+ if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT)
+ regmap_write(iep->map, ICSS_IEP_CMP1_REG1, upper_32_bits(cmp));
+ /* Configure SYNC, based on req on width */
+ regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG,
+ div_u64(ns_width, iep->def_inc));
+ regmap_write(iep->map, ICSS_IEP_SYNC0_PERIOD_REG, 0);
+ regmap_write(iep->map, ICSS_IEP_SYNC_START_REG,
+ div_u64(ns_start, iep->def_inc));
+ regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG, 0); /* one-shot mode */
+ /* Enable CMP 1 */
+ regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
+ IEP_CMP_CFG_CMP_EN(1), IEP_CMP_CFG_CMP_EN(1));
} else {
- if (on) {
- u64 start_ns;
-
- iep->period = ((u64)req->period.sec * NSEC_PER_SEC) +
- req->period.nsec;
- start_ns = ((u64)req->period.sec * NSEC_PER_SEC)
- + req->period.nsec;
- icss_iep_update_to_next_boundary(iep, start_ns);
-
- regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG,
- div_u64(ns_width, iep->def_inc));
- regmap_write(iep->map, ICSS_IEP_SYNC_START_REG,
- div_u64(ns_start, iep->def_inc));
- /* Enable Sync in single shot mode */
- regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG,
- IEP_SYNC_CTRL_SYNC_N_EN(0) | IEP_SYNC_CTRL_SYNC_EN);
- /* Enable CMP 1 */
- regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
- IEP_CMP_CFG_CMP_EN(1), IEP_CMP_CFG_CMP_EN(1));
- } else {
- /* Disable CMP 1 */
- regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
- IEP_CMP_CFG_CMP_EN(1), 0);
-
- /* clear CMP regs */
- regmap_write(iep->map, ICSS_IEP_CMP1_REG0, 0);
- if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT)
- regmap_write(iep->map, ICSS_IEP_CMP1_REG1, 0);
-
- /* Disable sync */
- regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG, 0);
- }
+ u64 start_ns;
+
+ iep->period = ((u64)req->period.sec * NSEC_PER_SEC) +
+ req->period.nsec;
+ start_ns = ((u64)req->period.sec * NSEC_PER_SEC)
+ + req->period.nsec;
+ icss_iep_update_to_next_boundary(iep, start_ns);
+
+ regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG,
+ div_u64(ns_width, iep->def_inc));
+ regmap_write(iep->map, ICSS_IEP_SYNC_START_REG,
+ div_u64(ns_start, iep->def_inc));
+ /* Enable Sync in single shot mode */
+ regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG,
+ IEP_SYNC_CTRL_SYNC_N_EN(0) | IEP_SYNC_CTRL_SYNC_EN);
+ /* Enable CMP 1 */
+ regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
+ IEP_CMP_CFG_CMP_EN(1), IEP_CMP_CFG_CMP_EN(1));
}
return 0;
@@ -498,11 +489,21 @@ static int icss_iep_perout_enable(struct icss_iep *iep,
{
int ret = 0;
+ if (!on)
+ goto disable;
+
/* Reject requests with unsupported flags */
if (req->flags & ~(PTP_PEROUT_DUTY_CYCLE |
PTP_PEROUT_PHASE))
return -EOPNOTSUPP;
+ /* Set default "on" time (1ms) for the signal if not passed by the app */
+ if (!(req->flags & PTP_PEROUT_DUTY_CYCLE)) {
+ req->on.sec = 0;
+ req->on.nsec = NSEC_PER_MSEC;
+ }
+
+disable:
mutex_lock(&iep->ptp_clk_mutex);
if (iep->pps_enabled) {
@@ -513,12 +514,6 @@ static int icss_iep_perout_enable(struct icss_iep *iep,
if (iep->perout_enabled == !!on)
goto exit;
- /* Set default "on" time (1ms) for the signal if not passed by the app */
- if (!(req->flags & PTP_PEROUT_DUTY_CYCLE)) {
- req->on.sec = 0;
- req->on.nsec = NSEC_PER_MSEC;
- }
-
ret = icss_iep_perout_enable_hw(iep, req, on);
if (!ret)
iep->perout_enabled = !!on;
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v4 1/3] net: ti: icssg-prueth: Fix kernel warning while bringing down network interface
2025-04-15 9:05 ` [PATCH net v4 1/3] net: ti: icssg-prueth: Fix kernel warning while bringing down network interface Meghana Malladi
@ 2025-04-15 17:41 ` Jacob Keller
0 siblings, 0 replies; 8+ messages in thread
From: Jacob Keller @ 2025-04-15 17:41 UTC (permalink / raw)
To: Meghana Malladi, dan.carpenter, javier.carrasco.cruz, diogo.ivo,
horms, john.fastabend, hawk, daniel, ast, richardcochran, pabeni,
kuba, edumazet, davem, andrew+netdev
Cc: bpf, linux-kernel, netdev, linux-arm-kernel, srk,
Vignesh Raghavendra, Roger Quadros, danishanwar
On 4/15/2025 2:05 AM, Meghana Malladi wrote:
> During network interface initialization, the NIC driver needs to register
> its Rx queue with the XDP, to ensure the incoming XDP buffer carries a
> pointer reference to this info and is stored inside xdp_rxq_info.
>
> While this struct isn't tied to XDP prog, if there are any changes in
> Rx queue, the NIC driver needs to stop the Rx queue by unregistering
> with XDP before purging and reallocating memory. Drop page_pool destroy
> during Rx channel reset as this is already handled by XDP during
> xdp_rxq_info_unreg (Rx queue unregister), failing to do will cause the
> following warning:
>
> warning logs: https://gist.github.com/MeghanaMalladiTI/eb627e5dc8de24e42d7d46572c13e576
>
I generally would prefer a minified warning in the commit message,
(perhaps in addition to the link) as such due to linkrot.
Regardless, the fix looks accurate to me.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Fixes: 46eeb90f03e0 ("net: ti: icssg-prueth: Use page_pool API for RX buffer allocation")
> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
> Reviewed-by: Simon Horman <horms@kernel.org>
> Reviewed-by: Roger Quadros <rogerq@kernel.org>
> ---
>
> Changes from v3 (v4-v3):
> - Collected RB tag from Roger Quadros <rogerq@kernel.org>
>
> drivers/net/ethernet/ti/icssg/icssg_common.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index 14002b026452..ec643fb69d30 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> @@ -1215,9 +1215,6 @@ void prueth_reset_rx_chan(struct prueth_rx_chn *chn,
> prueth_rx_cleanup);
> if (disable)
> k3_udma_glue_disable_rx_chn(chn->rx_chn);
> -
> - page_pool_destroy(chn->pg_pool);
> - chn->pg_pool = NULL;
> }
> EXPORT_SYMBOL_GPL(prueth_reset_rx_chan);
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v4 2/3] net: ti: icssg-prueth: Fix possible NULL pointer dereference inside emac_xmit_xdp_frame()
2025-04-15 9:05 ` [PATCH net v4 2/3] net: ti: icssg-prueth: Fix possible NULL pointer dereference inside emac_xmit_xdp_frame() Meghana Malladi
@ 2025-04-15 17:42 ` Jacob Keller
0 siblings, 0 replies; 8+ messages in thread
From: Jacob Keller @ 2025-04-15 17:42 UTC (permalink / raw)
To: Meghana Malladi, dan.carpenter, javier.carrasco.cruz, diogo.ivo,
horms, john.fastabend, hawk, daniel, ast, richardcochran, pabeni,
kuba, edumazet, davem, andrew+netdev
Cc: bpf, linux-kernel, netdev, linux-arm-kernel, srk,
Vignesh Raghavendra, Roger Quadros, danishanwar
On 4/15/2025 2:05 AM, Meghana Malladi wrote:
> There is an error check inside emac_xmit_xdp_frame() function which
> is called when the driver wants to transmit XDP frame, to check if
> the allocated tx descriptor is NULL, if true to exit and return
> ICSSG_XDP_CONSUMED implying failure in transmission.
>
> In this case trying to free a descriptor which is NULL will result
> in kernel crash due to NULL pointer dereference. Fix this error handling
> and increase netdev tx_dropped stats in the caller of this function
> if the function returns ICSSG_XDP_CONSUMED.
>
> Fixes: 62aa3246f462 ("net: ti: icssg-prueth: Add XDP support")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/70d8dd76-0c76-42fc-8611-9884937c82f5@stanley.mountain/
> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
> Reviewed-by: Simon Horman <horms@kernel.org>
> Reviewed-by: Roger Quadros <rogerq@kernel.org>
> ---
>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Changes from v3 (v4-v3):
> - Collected RB tag from Roger Quadros <rogerq@kernel.org>
>
> drivers/net/ethernet/ti/icssg/icssg_common.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
> index ec643fb69d30..b4be76e13a2f 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_common.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
> @@ -583,7 +583,7 @@ u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
> first_desc = k3_cppi_desc_pool_alloc(tx_chn->desc_pool);
> if (!first_desc) {
> netdev_dbg(ndev, "xdp tx: failed to allocate descriptor\n");
> - goto drop_free_descs; /* drop */
> + return ICSSG_XDP_CONSUMED; /* drop */
> }
>
> if (page) { /* already DMA mapped by page_pool */
> @@ -671,8 +671,10 @@ static u32 emac_run_xdp(struct prueth_emac *emac, struct xdp_buff *xdp,
>
> q_idx = smp_processor_id() % emac->tx_ch_num;
> result = emac_xmit_xdp_frame(emac, xdpf, page, q_idx);
> - if (result == ICSSG_XDP_CONSUMED)
> + if (result == ICSSG_XDP_CONSUMED) {
> + ndev->stats.tx_dropped++;
> goto drop;
> + }
>
> dev_sw_netstats_rx_add(ndev, xdpf->len);
> return result;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v4 3/3] net: ti: icss-iep: Fix possible NULL pointer dereference for perout request
2025-04-15 9:05 ` [PATCH net v4 3/3] net: ti: icss-iep: Fix possible NULL pointer dereference for perout request Meghana Malladi
@ 2025-04-15 17:45 ` Jacob Keller
0 siblings, 0 replies; 8+ messages in thread
From: Jacob Keller @ 2025-04-15 17:45 UTC (permalink / raw)
To: Meghana Malladi, dan.carpenter, javier.carrasco.cruz, diogo.ivo,
horms, john.fastabend, hawk, daniel, ast, richardcochran, pabeni,
kuba, edumazet, davem, andrew+netdev
Cc: bpf, linux-kernel, netdev, linux-arm-kernel, srk,
Vignesh Raghavendra, Roger Quadros, danishanwar
On 4/15/2025 2:05 AM, Meghana Malladi wrote:
> The ICSS IEP driver tracks perout and pps enable state with flags.
> Currently when disabling pps and perout signals during icss_iep_exit(),
> results in NULL pointer dereference for perout.
>
> To fix the null pointer dereference issue, the icss_iep_perout_enable_hw
> function can be modified to directly clear the IEP CMP registers when
> disabling PPS or PEROUT, without referencing the ptp_perout_request
> structure, as its contents are irrelevant in this case.
>
> Fixes: 9b115361248d ("net: ti: icssg-prueth: Fix clearing of IEP_CMP_CFG registers during iep_init")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/7b1c7c36-363a-4085-b26c-4f210bee1df6@stanley.mountain/
> Signed-off-by: Meghana Malladi <m-malladi@ti.com>
> ---
>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Changes from v3 (v4-v3):
> - Fix the logic in icss_iep_perout_enable_hw() to clear IEP registers
> when disabling periodic signal
>
> drivers/net/ethernet/ti/icssg/icss_iep.c | 121 +++++++++++------------
> 1 file changed, 58 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/icssg/icss_iep.c b/drivers/net/ethernet/ti/icssg/icss_iep.c
> index b4a34c57b7b4..2a1c43316f46 100644
> --- a/drivers/net/ethernet/ti/icssg/icss_iep.c
> +++ b/drivers/net/ethernet/ti/icssg/icss_iep.c
> @@ -430,64 +446,39 @@ static int icss_iep_perout_enable_hw(struct icss_iep *iep,
> if (ret)
> return ret;
>
> - if (on) {
> - /* Configure CMP */
> - regmap_write(iep->map, ICSS_IEP_CMP1_REG0, lower_32_bits(cmp));
> - if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT)
> - regmap_write(iep->map, ICSS_IEP_CMP1_REG1, upper_32_bits(cmp));
> - /* Configure SYNC, based on req on width */
> - regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG,
> - div_u64(ns_width, iep->def_inc));
> - regmap_write(iep->map, ICSS_IEP_SYNC0_PERIOD_REG, 0);
> - regmap_write(iep->map, ICSS_IEP_SYNC_START_REG,
> - div_u64(ns_start, iep->def_inc));
> - regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG, 0); /* one-shot mode */
> - /* Enable CMP 1 */
> - regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
> - IEP_CMP_CFG_CMP_EN(1), IEP_CMP_CFG_CMP_EN(1));
> - } else {
> - /* Disable CMP 1 */
> - regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
> - IEP_CMP_CFG_CMP_EN(1), 0);
> -
> - /* clear regs */
> - regmap_write(iep->map, ICSS_IEP_CMP1_REG0, 0);
> - if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT)
> - regmap_write(iep->map, ICSS_IEP_CMP1_REG1, 0);
> - }
> + /* Configure CMP */
> + regmap_write(iep->map, ICSS_IEP_CMP1_REG0, lower_32_bits(cmp));
> + if (iep->plat_data->flags & ICSS_IEP_64BIT_COUNTER_SUPPORT)
> + regmap_write(iep->map, ICSS_IEP_CMP1_REG1, upper_32_bits(cmp));
> + /* Configure SYNC, based on req on width */
> + regmap_write(iep->map, ICSS_IEP_SYNC_PWIDTH_REG,
> + div_u64(ns_width, iep->def_inc));
> + regmap_write(iep->map, ICSS_IEP_SYNC0_PERIOD_REG, 0);
> + regmap_write(iep->map, ICSS_IEP_SYNC_START_REG,
> + div_u64(ns_start, iep->def_inc));
> + regmap_write(iep->map, ICSS_IEP_SYNC_CTRL_REG, 0); /* one-shot mode */
> + /* Enable CMP 1 */
> + regmap_update_bits(iep->map, ICSS_IEP_CMP_CFG_REG,
> + IEP_CMP_CFG_CMP_EN(1), IEP_CMP_CFG_CMP_EN(1));
Nice to see this also has a marked improvement with removing a level of
indentation.
> @@ -498,11 +489,21 @@ static int icss_iep_perout_enable(struct icss_iep *iep,
> {
> int ret = 0;
>
> + if (!on)
> + goto disable;
> +
> /* Reject requests with unsupported flags */
> if (req->flags & ~(PTP_PEROUT_DUTY_CYCLE |
> PTP_PEROUT_PHASE))
> return -EOPNOTSUPP;
>
This likely causes a textual conflict with my .supported_perout_flags
patch. It looks like it wouldn't be too difficult to resolve though.
> + /* Set default "on" time (1ms) for the signal if not passed by the app */
> + if (!(req->flags & PTP_PEROUT_DUTY_CYCLE)) {
> + req->on.sec = 0;
> + req->on.nsec = NSEC_PER_MSEC;
> + }
> +
Regards,
Jake
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v4 0/3] Bug fixes from XDP and perout series
2025-04-15 9:05 [PATCH net v4 0/3] Bug fixes from XDP and perout series Meghana Malladi
` (2 preceding siblings ...)
2025-04-15 9:05 ` [PATCH net v4 3/3] net: ti: icss-iep: Fix possible NULL pointer dereference for perout request Meghana Malladi
@ 2025-04-17 10:30 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-04-17 10:30 UTC (permalink / raw)
To: Meghana Malladi
Cc: dan.carpenter, javier.carrasco.cruz, diogo.ivo, horms,
jacob.e.keller, john.fastabend, hawk, daniel, ast,
richardcochran, pabeni, kuba, edumazet, davem, andrew+netdev,
bpf, linux-kernel, netdev, linux-arm-kernel, srk, vigneshr,
rogerq, danishanwar
Hello:
This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 15 Apr 2025 14:35:40 +0530 you wrote:
> This patch series consists of bug fixes from the XDP series:
> 1. Fixes a kernel warning that occurs when bringing down the
> network interface.
> 2. Resolves a potential NULL pointer dereference in the
> emac_xmit_xdp_frame() function.
> 3. Resolves a potential NULL pointer dereference in the
> icss_iep_perout_enable() function
>
> [...]
Here is the summary with links:
- [net,v4,1/3] net: ti: icssg-prueth: Fix kernel warning while bringing down network interface
https://git.kernel.org/netdev/net/c/75bc74446644
- [net,v4,2/3] net: ti: icssg-prueth: Fix possible NULL pointer dereference inside emac_xmit_xdp_frame()
https://git.kernel.org/netdev/net/c/8ed2fa661350
- [net,v4,3/3] net: ti: icss-iep: Fix possible NULL pointer dereference for perout request
https://git.kernel.org/netdev/net/c/7349c9e99793
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-04-17 10:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-15 9:05 [PATCH net v4 0/3] Bug fixes from XDP and perout series Meghana Malladi
2025-04-15 9:05 ` [PATCH net v4 1/3] net: ti: icssg-prueth: Fix kernel warning while bringing down network interface Meghana Malladi
2025-04-15 17:41 ` Jacob Keller
2025-04-15 9:05 ` [PATCH net v4 2/3] net: ti: icssg-prueth: Fix possible NULL pointer dereference inside emac_xmit_xdp_frame() Meghana Malladi
2025-04-15 17:42 ` Jacob Keller
2025-04-15 9:05 ` [PATCH net v4 3/3] net: ti: icss-iep: Fix possible NULL pointer dereference for perout request Meghana Malladi
2025-04-15 17:45 ` Jacob Keller
2025-04-17 10:30 ` [PATCH net v4 0/3] Bug fixes from XDP and perout series patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®