2023-10-05 17:22:05

by Tatu Heikkilä

[permalink] [raw]
Subject: (Bisected) Accessing opened Bitlocker partition leads to memory fault and kernel panic on Imac8,1

Hello,
I think you and the lists are right recipients, forgive me if not, I've
never reported kernel bugs before. Naively this seems a crypto issue and
Herbert Xu from crypto maintainers made the buggy commit, but it edits
drivers/md/dm_crypt.c maintained by dm-devel people per MAINTAINERS, so
I'm going by that.

At the center of the issue is my Imac8,1 and an external 2TB SSD with 5
partitions: an EFI+MBR portable Arch Linux install with LUKS encrypted
ext4 /home, and a 1.7TB exFAT encrypted with Bitlocker.

Mounting the LUKS partition works fine on all my 4 computers (Imac8,1,
Imac12,2, two generic Intels; Fedora's GNOME gvfs volume monitor often
crashes on mount using this drive), and mounting the Bitlocker partition
works on all other computers, but my Imac8,1. On my other computers, I
can boot into the portable install which automounts the Bitlocker
partition fine. However, on my Imac8,1, regardless if I boot into the
external drive's portable Arch Linux install, or use the Imac's own
internal Debian testing install, any post-6.4 kernel reliably panics
(50+ times so far, 100% of the time) when accessing the unlocked
Bitlocker volume:

# cryptsetup open /dev/sdb5 --type bitlk crucial
Enter passphrase for /dev/sdb5:
# mount /dev/mapper/crucial temp [kernel immediately panics if I try to
tab-complete the mount point, making the shell also access the decrypted
device I assume, or try to run the command]

I originally ran into this when mounting using XFCE's Thunar
implementation. Using it, the mount fails with "Operation was cancelled"
and the system crashes within a minute.

Git bisect lead me to:
# first bad commit: [e3023094dffb41540330fb0c74cd3a019cd525c2] dm crypt:
Avoid using MAX_CIPHER_BLOCKSIZE

