[ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

Linhaifeng haifeng.lin at huawei.com
Wed Jun 3 07:04:58 UTC 2020



> -----Original Message-----
> From: Ben Pfaff [mailto:blp at ovn.org]
> Sent: Wednesday, June 3, 2020 1:26 PM
> To: Linhaifeng <haifeng.lin at huawei.com>
> Cc: Yanqin Wei <Yanqin.Wei at arm.com>; dev at openvswitch.org; nd
> <nd at arm.com>; Lilijun (Jerry) <jerry.lilijun at huawei.com>; chenchanghu
> <chenchanghu at huawei.com>; Lichunhe <lichunhe at huawei.com>
> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> 
> On Wed, Jun 03, 2020 at 01:22:52AM +0000, Linhaifeng wrote:
> >
> >
> > -----Original Message-----
> > From: Ben Pfaff [mailto:blp at ovn.org]
> > Sent: Wednesday, June 3, 2020 1:28 AM
> > To: Linhaifeng <haifeng.lin at huawei.com>
> > Cc: Yanqin Wei <Yanqin.Wei at arm.com>; dev at openvswitch.org; nd
> > <nd at arm.com>; Lilijun (Jerry) <jerry.lilijun at huawei.com>; chenchanghu
> > <chenchanghu at huawei.com>; Lichunhe <lichunhe at huawei.com>
> > 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 <haifeng.lin at huawei.com>
> >
> > 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.

Yes, it's really hard to happen. If it happened it's also hard to find the reason so I suggest it can be a rule for using rcu.


More information about the dev mailing list