Mutex error in mcFindDbMainFileName

[rogerbinns]

With apsw-sqlite3mc create the following file and run it with a sanitizer python:

import apsw

class TVFS(apsw.VFS):
    vfsname = "test"
    def __init__(self):
        super().__init__(self.vfsname, "")

    def xOpen(self, name, flags):
        return TVFSFile(name, flags)

class TVFSFile(apsw.VFSFile):
    def __init__(self, filename, flags):
        assert isinstance(filename, apsw.URIFilename)
        print(f"{filename.filename()=} {filename.parameters=}")
        super().__init__("", filename, flags)


tvfs = TVFS()
con = apsw.Connection("testdb", vfs=tvfs.vfsname)
con.pragma("key", "hellow world")

The code creates a new VFS named "test" then opens a not existing database testdb. The test VFS in its xOpen implementation creates a new VFSFile which simply opens the database using the default VFS.

When the pragma runs to set the encryption key the following sanitizer output is generated:

$ rm -f testdb*; python3 issue.py 
filename.filename()='/space/mc/testdb' filename.parameters=()
/space/mc/sqlite3/sqlite3.c:29743:3: runtime error: member access within misaligned address 0xfdfdfdfdfdfdfdfd for type 'struct sqlite3_mutex', which requires 8 byte alignment
0xfdfdfdfdfdfdfdfd: note: pointer points here
<memory cannot be printed>
AddressSanitizer:DEADLYSIGNAL
=================================================================
==3686912==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x723f5ec9ffe4 bp 0x7fff3079d220 sp 0x7fff3079d208 T0)
==3686912==The signal is caused by a READ memory access.
==3686912==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
    #0 0x723f5ec9ffe4 in ___pthread_mutex_lock nptl/pthread_mutex_lock.c:80
    #1 0x723f4c82d735 in pthreadMutexEnter /space/mc/sqlite3/sqlite3.c:29743
    #2 0x723f4c6deb9e in sqlite3_mutex_enter /space/mc/sqlite3/sqlite3.c:29198
    #3 0x723f4c790128 in mcFindDbMainFileName /space/mc/sqlite3/sqlite3.c:306761
    #4 0x723f4ca714ea in sqlite3mcSetCodec /space/mc/sqlite3/sqlite3.c:306857
    #5 0x723f4cc0951b in sqlite3mcCodecAttach /space/mc/sqlite3/sqlite3.c:283998
    #6 0x723f4cb5bf92 in sqlite3_key_v2 /space/mc/sqlite3/sqlite3.c:284069
    #7 0x723f4cb5c9d1 in sqlite3mcFileControlPragma /space/mc/sqlite3/sqlite3.c:283110
    #8 0x723f4cb5e328 in sqlite3Pragma /space/mc/sqlite3/sqlite3.c:139350
    #9 0x723f4cb8425c in yy_reduce /space/mc/sqlite3/sqlite3.c:177383
    #10 0x723f4cb8b1a1 in sqlite3Parser /space/mc/sqlite3/sqlite3.c:178014
    #11 0x723f4cb8c319 in sqlite3RunParser /space/mc/sqlite3/sqlite3.c:179348
    #12 0x723f4cb91163 in sqlite3Prepare /space/mc/sqlite3/sqlite3.c:142742
    #13 0x723f4cb92a64 in sqlite3LockAndPrepare /space/mc/sqlite3/sqlite3.c:142817
    #14 0x723f4cc2a4c5 in sqlite3_prepare_v3 /space/mc/sqlite3/sqlite3.c:142925
    #15 0x723f4cc44f9f in statementcache_prepare_internal src/statementcache.c:243
    #16 0x723f4cc4666b in statementcache_prepare src/statementcache.c:344
    #17 0x723f4cc4becf in APSWCursor_execute src/cursor.c:956
    #18 0x6167374e071e in cfunction_vectorcall_FASTCALL_KEYWORDS Objects/methodobject.c:438
    #19 0x6167373c143d in _PyObject_VectorcallTstate Include/internal/pycore_call.h:92
    #20 0x6167373c1586 in PyObject_Vectorcall Objects/call.c:325
    #21 0x723f4c92a28f in Connection_execute src/connection.c:4337
    #22 0x723f4c92b244 in Connection_pragma src/connection.c:4434
    #23 0x6167373f40f1 in method_vectorcall_FASTCALL_KEYWORDS Objects/descrobject.c:427
    #24 0x6167373c143d in _PyObject_VectorcallTstate Include/internal/pycore_call.h:92
    #25 0x6167373c1586 in PyObject_Vectorcall Objects/call.c:325
    #26 0x61673776da8c in _PyEval_EvalFrameDefault Python/bytecodes.c:2706
    #27 0x616737784160 in _PyEval_EvalFrame Include/internal/pycore_ceval.h:89
    #28 0x616737784412 in _PyEval_Vector Python/ceval.c:1683
    #29 0x616737784676 in PyEval_EvalCode Python/ceval.c:578
    #30 0x6167378dde85 in run_eval_code_obj Python/pythonrun.c:1722
    #31 0x6167378de09c in run_mod Python/pythonrun.c:1743
    #32 0x6167378e152d in pyrun_file Python/pythonrun.c:1643
    #33 0x6167378e5eae in _PyRun_SimpleFileObject Python/pythonrun.c:433
    #34 0x6167378e6193 in _PyRun_AnyFileObject Python/pythonrun.c:78
    #35 0x616737968721 in pymain_run_file_obj Modules/main.c:360
    #36 0x6167379689fb in pymain_run_file Modules/main.c:379
    #37 0x61673796b59e in pymain_run_python Modules/main.c:629
    #38 0x61673796b7a6 in Py_RunMain Modules/main.c:709
    #39 0x61673796b9bb in pymain_main Modules/main.c:739
    #40 0x61673796bd40 in Py_BytesMain Modules/main.c:763
    #41 0x616737116aa5 in main Programs/python.c:15
    #42 0x723f5ec2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #43 0x723f5ec2a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #44 0x6167371169d4 in _start (/space/pydebug/bin/python3.12+0x10859d4) (BuildId: 2e0f261b4b9f3e971f6e374ca89e9e487c24f8ac)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV nptl/pthread_mutex_lock.c:80 in ___pthread_mutex_lock
