* [PATCH net] net: openvswitch: fix possible kfree_skb of ERR_PTR
@ 2026-06-04 12:19 Adrian Moreno
2026-06-04 15:36 ` [ovs-dev] " Aaron Conole
2026-06-05 6:32 ` Eelco Chaudron
0 siblings, 2 replies; 4+ messages in thread
From: Adrian Moreno @ 2026-06-04 12:19 UTC (permalink / raw)
To: netdev
Cc: Adrian Moreno, Aaron Conole, Eelco Chaudron, Ilya Maximets,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Pravin B Shelar, Jarno Rajahalme,
open list:OPENVSWITCH, open list
After the patch in the "Fixes" tag, the allocation of the "reply" skb
can happen either before or after locking the ovs_mutex.
However, error cleanups still follow the classical reversed order,
assuming "reply" is allocated before locking: it is freed after unlocking.
If "reply" allocation happens after locking the mutex and it fails,
"reply" is left with an ERR_PTR, and execution jumps to the correspondent
cleanup stage which will try to free an invalid pointer.
Fix this by setting the pointer to NULL after having saved its error
value.
Fixes: 893f139b9a6c ("openvswitch: Minimize ovs_flow_cmd_new|set critical sections.")
Signed-off-by: Adrian Moreno <amorenoz@redhat•com>
---
net/openvswitch/datapath.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index bbbde50fc649..f0164817d9b7 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1316,6 +1316,7 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info)
if (IS_ERR(reply)) {
error = PTR_ERR(reply);
+ reply = NULL;
goto err_unlock_ovs;
}
}
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [ovs-dev] [PATCH net] net: openvswitch: fix possible kfree_skb of ERR_PTR
2026-06-04 12:19 [PATCH net] net: openvswitch: fix possible kfree_skb of ERR_PTR Adrian Moreno
@ 2026-06-04 15:36 ` Aaron Conole
[not found] ` <CAJ0BgHcpo68Cx0tL_rRFFzZm8vNpDagF-hfy9KgUhPh4maGfFQ@mail.gmail.com>
2026-06-05 6:32 ` Eelco Chaudron
1 sibling, 1 reply; 4+ messages in thread
From: Aaron Conole @ 2026-06-04 15:36 UTC (permalink / raw)
To: Adrian Moreno via dev
Cc: netdev, Adrian Moreno, open list:OPENVSWITCH, Paolo Abeni,
Pravin B Shelar, Ilya Maximets, open list, Eric Dumazet,
Simon Horman, Jarno Rajahalme, Jakub Kicinski, David S. Miller,
Minxi Hou
Hi Adrian,
Adrian Moreno via dev <ovs-dev@openvswitch•org> writes:
> After the patch in the "Fixes" tag, the allocation of the "reply" skb
> can happen either before or after locking the ovs_mutex.
>
> However, error cleanups still follow the classical reversed order,
> assuming "reply" is allocated before locking: it is freed after unlocking.
>
> If "reply" allocation happens after locking the mutex and it fails,
> "reply" is left with an ERR_PTR, and execution jumps to the correspondent
> cleanup stage which will try to free an invalid pointer.
>
> Fix this by setting the pointer to NULL after having saved its error
> value.
>
> Fixes: 893f139b9a6c ("openvswitch: Minimize ovs_flow_cmd_new|set
> critical sections.")
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat•com>
> ---
Good catch - I guess this should only happen when modifying an existing
flow without putting any actions (and that would be only from an
implicit drop case since the actions list would be empty). CC'ing
Minxi, since he's recently had interest in the selftests area and may be
able to help with writing a test case for the scenario.
Reviewed-by: Aaron Conole <aconole@redhat•com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: openvswitch: fix possible kfree_skb of ERR_PTR
2026-06-04 12:19 [PATCH net] net: openvswitch: fix possible kfree_skb of ERR_PTR Adrian Moreno
2026-06-04 15:36 ` [ovs-dev] " Aaron Conole
@ 2026-06-05 6:32 ` Eelco Chaudron
1 sibling, 0 replies; 4+ messages in thread
From: Eelco Chaudron @ 2026-06-05 6:32 UTC (permalink / raw)
To: Adrian Moreno
Cc: netdev, Aaron Conole, Ilya Maximets, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Pravin B Shelar, Jarno Rajahalme, open list:OPENVSWITCH,
open list
On 4 Jun 2026, at 14:19, Adrian Moreno wrote:
> After the patch in the "Fixes" tag, the allocation of the "reply" skb
> can happen either before or after locking the ovs_mutex.
>
> However, error cleanups still follow the classical reversed order,
> assuming "reply" is allocated before locking: it is freed after unlocking.
>
> If "reply" allocation happens after locking the mutex and it fails,
> "reply" is left with an ERR_PTR, and execution jumps to the correspondent
> cleanup stage which will try to free an invalid pointer.
>
> Fix this by setting the pointer to NULL after having saved its error
> value.
>
> Fixes: 893f139b9a6c ("openvswitch: Minimize ovs_flow_cmd_new|set critical sections.")
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat•com>
Thanks Adrian for finding and fixing this! The change looks good to me.
Acked-by: Eelco Chaudron <echaudro@redhat•com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [ovs-dev] [PATCH net] net: openvswitch: fix possible kfree_skb of ERR_PTR
[not found] ` <CAJ0BgHcpo68Cx0tL_rRFFzZm8vNpDagF-hfy9KgUhPh4maGfFQ@mail.gmail.com>
@ 2026-06-05 23:50 ` Jakub Kicinski
0 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-06-05 23:50 UTC (permalink / raw)
To: 侯敏熙
Cc: Aaron Conole, Adrian Moreno via dev, netdev, Adrian Moreno,
open list:OPENVSWITCH, Paolo Abeni, Pravin B Shelar,
Ilya Maximets, open list, Eric Dumazet, Simon Horman,
Jarno Rajahalme, David S. Miller
On Fri, 5 Jun 2026 01:10:16 +0800 侯敏熙 wrote:
> Should I target net or net-next for the test patch?
net-next, we _allow_ posting tests with the fixes for ease of testing
and review. But if the test comes later, anyway, it should just go to
net-next.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-05 23:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-04 12:19 [PATCH net] net: openvswitch: fix possible kfree_skb of ERR_PTR Adrian Moreno
2026-06-04 15:36 ` [ovs-dev] " Aaron Conole
[not found] ` <CAJ0BgHcpo68Cx0tL_rRFFzZm8vNpDagF-hfy9KgUhPh4maGfFQ@mail.gmail.com>
2026-06-05 23:50 ` Jakub Kicinski
2026-06-05 6:32 ` Eelco Chaudron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox