The luo_flb_get_private() function, which is responsible for lazily
initializing the private state of FLB objects, can be called
concurrently from multiple threads. This creates a data
race on the 'initialized' flag and can lead to multiple executions of
mutex_init() and INIT_LIST_HEAD() on the same memory.
Introduce a static spinlock (luo_flb_init_lock) local to the function
to synchronize the initialization path. Use smp_load_acquire() and
smp_store_release() for memory ordering between the fast path and the
slow path.
Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
---
kernel/liveupdate/luo_flb.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/liveupdate/luo_flb.c b/kernel/liveupdate/luo_flb.c
index 855af655b09b..317c02a94da5 100644
--- a/kernel/liveupdate/luo_flb.c
+++ b/kernel/liveupdate/luo_flb.c
@@ -89,13 +89,18 @@ struct luo_flb_link {
static struct luo_flb_private *luo_flb_get_private(struct liveupdate_flb *flb)
{
struct luo_flb_private *private = &ACCESS_PRIVATE(flb, private);
+ static DEFINE_SPINLOCK(luo_flb_init_lock);
+ if (smp_load_acquire(&private->initialized))
+ return private;
+
+ guard(spinlock)(&luo_flb_init_lock);
if (!private->initialized) {
mutex_init(&private->incoming.lock);
mutex_init(&private->outgoing.lock);
INIT_LIST_HEAD(&private->list);
private->users = 0;
- private->initialized = true;
+ smp_store_release(&private->initialized, true);
}
return private;
--
2.43.0On Fri, Mar 27 2026, Pasha Tatashin wrote:
> The luo_flb_get_private() function, which is responsible for lazily
> initializing the private state of FLB objects, can be called
> concurrently from multiple threads. This creates a data
> race on the 'initialized' flag and can lead to multiple executions of
> mutex_init() and INIT_LIST_HEAD() on the same memory.
>
> Introduce a static spinlock (luo_flb_init_lock) local to the function
> to synchronize the initialization path. Use smp_load_acquire() and
> smp_store_release() for memory ordering between the fast path and the
> slow path.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
But... wouldn't it be a whole lot simpler if we introduce a
DEFINE_LUO_FLB() and get rid of luo_flb_get_private() entirely:
#define DEFINE_LUO_FLB(_name, _ops, _compatible) \
struct liveupdate_flb _name = { \
.ops = _ops, \
.compatible = _compatible, \
.private = { \
.list = LIST_HEAD_INIT(_name.private.list), \
.list = LIST_HEAD_INIT(_name.private.list), \
/ ...
}, \
}
I can't get sparse to work so not sure if I need some special syntax to
initialize stuff in .private, but I reckon we can get something working.
[...]
--
Regards,
Pratyush Yadav
On Tue, Mar 31, 2026 at 6:38 AM Pratyush Yadav <pratyush@kernel.org> wrote:
>
> On Fri, Mar 27 2026, Pasha Tatashin wrote:
>
> > The luo_flb_get_private() function, which is responsible for lazily
> > initializing the private state of FLB objects, can be called
> > concurrently from multiple threads. This creates a data
> > race on the 'initialized' flag and can lead to multiple executions of
> > mutex_init() and INIT_LIST_HEAD() on the same memory.
> >
> > Introduce a static spinlock (luo_flb_init_lock) local to the function
> > to synchronize the initialization path. Use smp_load_acquire() and
> > smp_store_release() for memory ordering between the fast path and the
> > slow path.
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
>
> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
Thank you.
>
> But... wouldn't it be a whole lot simpler if we introduce a
> DEFINE_LUO_FLB() and get rid of luo_flb_get_private() entirely:
We are planning some updates to this code in the near future:
1. David Matlack will send a patch in the next version of VFIO LUO
preservation series to add liveupdate_flb_put_incoming() so that
caller can protect from race with release of the last FD release that
will also release FLB.
This change will increment FLB 'count' not only when FH uses it, but
also when the object is being accessed.
2. After I plan to convert 'count' to use kref, and also decouble init
and liveupdate_flb_get_incoming(), this will allow to access FLB
object with interrupts disabled, as requested by Sami for the IOMMU
work. I think, that while working on this 2nd change, we can also do
some clean-ups if necessary.
Pasha
>
> #define DEFINE_LUO_FLB(_name, _ops, _compatible) \
> struct liveupdate_flb _name = { \
> .ops = _ops, \
> .compatible = _compatible, \
> .private = { \
> .list = LIST_HEAD_INIT(_name.private.list), \
> .list = LIST_HEAD_INIT(_name.private.list), \
> / ...
> }, \
> }
>
> I can't get sparse to work so not sure if I need some special syntax to
> initialize stuff in .private, but I reckon we can get something working.
>
> [...]
>
> --
> Regards,
> Pratyush Yadav
>
On Tue, Mar 31 2026, Pasha Tatashin wrote:
> On Tue, Mar 31, 2026 at 6:38 AM Pratyush Yadav <pratyush@kernel.org> wrote:
>>
>> On Fri, Mar 27 2026, Pasha Tatashin wrote:
>>
>> > The luo_flb_get_private() function, which is responsible for lazily
>> > initializing the private state of FLB objects, can be called
>> > concurrently from multiple threads. This creates a data
>> > race on the 'initialized' flag and can lead to multiple executions of
>> > mutex_init() and INIT_LIST_HEAD() on the same memory.
>> >
>> > Introduce a static spinlock (luo_flb_init_lock) local to the function
>> > to synchronize the initialization path. Use smp_load_acquire() and
>> > smp_store_release() for memory ordering between the fast path and the
>> > slow path.
>> >
>> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
>>
>> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
>
> Thank you.
>
>>
>> But... wouldn't it be a whole lot simpler if we introduce a
>> DEFINE_LUO_FLB() and get rid of luo_flb_get_private() entirely:
>
> We are planning some updates to this code in the near future:
>
> 1. David Matlack will send a patch in the next version of VFIO LUO
> preservation series to add liveupdate_flb_put_incoming() so that
> caller can protect from race with release of the last FD release that
> will also release FLB.
> This change will increment FLB 'count' not only when FH uses it, but
> also when the object is being accessed.
>
> 2. After I plan to convert 'count' to use kref, and also decouble init
> and liveupdate_flb_get_incoming(), this will allow to access FLB
> object with interrupts disabled, as requested by Sami for the IOMMU
> work. I think, that while working on this 2nd change, we can also do
> some clean-ups if necessary.
Okay, makes sense. I really think this initialization of private state
on first use is ugly and it should be gotten rid of. So please, whenever
you plan to do this refactor, include something like the static
initialization I mentioned.
>
> Pasha
>
>>
>> #define DEFINE_LUO_FLB(_name, _ops, _compatible) \
>> struct liveupdate_flb _name = { \
>> .ops = _ops, \
>> .compatible = _compatible, \
>> .private = { \
>> .list = LIST_HEAD_INIT(_name.private.list), \
>> .list = LIST_HEAD_INIT(_name.private.list), \
>> / ...
>> }, \
>> }
>>
>> I can't get sparse to work so not sure if I need some special syntax to
>> initialize stuff in .private, but I reckon we can get something working.
>>
--
Regards,
Pratyush Yadav
On Tue, Mar 31, 2026 at 3:22 PM Pratyush Yadav <pratyush@kernel.org> wrote:
>
> On Tue, Mar 31 2026, Pasha Tatashin wrote:
>
> > On Tue, Mar 31, 2026 at 6:38 AM Pratyush Yadav <pratyush@kernel.org> wrote:
> >>
> >> On Fri, Mar 27 2026, Pasha Tatashin wrote:
> >>
> >> > The luo_flb_get_private() function, which is responsible for lazily
> >> > initializing the private state of FLB objects, can be called
> >> > concurrently from multiple threads. This creates a data
> >> > race on the 'initialized' flag and can lead to multiple executions of
> >> > mutex_init() and INIT_LIST_HEAD() on the same memory.
> >> >
> >> > Introduce a static spinlock (luo_flb_init_lock) local to the function
> >> > to synchronize the initialization path. Use smp_load_acquire() and
> >> > smp_store_release() for memory ordering between the fast path and the
> >> > slow path.
> >> >
> >> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> >>
> >> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
> >
> > Thank you.
> >
> >>
> >> But... wouldn't it be a whole lot simpler if we introduce a
> >> DEFINE_LUO_FLB() and get rid of luo_flb_get_private() entirely:
> >
> > We are planning some updates to this code in the near future:
> >
> > 1. David Matlack will send a patch in the next version of VFIO LUO
> > preservation series to add liveupdate_flb_put_incoming() so that
> > caller can protect from race with release of the last FD release that
> > will also release FLB.
> > This change will increment FLB 'count' not only when FH uses it, but
> > also when the object is being accessed.
> >
> > 2. After I plan to convert 'count' to use kref, and also decouble init
> > and liveupdate_flb_get_incoming(), this will allow to access FLB
> > object with interrupts disabled, as requested by Sami for the IOMMU
> > work. I think, that while working on this 2nd change, we can also do
> > some clean-ups if necessary.
>
> Okay, makes sense. I really think this initialization of private state
> on first use is ugly and it should be gotten rid of. So please, whenever
> you plan to do this refactor, include something like the static
> initialization I mentioned.
I am not sure if it is going to be possible without giving an access
to "private fields" to clients that declare FLBs, those sould really
be accessed internally by FLB itself. If that is doable than sure,
otherwise I prefer to keeping the logical separate that is enforced by
"__private"
Pasha
>
> >
> > Pasha
> >
> >>
> >> #define DEFINE_LUO_FLB(_name, _ops, _compatible) \
> >> struct liveupdate_flb _name = { \
> >> .ops = _ops, \
> >> .compatible = _compatible, \
> >> .private = { \
> >> .list = LIST_HEAD_INIT(_name.private.list), \
> >> .list = LIST_HEAD_INIT(_name.private.list), \
> >> / ...
> >> }, \
> >> }
> >>
> >> I can't get sparse to work so not sure if I need some special syntax to
> >> initialize stuff in .private, but I reckon we can get something working.
> >>
>
> --
> Regards,
> Pratyush Yadav
© 2016 - 2026 Red Hat, Inc.