==3686912==ABORTING

The same issue occurs with the same call stack in other places in apsw-sqlite3mc tests when setting encryption keys on every opened database via a connection hook.

[utelle]

I suspect uninitialized memory in the sqlite3_vfs structure, but at the moment I have no clue why and where this could happen.

The default VFS (which will be something like multipleciphers-unix-none) is initialized correctly. Therefore I currently see 2 possible reasons:

1. A flaw on inheriting the default VFS
2. A flaw in the multiple-ciphers VFS shim, triggered by the fact that it is no longer the top VFS in the inheritance chain

The latter is more likely than the former.

[rogerbinns]

The code doesn't change the VFS order, although there are other tests that do. The C code of what is going on is:

const char *vfs = NULL;

sqlite3_vfs *vfstouse = sqlite3_vfs_find(vfs);
sqlite3_file *file = calloc(1, vfstouse->szOsFile);
int xopenresult = vfstouse->xOpen(vfstouse, self->filename, file, (int)flagsin, &flagsout);

But crucially vfstouse is not the same VFS as received the first xOpen call. ie a C/Python VFS in APSW received an xOpen request. It executes the code above and saves the result. Then it returns its own sqlite3_file object. When that latter object gets any method calls they end up passing them on to the methods from the first sqlite3_file.

[utelle]

Commit ec04fad fixes the problem.

Although the internal method to find the Multiple Ciphers VFS in the VFS stack worked correctly, the wrong VFS pointer was passed onwards. Now the correct pointer is used and all seems to work as expected.

However, AFAICT handling of the control code SQLITE_FCNTL_VFSNAME of the function xFileControl seems not to be implemented in APSW. At least there is no trace of the test VFS in the VFS stack of the database file. Here it works nevertheless, but it may or may not work, if the APSW VFS is not the top level VFS.

[rogerbinns]

SQLITE_FCNTL_VFSNAME is indeed not implemented in APSW. APSW doesn't implement any of VFS, only plumbing.

