| Message ID | 20200129144948.2161551-1-luc.michel@greensocs.com |
|---|---|
| State | New |
| Headers | show |
| Series | seqlock: fix seqlock_write_unlock_impl function | expand |
On 1/29/20 3:49 PM, Luc Michel wrote: > The seqlock write unlock function was incorrectly calling > seqlock_write_begin() instead of seqlock_write_end(), and was releasing > the lock before incrementing the sequence. This could lead to a race > condition and a corrupted sequence number becoming odd even though the > lock is not held. I'm surprised it took 18 months to figure this out. Fixes: 988fcafc730 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Signed-off-by: Luc Michel <luc.michel@greensocs.com> > --- > include/qemu/seqlock.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h > index fd408b7ec5..8b6b4ee4bb 100644 > --- a/include/qemu/seqlock.h > +++ b/include/qemu/seqlock.h > @@ -53,15 +53,15 @@ static inline void seqlock_write_lock_impl(QemuSeqLock *sl, QemuLockable *lock) > seqlock_write_begin(sl); > } > #define seqlock_write_lock(sl, lock) \ > seqlock_write_lock_impl(sl, QEMU_MAKE_LOCKABLE(lock)) > > -/* Lock out other writers and update the count. */ > +/* Update the count and release the lock. */ > static inline void seqlock_write_unlock_impl(QemuSeqLock *sl, QemuLockable *lock) > { > + seqlock_write_end(sl); > qemu_lockable_unlock(lock); > - seqlock_write_begin(sl); > } > #define seqlock_write_unlock(sl, lock) \ > seqlock_write_unlock_impl(sl, QEMU_MAKE_LOCKABLE(lock)) > > >
On 29/01/20 15:49, Luc Michel wrote: > The seqlock write unlock function was incorrectly calling > seqlock_write_begin() instead of seqlock_write_end(), and was releasing > the lock before incrementing the sequence. This could lead to a race > condition and a corrupted sequence number becoming odd even though the > lock is not held. > > Signed-off-by: Luc Michel <luc.michel@greensocs.com> > --- > include/qemu/seqlock.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h > index fd408b7ec5..8b6b4ee4bb 100644 > --- a/include/qemu/seqlock.h > +++ b/include/qemu/seqlock.h > @@ -53,15 +53,15 @@ static inline void seqlock_write_lock_impl(QemuSeqLock *sl, QemuLockable *lock) > seqlock_write_begin(sl); > } > #define seqlock_write_lock(sl, lock) \ > seqlock_write_lock_impl(sl, QEMU_MAKE_LOCKABLE(lock)) > > -/* Lock out other writers and update the count. */ > +/* Update the count and release the lock. */ > static inline void seqlock_write_unlock_impl(QemuSeqLock *sl, QemuLockable *lock) > { > + seqlock_write_end(sl); > qemu_lockable_unlock(lock); > - seqlock_write_begin(sl); > } > #define seqlock_write_unlock(sl, lock) \ > seqlock_write_unlock_impl(sl, QEMU_MAKE_LOCKABLE(lock)) > > > Queued, thanks very much. Paolo
diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h index fd408b7ec5..8b6b4ee4bb 100644 --- a/include/qemu/seqlock.h +++ b/include/qemu/seqlock.h @@ -53,15 +53,15 @@ static inline void seqlock_write_lock_impl(QemuSeqLock *sl, QemuLockable *lock) seqlock_write_begin(sl); } #define seqlock_write_lock(sl, lock) \ seqlock_write_lock_impl(sl, QEMU_MAKE_LOCKABLE(lock)) -/* Lock out other writers and update the count. */ +/* Update the count and release the lock. */ static inline void seqlock_write_unlock_impl(QemuSeqLock *sl, QemuLockable *lock) { + seqlock_write_end(sl); qemu_lockable_unlock(lock); - seqlock_write_begin(sl); } #define seqlock_write_unlock(sl, lock) \ seqlock_write_unlock_impl(sl, QEMU_MAKE_LOCKABLE(lock))
The seqlock write unlock function was incorrectly calling seqlock_write_begin() instead of seqlock_write_end(), and was releasing the lock before incrementing the sequence. This could lead to a race condition and a corrupted sequence number becoming odd even though the lock is not held. Signed-off-by: Luc Michel <luc.michel@greensocs.com> --- include/qemu/seqlock.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)