Bug report: Potential thread safety issues in sqlite3_initialize

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug report: Potential thread safety issues in sqlite3_initialize

Oystein Eftevaag
Hi folks,

Data races in sqlite3_initialize was previously reported in
https://www.mail-archive.com/sqlite-users@.../msg94225.html
and
a fix landed, however while investigating internal TSan reports of this, as
far as we can tell the issue is still present (on non-x86 platforms with
less memory ordering guarantees) as mentioned previously in
https://www.mail-archive.com/sqlite-users@.../msg94225.html
.

Specifically, in sqlite3MutexInit() sqlite3GlobalConfig.mutex.xMutexAlloc
can be read as being set on a core, while the rest of the initialization
done in sqlite3MutexInit() still is being read as unset. The same is true
for sqlite3GlobalConfig.isInit within sqlite3_initialize(); a core calling
the function concurrently with another, could see
sqlite3GlobalConfig.isInit as set while the rest of the init work done is
still seen as unset. This is due to the current memory barriers ordering
the writes, but there's no barriers ensuring that other cores haven't done
the loads out of order.

Thanks,
Oystein
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Bug report: Potential thread safety issues in sqlite3_initialize

Richard Hipp-3
On 1/28/20, Oystein Eftevaag <[hidden email]> wrote:
> in sqlite3MutexInit() sqlite3GlobalConfig.mutex.xMutexAlloc
> can be read as being set on a core, while the rest of the initialization
> done in sqlite3MutexInit() still is being read as unset.

Doesn't the memory barrier at
https://www.sqlite.org/src/artifact/bae36f8af32c22ad?ln=247 prevent
that?  Do you have a suggested patch to make it work?


--
D. Richard Hipp
[hidden email]
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Bug report: Potential thread safety issues in sqlite3_initialize

Oystein Eftevaag
As I understand it, the barrier in that patch ensures that for whichever
thread executes the if(!sqlite3GlobalConfig.mutex.xMutexAlloc codepath)
{...}, the write to pTo->xMutexAlloc will be stored after the rest of the
xMutex* field writes. But there's nothing preventing another thread
*loading* them out of order; e.g. loading an uninitialized
sqlite3GlobalConfig.mutex.xMutexInit into a register prior to loading a now
initialized sqlite3GlobalConfig.mutex.xMutexAlloc (hence skipping the if()
block  in sqlite3MutexInit()), which could effectively result in a
sqlite3MutexInit() call with sqlite3GlobalConfig.mutex.xMutexInit
still being null after (for some number of cycles).

https://gist.github.com/vinterstum/ff4bc1ea715cc1d4c5da45864c9de4af should
help I think.

On Tue, Jan 28, 2020 at 4:57 PM Richard Hipp <[hidden email]> wrote:

> On 1/28/20, Oystein Eftevaag <[hidden email]> wrote:
> > in sqlite3MutexInit() sqlite3GlobalConfig.mutex.xMutexAlloc
> > can be read as being set on a core, while the rest of the initialization
> > done in sqlite3MutexInit() still is being read as unset.
>
> Doesn't the memory barrier at
> https://www.sqlite.org/src/artifact/bae36f8af32c22ad?ln=247 prevent
> that?  Do you have a suggested patch to make it work?
>
>
> --
> D. Richard Hipp
> [hidden email]
>
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users