"bcachefs unlock -f /dev/null" quits with SIGSEGV, Segmentation fault

[marcin-github]

bcachefs unlock /dev/xxxx -f /dev/null throws core:

gdb:

(gdb) bt
#0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
#1  0x000055bcf47bcb75 in derive_passphrase (crypt=<optimized out>, passphrase=0x0) at c_src/crypto.c:87
#2  0x000055bcf47bccd8 in bch2_passphrase_check (sb=0x55bd14812000, passphrase=0x0, passphrase_key=0x7ffdb5df7170, sb_key=0x7ffdb5df7190) at c_src/crypto.c:125
#3  0x000055bcf47bce45 in bch2_add_key (sb=0x55bd14812000, type=type@entry=0x55bcf499ca77 "user", keyring_str=keyring_str@entry=0x55bcf499ca77 "user", passphrase=passphrase@entry=0x0) at c_src/crypto.c:154
#4  0x000055bcf47b8fef in cmd_unlock (argc=<optimized out>, argv=<optimized out>) at c_src/cmd_key.c:80
#5  0x000055bcf47a4600 in bcachefs::main ()
#6  0x000055bcf47a7dd3 in std::sys_common::backtrace::__rust_begin_short_backtrace ()
#7  0x000055bcf47a32d9 in std::rt::lang_start::{{closure}} ()
#8  0x000055bcf4955491 in core::ops::function::impls::{impl#2}::call_once<(), (dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe)> ()
    at library/core/src/ops/function.rs:284
#9  std::panicking::try::do_call<&(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe), i32> () at library/std/src/panicking.rs:554
#10 std::panicking::try<i32, &(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe)> () at library/std/src/panicking.rs:518
#11 std::panic::catch_unwind<&(dyn core::ops::function::Fn<(), Output=i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe), i32> () at library/std/src/panic.rs:142
#12 std::rt::lang_start_internal::{closure#2} () at library/std/src/rt.rs:148
#13 std::panicking::try::do_call<std::rt::lang_start_internal::{closure_env#2}, isize> () at library/std/src/panicking.rs:554
#14 std::panicking::try<isize, std::rt::lang_start_internal::{closure_env#2}> () at library/std/src/panicking.rs:518
#15 std::panic::catch_unwind<std::rt::lang_start_internal::{closure_env#2}, isize> () at library/std/src/panic.rs:142
#16 std::rt::lang_start_internal () at library/std/src/rt.rs:148
#17 0x000055bcf47a4c25 in main ()

[marcin-github]

The same happens if password file is empty (I have fs with empty password).

[matthiasgoergens]

For a full reproduction against current master:

head --bytes=1G < /dev/zero > loop
yes | bcachefs mkfs --encrypted loop
bcachefs unlock -f /dev/null loop

[matthiasgoergens]

The error originates in https://github.com/koverstreet/bcachefs-tools/blob/master/c_src/tools-util.c#L127

    if (!strlen(buf)) {
        free(buf);
        buf = NULL;
    }

After reading the passphrase from file, we return NULL if the passphrase was empty. Then later crypto_pwhash_scryptsalsa208sha256_ll dies when it's been fed a null pointer for its passwd argument.

Commit 7c39a1cf1bc499ff5c8776b455bb94dea4b8ca59 by Kent introduced the check for an empty passphrase, but I don't know why.

We have two ways forward:

Either: forbid empty passphrases, and fail with a proper error message. That would also entail teaching mkfs to reject empty passphrases.

Or: handle empty passphrases in unlock properly. Perhaps that's as easy as just deleting the four offending lines, but perhaps there are other consequences.