If I git revert e3023094dffb41540330fb0c74cd3a019cd525c2 on current
Linus' git master, the issue goes away. So I'm personally not all that
affected anymore (if I'm ready to compile my kernels from now on), and I
understand that you have no clear way to reproduce this as it seems
strongly bound to hardware, but seems like this could point to a
potentially serious security issue since it involves both crypto and
undefined behaviour.

Kdump dmesg logs (the error output is not completely consistent between
panics) & .config can be found in a dummy Bugzilla report
https://bugzilla.kernel.org/show_bug.cgi?id=217982

Please let me know if I can help you in any way. I don't mind using this
as a gateway to learn more about kernel debugging etc.

Best regards,
Tatu Heikkilä


2023-10-06 01:04:44

by Bagas Sanjaya

[permalink] [raw]
Subject: Re: (Bisected) Accessing opened Bitlocker partition leads to memory fault and kernel panic on Imac8,1

On Thu, Oct 05, 2023 at 08:15:43PM +0300, Tatu Heikkilä wrote:
> Hello,
> I think you and the lists are right recipients, forgive me if not, I've
> never reported kernel bugs before. Naively this seems a crypto issue and
> Herbert Xu from crypto maintainers made the buggy commit, but it edits
> drivers/md/dm_crypt.c maintained by dm-devel people per MAINTAINERS, so I'm
> going by that.
>
> At the center of the issue is my Imac8,1 and an external 2TB SSD with 5
> partitions: an EFI+MBR portable Arch Linux install with LUKS encrypted ext4
> /home, and a 1.7TB exFAT encrypted with Bitlocker.
>
> Mounting the LUKS partition works fine on all my 4 computers (Imac8,1,
> Imac12,2, two generic Intels; Fedora's GNOME gvfs volume monitor often
> crashes on mount using this drive), and mounting the Bitlocker partition
> works on all other computers, but my Imac8,1. On my other computers, I can
> boot into the portable install which automounts the Bitlocker partition
> fine. However, on my Imac8,1, regardless if I boot into the external drive's
> portable Arch Linux install, or use the Imac's own internal Debian testing
> install, any post-6.4 kernel reliably panics (50+ times so far, 100% of the
> time) when accessing the unlocked Bitlocker volume:
>
> # cryptsetup open /dev/sdb5 --type bitlk crucial
> Enter passphrase for /dev/sdb5:
> # mount /dev/mapper/crucial temp [kernel immediately panics if I try to
> tab-complete the mount point, making the shell also access the decrypted
> device I assume, or try to run the command]
>
> I originally ran into this when mounting using XFCE's Thunar implementation.
> Using it, the mount fails with "Operation was cancelled" and the system
> crashes within a minute.
>
> Git bisect lead me to:
> # first bad commit: [e3023094dffb41540330fb0c74cd3a019cd525c2] dm crypt:
> Avoid using MAX_CIPHER_BLOCKSIZE
>
> If I git revert e3023094dffb41540330fb0c74cd3a019cd525c2 on current Linus'
> git master, the issue goes away. So I'm personally not all that affected
> anymore (if I'm ready to compile my kernels from now on), and I understand
> that you have no clear way to reproduce this as it seems strongly bound to
> hardware, but seems like this could point to a potentially serious security
> issue since it involves both crypto and undefined behaviour.
>
> Kdump dmesg logs (the error output is not completely consistent between
> panics) & .config can be found in a dummy Bugzilla report
> https://bugzilla.kernel.org/show_bug.cgi?id=217982
>
> Please let me know if I can help you in any way. I don't mind using this as
> a gateway to learn more about kernel debugging etc.
>

Thanks for the regression report. I'm adding it to regzbot:

#regzbot ^introduced: e3023094dffb41
#regzbot title: kernel panic when accessing opened bitlocker partition due to avoiding MAX_CIPHER_BLOCKSIZE
#regzbot link: https://bugzilla.kernel.org/show_bug.cgi?id=217982

--
An old man doll... just what I always wanted! - Clara


Attachments:
(No filename) (2.96 kB)
signature.asc (235.00 B)
Download all attachments

2023-10-06 01:42:12

by Herbert Xu

[permalink] [raw]
Subject: [PATCH] dm crypt: Fix reqsize in crypt_iv_eboiv_gen

On Fri, Oct 06, 2023 at 08:04:18AM +0700, Bagas Sanjaya wrote:
>
> > Git bisect lead me to:
> > # first bad commit: [e3023094dffb41540330fb0c74cd3a019cd525c2] dm crypt:
> > Avoid using MAX_CIPHER_BLOCKSIZE
> >
> > If I git revert e3023094dffb41540330fb0c74cd3a019cd525c2 on current Linus'
> > git master, the issue goes away. So I'm personally not all that affected
> > anymore (if I'm ready to compile my kernels from now on), and I understand
> > that you have no clear way to reproduce this as it seems strongly bound to
> > hardware, but seems like this could point to a potentially serious security
> > issue since it involves both crypto and undefined behaviour.

Thanks for the report. Sorry this is indeed my fault. The allocated
buffer is too small as it's missing the size for the request object
itself.

Mike, would you be OK with me picking this fix up and pushing it to
Linus?

Cheers,

---8<---
A skcipher_request object is made up of struct skcipher_request
followed by a variable-sized trailer. The allocation of the
skcipher_request and IV in crypt_iv_eboiv_gen is missing the
memory for struct skcipher_request. Fix it by adding it to
reqsize.

Fixes: e3023094dffb ("dm crypt: Avoid using MAX_CIPHER_BLOCKSIZE")
Reported-by: Tatu Heikkil� <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index f2662c21a6df..5315fd261c23 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -753,7 +753,8 @@ static int crypt_iv_eboiv_gen(struct crypt_config *cc, u8 *iv,
int err;
u8 *buf;

- reqsize = ALIGN(crypto_skcipher_reqsize(tfm), __alignof__(__le64));
+ reqsize = sizeof(*req) + crypto_skcipher_reqsize(tfm);
+ reqsize = ALIGN(reqsize, __alignof__(__le64));

req = kmalloc(reqsize + cc->iv_size, GFP_NOIO);
if (!req)
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-10-06 02:27:17

by Mike Snitzer

[permalink] [raw]
Subject: Re: dm crypt: Fix reqsize in crypt_iv_eboiv_gen

On Thu, Oct 05 2023 at 9:41P -0400,
Herbert Xu <[email protected]> wrote:

> On Fri, Oct 06, 2023 at 08:04:18AM +0700, Bagas Sanjaya wrote:
> >
> > > Git bisect lead me to:
> > > # first bad commit: [e3023094dffb41540330fb0c74cd3a019cd525c2] dm crypt:
> > > Avoid using MAX_CIPHER_BLOCKSIZE
> > >
> > > If I git revert e3023094dffb41540330fb0c74cd3a019cd525c2 on current Linus'
> > > git master, the issue goes away. So I'm personally not all that affected
> > > anymore (if I'm ready to compile my kernels from now on), and I understand
> > > that you have no clear way to reproduce this as it seems strongly bound to
> > > hardware, but seems like this could point to a potentially serious security
> > > issue since it involves both crypto and undefined behaviour.
>
> Thanks for the report. Sorry this is indeed my fault. The allocated
> buffer is too small as it's missing the size for the request object
> itself.
>
> Mike, would you be OK with me picking this fix up and pushing it to
> Linus?
>
> Cheers,
>
> ---8<---
> A skcipher_request object is made up of struct skcipher_request
> followed by a variable-sized trailer. The allocation of the
> skcipher_request and IV in crypt_iv_eboiv_gen is missing the
> memory for struct skcipher_request. Fix it by adding it to
> reqsize.
>
> Fixes: e3023094dffb ("dm crypt: Avoid using MAX_CIPHER_BLOCKSIZE")
> Reported-by: Tatu Heikkil� <[email protected]>
> Signed-off-by: Herbert Xu <[email protected]>
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index f2662c21a6df..5315fd261c23 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -753,7 +753,8 @@ static int crypt_iv_eboiv_gen(struct crypt_config *cc, u8 *iv,
> int err;
> u8 *buf;
>
> - reqsize = ALIGN(crypto_skcipher_reqsize(tfm), __alignof__(__le64));
> + reqsize = sizeof(*req) + crypto_skcipher_reqsize(tfm);
> + reqsize = ALIGN(reqsize, __alignof__(__le64));
>
> req = kmalloc(reqsize + cc->iv_size, GFP_NOIO);
> if (!req)

Sure, please do. Shouldn't your header Cc: stable given that the
Fixes commit implies v6.5 needs this fix?

(sorry I missed this the first time I 'Reviewed-by' the original
commit)

Reviewed-by: Mike Mike Snitzer <[email protected]>

Thanks,
Mike

2023-10-06 11:04:03

by Tatu Heikkilä

[permalink] [raw]
Subject: Re: [PATCH] dm crypt: Fix reqsize in crypt_iv_eboiv_gen

On Fri, Oct 06, 2023 at 09:41:55 +0800, Herbert Xu wrote:
>
> On Fri, Oct 06, 2023 at 08:04:18AM +0700, Bagas Sanjaya wrote:
> >
> > > Git bisect lead me to:
> > > # first bad commit: [e3023094dffb41540330fb0c74cd3a019cd525c2] dm crypt:
> > > Avoid using MAX_CIPHER_BLOCKSIZE
> > >
> > > If I git revert e3023094dffb41540330fb0c74cd3a019cd525c2 on current Linus'
> > > git master, the issue goes away. So I'm personally not all that affected
> > > anymore (if I'm ready to compile my kernels from now on), and I understand
> > > that you have no clear way to reproduce this as it seems strongly bound to
> > > hardware, but seems like this could point to a potentially serious security
> > > issue since it involves both crypto and undefined behaviour.
>
> Thanks for the report. Sorry this is indeed my fault. The allocated
> buffer is too small as it's missing the size for the request object
> itself.

Thank you for your prompt fix, I can access the volume without issue now. :-)
-Tatu

Subject: Re: (Bisected) Accessing opened Bitlocker partition leads to memory fault and kernel panic on Imac8,1

On 06.10.23 03:04, Bagas Sanjaya wrote:
> On Thu, Oct 05, 2023 at 08:15:43PM +0300, Tatu Heikkilä wrote:
>> Hello,
>> I think you and the lists are right recipients, forgive me if not, I've
>> never reported kernel bugs before. Naively this seems a crypto issue and
>> Herbert Xu from crypto maintainers made the buggy commit, but it edits
>> drivers/md/dm_crypt.c maintained by dm-devel people per MAINTAINERS, so I'm
>> going by that.
>> [...]

#regzbot fix: 152d0bcdf1efcb
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.