linux-kernel - Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on lock structures

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <e27b6484-8fb9-4c7f-9c8f-4d583cb64781@linux.dev>
Date: Sun, 24 Aug 2025 13:02:32 +0800
From: Lance Yang <lance.yang@...ux.dev>
To: Finn Thain <fthain@...ux-m68k.org>
Cc: akpm@...ux-foundation.org, mhiramat@...nel.org,
 kernel test robot <lkp@...el.com>, geert@...ux-m68k.org,
 senozhatsky@...omium.org, oe-kbuild-all@...ts.linux.dev,
 amaindex@...look.com, anna.schumaker@...cle.com, boqun.feng@...il.com,
 ioworker0@...il.com, joel.granados@...nel.org, jstultz@...gle.com,
 kent.overstreet@...ux.dev, leonylgao@...cent.com,
 linux-kernel@...r.kernel.org, linux-m68k@...ts.linux-m68k.org,
 longman@...hat.com, mingo@...hat.com, mingzhe.yang@...com,
 oak@...sinkinet.fi, rostedt@...dmis.org, tfiga@...omium.org,
 will@...nel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/1] hung_task: fix warnings by enforcing alignment on
 lock structures



On 2025/8/24 12:18, Finn Thain wrote:
> 
> On Sun, 24 Aug 2025, Lance Yang wrote:
> 
>> On 2025/8/24 08:47, Finn Thain wrote:
>>>
>>> On Sun, 24 Aug 2025, kernel test robot wrote:
>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>>      In file included from sound/soc/codecs/mt6660.c:15:
>>>>>> sound/soc/codecs/mt6660.h:28:1: warning: alignment 1 of 'struct
>>>>>> mt6660_chip' is less than 8 [-Wpacked-not-aligned]
>>>>         28 | };
>>>>            | ^
>>>>>> sound/soc/codecs/mt6660.h:25:22: warning: 'io_lock' offset 49 in 'struct
>>>>>> mt6660_chip' isn't aligned to 8 [-Wpacked-not-aligned]
>>>>         25 |         struct mutex io_lock;
>>>>            |                      ^~~~~~~
>>>>
>>>
>>> Misalignment warnings like this one won't work if you just pick an
>>> alignment arbitrarily i.e. to suit whatever bitfield you happen to need.
>>
>> Yes.
>>
>> The build warnings reported by the test robot are exactly the kind of
>> unintended side effect I was concerned about. It confirms that forcing
>> alignment on a core structure like struct mutex breaks other parts of
>> the kernel that rely on packed structures ;)
>>
> 
> Sure, your patch broke the build. So why not write a better patch? You
> don't need to align the struct, you need to align the lock, like I said
> already.

I think there might be a misunderstanding about the level of abstraction
at which the blocker tracking operates.

The blocker tracking mechanism operates on pointers to higher-level
locks (like struct mutex), as that is what is stored in the
task_struct->blocker field. It does not operate on the lower-level
arch_spinlock_t inside it.

While we could track the internal arch_spinlock_t, that would break
encapsulation. The hung task detector should remain generic and not
depend on lock-specific implementation details ;)

> 
>>>
>>> Instead, I think I would naturally align the actual locks, that is,
>>> arch_spinlock_t and arch_rwlock_t in include/linux/spinlock_types*.h.
>>
>> That's an interesting point. The blocker tracking mechanism currently
>> operates on higher-level structures like struct mutex. Moving the type
>> encoding down to the lowest-level locks would be a more complex and
>> invasive change, likely beyond the scope of fixing this particular
>> issue.
>>
> 
> I don't see why changing kernel struct layouts on m68k is particularly
> invasive. Perhaps I'm missing something (?)
> 
>> Looking further ahead, a better long-term solution might be to stop
>> repurposing pointer bits altogether. We could add an explicit
>> blocker_type field to task_struct to be used alongside the blocker
>> field. That would be a much cleaner design. TODO +1 for that idea :)
>>
>> So, let's drop the patch[1] that enforces alignment and go back to my
>> initial proposal[2], which adjusts the runtime checks to gracefully
>> handle unaligned pointers. That one is self-contained, has minimal
>> impact, and is clearly the safer solution for now.
>>
>> [1] https://lore.kernel.org/lkml/20250823074048.92498-1-lance.yang@linux.dev
>> [2] https://lore.kernel.org/lkml/20250823050036.7748-1-lance.yang@linux.dev
>>
> 
> I am willing to send a patch if it serves correctness and portability. So
> you may wish to refrain from crippling your blocker tracking algorithm for
> now.


Completely agreed that correctness and portability are the goals.

Please, feel free to send a patch.