When your create a VFS you can specify the structure fields including name, make default, max pathname, iVersion, and which pointers you want left as NULL. You can also specify a base to "inherit" from.

For each callback it will the corresponding method name that the developer has to implement. Exceptions/results from that method are then returned back to SQLite. If you specified a base, then that VFS's method can be called explicitly via super(), or implicitly by not implementing the method yourself.

It is really analogous to implementing the structure methods yourself in Python instead of C, and no more than that.

SQLITE_FCNTL_VFSNAME isn't documented as working, and is for diagnostic purposes only. If you really care about detecting your VFS I'd recommend using a completely different opcode (ie not one used by SQLite). Be aware that any APSW VFS inheriting will pass the file control along without doing anything itself.

I'll try the main APSW test suite again using the apply_encryption from the readme to see what happens. It will result in all sorts of VFS stuff.

[utelle]

SQLITE_FCNTL_VFSNAME is indeed not implemented in APSW. APSW doesn't implement any of VFS, only plumbing.

Yes, that is clear. However, a VFS should typically return its own name to the caller, if it is a stand-alone VFS. If it is VFS shim it should implement this method like the demo VFS shims coming with SQLite. The VFS name is the combination of the name coming from the underlying VFS prepended by the name of the VFS shim. Unfortunately, this is not really documented, except the implementation code in the sample VFSes.

When your create a VFS you can specify the structure fields including name, make default, max pathname, iVersion, and which pointers you want left as NULL. You can also specify a base to "inherit" from.

The sample given in the first issue post inherits the default VFS (which could be for example multipleciphers-unix on a Linux machine). If you ask the SQLite shell with the dot command .vfsname, it will return - for example on Windows - multipleciphers-win32/win32 where multipleciphers-win32 is the VFS shim and win32 the real VFS actually doing the work of reading and writing from/to disk. But there is no trace of the top level VFS tvfs.

While a list of all registered VFSes can be produced, because the VFSes are chained in a linked list, there is no comparable mechanism available for accessing the VFS stack (where you have 1 or more VFS shims).

In our use case it is not necessary to know that the VFS test even exists. It is enough to be able to access the multipleciphers VFS shim. And this is possible, because the method SQLITE_FCNTL_VFSNAME is implemented.

SQLITE_FCNTL_VFSNAME isn't documented as working, and is for diagnostic purposes only. If you really care about detecting your VFS I'd recommend using a completely different opcode (ie not one used by SQLite). Be aware that any APSW VFS inheriting will pass the file control along without doing anything itself.

For the multipleciphers VFS shim it is enough, if the VFS file control function of the underlying VFS is called. This allows to retrieve the name of the current VFS name and to detect whether a multipleciphers shim is on the VFS stack.

I'll try the main APSW test suite again using the apply_encryption from the readme to see what happens. It will result in all sorts of VFS stuff.

AFAICT the VFS stuff should simply work - as long as the VFS doesn't plays tricks on the number of reserved bytes per page.

[rogerbinns]

However, a VFS should typically return its own name to the caller

This is semantically difficult in APSW. There is a distinction between a VFS and a VFSFile. There is absolutely no requirement in the code that a VFSFile correspond to a VFS. It is possible to have VFSFile that doesn't have a corresponding VFS. Both can shim to different VFS too.

I've made an issue to work on it.

SQLite shell with the dot command .vfsname

BTW APSW has a shell too. Just do python3 -m apsw dbname.

I also implemented that command, but it just prints the zName field of struct sqlite3_vfs. Mmmm - I can't really do any better without C helper code. Will think on it.

Do try the .vfslist command under apsw.

I still think that relying on names to a barely documented fcntl is fragile. The good news is that code that "inherits" including example code all default to passing file controls to their base.

[utelle]

However, a VFS should typically return its own name to the caller

This is semantically difficult in APSW. There is a distinction between a VFS and a VFSFile. There is absolutely no requirement in the code that a VFSFile correspond to a VFS. It is possible to have VFSFile that doesn't have a corresponding VFS. Both can shim to different VFS too.

AFAIK a VFSFile is created through a call to the xOpen method of a VFS. That is, a VFSFile should always have an associated VFS. If it doesn't or if the associated VFS is not one which is linked to the SQLite3MC VFS shim, the VFSFile will not be encrypted - at least not by SQLite3MC.

