Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1562510

Re: kvm: use-after-free in process_srcu

Show all headers | View raw

On 18/01/2017 23:15, Paul E. McKenney wrote:
> On Wed, Jan 18, 2017 at 09:53:19AM +0100, Paolo Bonzini wrote:
>>
>>
>> On 17/01/2017 21:34, Paul E. McKenney wrote:
>>> Do any of your callback functions invoke call_srcu()?  (Hey, I have to ask!)
>>
>> No, we only use synchronize_srcu and synchronize_srcu_expedited, so our
>> only callback comes from there.
> 
> OK, so the next question is whether your code makes sure that all of its
> synchronize_srcu() and synchronize_srcu_expedited() calls return before
> the call to cleanup_srcu_struct().

It certainly should!  Or at least that would be our bug.

> You should only need srcu_barrier() if there were calls to call_srcu().
> Given that you only have synchronize_srcu() and synchronize_srcu_expedited(),
> you -don't- need srcu_barrier().  What you need instead is to make sure
> that all synchronize_srcu() and synchronize_srcu_expedited() have
> returned before the call to cleanup_srcu_struct().

Ok, good.

>> If this is incorrect, then one flush_delayed_work is enough.  If it is
>> correct, the possible alternatives are:
>>
>> * srcu_barrier in the caller, flush_delayed_work+WARN_ON(sp->running) in
>> cleanup_srcu_struct.  I strongly dislike this one---because we don't use
>> call_srcu at all, there should be no reason to use srcu_barrier in KVM
>> code.  Plus I think all other users have the same issue.
>>
>> * srcu_barrier+flush_delayed_work+WARN_ON(sp->running) in
>> cleanup_srcu_struct
>>
>> * flush_delayed_work+flush_delayed_work+WARN_ON(sp->running) in
>> cleanup_srcu_struct
>>
>> * while(flush_delayed_work) in cleanup_srcu_struct
>>
>> * "while(sp->running) flush_delayed_work" in cleanup_srcu_struct
> 
> My current thought is flush_delayed_work() followed by a warning if
> there are any callbacks still posted, and also as you say sp->running.

Yes, that would work for KVM and anyone else who doesn't use call_srcu
(and order synchronize_srcu correctly against destruction).

On the other hand, users of call_srcu, such as rcutorture, _do_ need to
place an srcu_barrier before cleanup_srcu_struct, or they need two
flush_delayed_work() calls back to back in cleanup_srcu_struct.  Do you
agree?

Thanks,

Paolo

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: kvm: use-after-free in process_srcu Dmitry Vyukov <dvyukov@google.com> - 2017-01-16 22:40 +0100
  Re: kvm: use-after-free in process_srcu Paolo Bonzini <pbonzini@redhat.com> - 2017-01-16 22:50 +0100
    Re: kvm: use-after-free in process_srcu Dmitry Vyukov <dvyukov@google.com> - 2017-01-17 10:50 +0100
      Re: kvm: use-after-free in process_srcu Dmitry Vyukov <dvyukov@google.com> - 2017-01-17 11:00 +0100
        Re: kvm: use-after-free in process_srcu Paolo Bonzini <pbonzini@redhat.com> - 2017-01-17 12:10 +0100
          Re: kvm: use-after-free in process_srcu Dmitry Vyukov <dvyukov@google.com> - 2017-01-17 12:20 +0100
            Re: kvm: use-after-free in process_srcu Paolo Bonzini <pbonzini@redhat.com> - 2017-01-17 13:10 +0100
              Re: kvm: use-after-free in process_srcu "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-17 22:10 +0100
                Re: kvm: use-after-free in process_srcu Paolo Bonzini <pbonzini@redhat.com> - 2017-01-18 10:00 +0100
                Re: kvm: use-after-free in process_srcu "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-01-19 03:50 +0100
                Re: kvm: use-after-free in process_srcu Paolo Bonzini <pbonzini@redhat.com> - 2017-01-19 10:30 +0100
                Re: kvm: use-after-free in process_srcu Paul McKenney <paulmckrcu@gmail.com> - 2017-01-19 23:10 +0100

csiph-web