On Mon, Sep 27, 2021 at 6:22 AM Arnd Bergmann <arnd@xxxxxxxxxx> wrote: > > More specifically, ' think '\377' may be either -1 or 255 depending on > the architecture. > On most architectures, 'char' is implicitly signed, but on some others > it is not. Yeah. That code is just broken. And Arnd, your patch may be "conceptually minimal", in that it keeps thed broken code and makes it work. But it just dials up the oddity to 11. The proper patch is just this appended thing that stops playing silly games, and just uses "memcmp()". I've verified that with sane build configurations, it just generates testq %rsi, %rsi je .L25 cmpl $-33620224, (%rsi) je .L31 for that if (data && !memcmp(data, VBSF_MOUNT_SIGNATURE, 4)) { test. With a lot of crazy debug options you'll actually see the "memcmp()", but the bad code generation is the least of your options in that case. Linus
fs/vboxsf/super.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/fs/vboxsf/super.c b/fs/vboxsf/super.c index 4f5e59f06284..37dd3fe5b1e9 100644 --- a/fs/vboxsf/super.c +++ b/fs/vboxsf/super.c @@ -21,10 +21,7 @@ #define VBOXSF_SUPER_MAGIC 0x786f4256 /* 'VBox' little endian */ -#define VBSF_MOUNT_SIGNATURE_BYTE_0 ('\000') -#define VBSF_MOUNT_SIGNATURE_BYTE_1 ('\377') -#define VBSF_MOUNT_SIGNATURE_BYTE_2 ('\376') -#define VBSF_MOUNT_SIGNATURE_BYTE_3 ('\375') +static const unsigned char VBSF_MOUNT_SIGNATURE[4] = "\000\377\376\375"; static int follow_symlinks; module_param(follow_symlinks, int, 0444); @@ -386,12 +383,7 @@ static int vboxsf_setup(void) static int vboxsf_parse_monolithic(struct fs_context *fc, void *data) { - unsigned char *options = data; - - if (options && options[0] == VBSF_MOUNT_SIGNATURE_BYTE_0 && - options[1] == VBSF_MOUNT_SIGNATURE_BYTE_1 && - options[2] == VBSF_MOUNT_SIGNATURE_BYTE_2 && - options[3] == VBSF_MOUNT_SIGNATURE_BYTE_3) { + if (data && !memcmp(data, VBSF_MOUNT_SIGNATURE, 4)) { vbg_err("vboxsf: Old binary mount data not supported, remove obsolete mount.vboxsf and/or update your VBoxService.\n"); return -EINVAL; }