When method xFileControl of a VFSFile is called and the VFSFile is part of a VFS shim, the call will be forwarded to the underlying implementation. Thus the op code SQLITE_FCNTL_VFSNAME will return the name of the associated VFS.

If the SQLite3MC VFS is involved, then the returned name string will contain the string multipleciphers- followed by the name of the real underlying VFS. The prefix string is searched and the full name including the name of the underlying VFS will be determined. Then function sqlite3_vfs_find is used to get a pointer to the VFS, and finally this pointer is verified to point to a SQLite3MC VFS implementation.

Unfortunately, this is the only way to determine the pointer to a VFS shim within a VFS stack. There is no SQLite API to walk through the VFS stack.

I also implemented that command, but it just prints the zName field of struct sqlite3_vfs.

In the SQLite shell you have 3 VFS related dot commands:

1. .vfsinfo - shows information about the top-level VFS, including its name
2. .vfslist - shows a list of all registered VFSes
3. .vfsname - shows the name of the VFS stack

.vfsname forwards internally a call to xFileControl of the underlying implementation. All sample VFS shim implementations in the SQLite distribution prepend their own VFS name to the name string returned by xFileControl, separated by a slash (/).

Your APSW implementation should do the same, if possible.

Do try the .vfslist command under apsw.

This command displays a list of all registered VFSes. Unfortunately, there can be more than one registered VFS using the SQLite3MC VFS shim. This command doesn't help to identify which one is used for the current VFSFile.

I still think that relying on names to a barely documented fcntl is fragile. The good news is that code that "inherits" including example code all default to passing file controls to their base.

My code uses the SQLite API function sqlite3_file_control with the op code SQLITE_FCNTL_VFSNAME to determine the name of the VFS stack. As long as your implementation forwards the call to the underlying implementation, all is well.

Your implementation should prepend its own VFS name (here test) to the string returned by xFileControl (resulting in something like test/multipleciphers-unix/unix) to make clear it is involved. But actually it doesn't matter for my use case.

[utelle]

BTW, on initialization SQLite3MC registers a VFS using the SQLite3MC VFS shim and the normal default VFS, and makes it the new default VFS. Sometimes this is not what a user wants. Sometimes a user wants to use a non-default VFS together with encryption.

There are 2 possible ways to achieve this:

1. Explicitly create and register the required VFS combination using a SQLite3MC API function.
2. Implicitly, by simply prepending the string multipleciphers- to the name of the required underlying VFS on opening a database file. Specifying this combined VFS name will then implicitly create the VFS, if it isn't registered yet.

[rogerbinns]

At the C level a VFS consists of a bundle of two data structures - sqlite3_vfs and sqlite3_io_methods. The former implements xOpen while the latter implements xRead.

Conventional VFS implement both in the same source file. APSW makes absolutely no requirement that there is any relation between them. The problem is that SQLITE_FCNTL_VFSNAME is asking a sqlite3_io_methods for a name belonging to a sqlite3_vfs. So what does it want to know - who is implementing xOpen or who is implementing xRead?

This is why I think you should not depend on SQLITE_FCNTL_VFSNAME and really should use a separate fcntl with your own number to locate your VFS. That way you can get the exact semantics you want, and not depend on cosmetic strings.

I will implemented SQLITE_FCNTL_VFSNAME in APSW but only for the diagnostic purposes. That will also include updating the APSW shell .vfsname command.

The .vfsinfo and .vfslist are already implemented and provide a superset of information that the SQLite shell does.

It is entirely reasonable that a VFS wanting to use encrypted storage has to pass on all calls to sqlite3_io_methods to another VFS. It is unreasonable to expect that it can futz with stuff in the same way encryption does,

[utelle]

At the C level a VFS consists of a bundle of two data structures - sqlite3_vfs and sqlite3_io_methods. The former implements xOpen while the latter implements xRead.

Conventional VFS implement both in the same source file. APSW makes absolutely no requirement that there is any relation between them.

