On Wed, Jun 03, 2020 at 01:22:52AM +0000, Linhaifeng wrote:
>
>
> -----Original Message-----
> From: Ben Pfaff [mailto:[email protected]]
> Sent: Wednesday, June 3, 2020 1:28 AM
> To: Linhaifeng <[email protected]>
> Cc: Yanqin Wei <[email protected]>; [email protected]; nd <[email protected]>;
> Lilijun (Jerry) <[email protected]>; chenchanghu
> <[email protected]>; Lichunhe <[email protected]>
> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
>
> On Tue, Jun 02, 2020 at 07:27:59AM +0000, Linhaifeng wrote:
> > We should update rcu pointer first then use ovsrcu_postpone to free
> > otherwise maybe cause use-after-free.
> > e.g.,reader indicates momentary quiescent and access old pointer after
> > writer postpone free old pointer and before setting new pointer.
> >
> > Signed-off-by: Linhaifeng <[email protected]>
>
> I don't see how that's possible, since the writer hasn't quiesced.
>
> I think the logic is as follow, Could you help me find out where is incorrect?
>
> 1.1 -> 1.2 -> 3.1 -> 3.2 -> 2.1 -> 2.2 -> 2.3 -> 2.1 -> 1.3 -> 1.4 -> 3.3 ->
> 2.2(use after free)
>
> wirter:
> 1.1 use postone to free old pointer
> 1.2 flush cbsets to flushed_cbsets
> 1.3 update new pointer
> 1.4 quiesced
>
> Read:
> 2.1. read pointer
> 2.2. use pointer
> 2.3. quiesced
>
> Rcu:
> 3.1 pop flushed_cbsets
> 3.2 ovsrcu_synchronize
> 3.3 call all cb to free
So you're saying this:
1.1 use postone to free old pointer (A)
1.2 flush cbsets to flushed_cbsets
3.1 pop flushed_cbsets
3.2 ovsrcu_synchronize
2.1. read pointer (A)
2.2. use pointer (A)
2.3. quiesced
2.1. read pointer (A)
1.3 update new pointer (B)
1.4 quiesced
3.3 call all cb to free (A)
2.2. use pointer (A)
Wow, you are absolutely right. This had never occurred to me. Thank
you! I'll review your patch.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev