linux-kernel - Re: [PATCH] can: m_can: initialize spin lock on device probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMZ6RqLXrdfS9sjaZaFcZtOyaP9Q8hHk8Wb+d7D1ovVEvK_OwA@mail.gmail.com>
Date: Fri, 25 Apr 2025 18:36:44 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: Antonios Salios <antonios@....re>
Cc: Marc Kleine-Budde <mkl@...gutronix.de>, rcsekar@...sung.com, linux-can@...r.kernel.org, 
	linux-kernel@...r.kernel.org, lukas@....re, jan@....re, 
	Markus Schneider-Pargmann <msp@...libre.com>
Subject: Re: [PATCH] can: m_can: initialize spin lock on device probe

On Fri. 25 Apr. 2025 à 18:18, Antonios Salios <antonios@....re> wrote:
> On Fri, 2025-04-25 at 16:18 +0900, Vincent Mailhol wrote:
> > I guess this is because your kernel has CONFIG_DEBUG_SPINLOCK:
>
> Indeed.
>
> > Without it, this would have been a more severe NULL pointer
> > dereference.
>
> Strangely, a NULL pointer dereference does not occur, when I try again
> with CONFIG_DEBUG_SPINLOCK disabled. The kernel does not crash, at
> least on rv64.
>
> Looking through the implementations of arch_spinlock_t, it seems that
> only PARISC's implementation would cause problems in this case since it
> uses an array.
>
> https://elixir.bootlin.com/linux/v6.15-rc3/source/arch/parisc/include/asm/spinlock_types.h#L11
>
> I think I'm missing something, why do you think a NULL pointer deref
> would occur in this case?

I see. Thanks for your test. I went a bit too quick in my analysis
when I saw things like:

  raw_spin_lock(&lock->rlock);

in

  https://elixir.bootlin.com/linux/v6.14/source/include/linux/spinlock.h#L351

I thought about the NULL pointer dereference. But indeed, you are
right. The spinlock_t is just one attribute of a structure and will be
allocated anyway even if spin_lock_init is not called, so calling

  spin_lock_irqsave(&cdev->tx_handling_spinlock, irqflags);

will still pass a valid address.

The other thing which put me off guard is that some other "spinlock
bad magic" got assigned some CVE.

https://lore.kernel.org/linux-cve-announce/2025031217-CVE-2025-21862-e8a0@gregkh/

But here as well, that does not imply a NULL pointer dereference. I
think that the bug is only that the spin_lock is not working as
intended.

Regardless, just saying that it is a spinlock bad magic bug with the
dmesg trace is enough. Thanks again for your tests!


Yours sincerely,
Vincent Mailhol