Yes. And that imposes a problem for SQLite3MC, when a codec needs to be located, but only the connection is known. A connection has an associated VFS, but the SQLite3MC VFS is not necessarily the top-level one. To find out whether the SQLite3MC VFS is member of the VFS stack or not, xFileControl with op code SQLITE_FCNTL_VFSNAME is used. If it is a member, then its name will be part of the result string. And that is actually enough. It doesn't matter whether other VFSes on the stack add their names to the result string or not.

The problem is that SQLITE_FCNTL_VFSNAME is asking a sqlite3_io_methods for a name belonging to a sqlite3_vfs. So what does it want to know - who is implementing xOpen or who is implementing xRead?

I consulted once again the source code of the sample VFSes coming with SQLite. Actually, most of them do not use the real VFS name for generating the VFS name. Usually they use only something similar. That is, typically it is not possible to identify the associated VFS from this string.

However, the SQLite3MC implementation returns the real VFS name.

This is why I think you should not depend on SQLITE_FCNTL_VFSNAME and really should use a separate fcntl with your own number to locate your VFS. That way you can get the exact semantics you want, and not depend on cosmetic strings.

I don't want to invent my own op code for this purpose, because there is always a chance it could collide with another op code. The op code name SQLITE_FCNTL_VFSNAME implies the right thing. Therefore it seemed logical to use it.

In the end it doesn't matter what other implementations of xFileControl do, as long as they do not remove the string coming from SQLite3MC's implementation from the result string.

I know this is somewhat fragile, because a malicious implementation could fake the result string. Therefore the result is carefully checked. (If I can come up with a better solution in the future, I will change my implementation.)

I will implemented SQLITE_FCNTL_VFSNAME in APSW but only for the diagnostic purposes. That will also include updating the APSW shell .vfsname command.

SQLite's documentation says that SQLITE_FCNTL_VFSNAME is for diagnostic purposes. And that's ok.

It is entirely reasonable that a VFS wanting to use encrypted storage has to pass on all calls to sqlite3_io_methods to another VFS. It is unreasonable to expect that it can fuzz with stuff in the same way encryption does.

You are right, of course.

One thing one has to keep in mind is that the SQLite3MC VFS shim will be incompatible with other VFSes or VFS shims which want to make use of reserved bytes on database pages themselves.

[rogerbinns]

I don't want to invent my own op code for this purpose, because there is always a chance it could collide with another op code.

There are 4 billion possible opcodes. If you are worried about a collision, I'll want winning lottery numbers from you!

Use the number below. Make it so that the associated void * starts out as NULL, and if your code sees the file control, it makes the pointer point to the data structure of your choosing. if the lottery is won, and someone uses the same number, and for example thinks the pointer is really a boolean, and they use this project, and they fill in a bad value they will get an immediate crash that can be diagnosed.

On the other hand depending on behaviour in a poorly documented arbitrarily manipulated string where others could use multicipher unintentionally seems far riskier, and a lot harder to diagnose problems with.

#define SQLITE_FCNTL_GET_MULTI_CIPHER  0x3f98c078

As to how I came up with that number:

$ python3 -c "import hashlib; print(hashlib.sha3_256(b'SQLITE_FCNTL_GET_MULTI_CIPHER').hexdigest()[-8:])"

Since you are still nervous about a clash, I offer you a lookup in the magic number database showing it hasn't been used for anything.

[utelle]

Citing from the SQLite VFS documentation:

Applications that define a custom xFileControl method should use opcodes greater than 100 to avoid conflicts.

So, you are right. that it is a common way to define one's own op codes.

Nevertheless I think that the op code SQLITE_FCNTL_VFSNAME expresses well what is wanted and would play well together with the VFS shim implementations coming with SQLite. For the time being I will stick to it.

[rogerbinns]

0x3f98c078 still has your name on it :-) I just looked at the sqlite3mc_vfs and I am surprised you prefer that over an unambiguous fcntl that gets you the pointer directly. The name approach does still have other problems = for example it assumes the vfs name is stable. But you can add a new vfs of an existing name at any time. Behind the scenes SQLite uses reference counting, so it won't get confused between the old and new data structures sharing the same name.

It looks like the the pragma wants to know which sqlite3_vfs has the xOpen method that would return the sqlite3_io_methods in use on this file.