Zack's Kernel News

Zack's Kernel News

Article from Issue 264/2022
Author(s):

Zack discusses mysterious alignments in the kernel; and discovery and invention.

Mysterious Alignments in the Kernel

Sudip Mukherjee reported a build failure with the mainline kernel source tree. This is fairly unusual. Most patches get tested to see if they'll compile on a variety of different systems before the patch is accepted, but a build failure can happen.

After noticing the failure, Sudip did a git bisect – a fast way to jump half the distance to the problem, then half the remaining distance, and so on. It's one of the most common techniques for identifying which patch actually caused a bug. Sudip used it to track the problem down to a small patch that compared the number of bytes used by two different variables that needed to be the same.

Linus Torvalds is often interested in build failures or crashes in the mainline tree because he maintains that tree himself. Linus responded right away to Sudip. He said he had looked at the data size comparison, and it seemed like a good patch to him. He said, "That BUILD_BUG_ON() looks entirely correct, and if that sizeof() doesn't work, then the code doesn't work." In other words, the check was appropriate. Linus added, "I'm not seeing what could go wrong in there."

Linus was not seeing the build failure himself – he was simply examining the bug report. But he remarked, "it would seem to be something very peculiar to your build environment (either that config, or your compiler)."

This is not what someone wants to hear if they are hoping the developers on a given project will help track down their problem. Sudip replied that his build environment had not changed at all since his previous successful build. Not only that, but he encountered the exact same problem on his laptop – presumably a quite different environment.

Linus asked, "Hmm. What compiler do you have? Because it seems very broken."

Linus dug further into the code, looking at the machine language opcodes produced by the compiler, among other things. He speculated, "I suspect some of the structs inside of that 'struct edid' end up getting aligned, despite the '__attribute__((packed))'. For example, 'struct est_timings' is supposed to be just 3 bytes, and it's at an odd offset too (byte offset 35 in the 'struct edid' if I did the math correctly)."

But he reiterated, "it obviously doesn't happen for me or for most other people, so it's something in your setup. Unusual compiler?"

Sudip was certain it was not his personal setup. He replied, "I am using gcc version 11.3.1 20220517 (GCC). And I am not just building spear3xx_defconfig; I am building all the arm configs with the same compiler in the same setup and only spear3xx_defconfig started failing." In other words, Sudip was using a very normal environment using a standard compiler. He added that he was also compiling under a clean Debian virtual Docker instance – no special circumstances.

Linus continued to dig into the code, and he found that the variable sizing – the thing that seems to have caused the problem for Sudip in the first place – was actually very different from what it needed to be. He didn't know why the discrepancy was there, but he felt that he had put his finger on something that had to be near the solution.

He posted a patch that seemed to fix the problem – but he also said, "I have no idea what it is that triggers this only on spear3xx_defconfig. Some ARM ABI [Application Binary Interface] issue that is triggered by some very particular ARM compiler flag enabled by that config?"

Linus added, "This smells like a compiler bug triggered by 'there's a 16-bit member field in that gtf2 structure, and despite it being packed and aligned to 1, we somehow still align the size to 2'."

Arnd Bergmann thought Linus's patch looked correct. However, Russell King also came into the conversation at this point, with the following explanation:

"It's an age old thing, it's no compiler bug, and it's completely compliant with the C standards. Implementations are permitted by the C standard to pad structures and unions how they see fit – and some do if it makes sense for performance.

"The mistake is that people forget this detail, and they expect structs and unions to be laid out a certain way – because it doesn't matter to the same extent on x86.

"However, as older ARM CPUs could not do unaligned loads, ensuring that things were naturally aligned made complete sense, even if it meant that people who assume the world is x86 got tripped up – the only way around that would be to make every load very expensive.

"It's not 'align to size of 2' in OABI; it tends to be align to a multiple of 4, because the underlying architecture is 32-bit."

Jani Nikula, the author of the original patch that triggered Sudip's build failure, replied, "Indeed the bug is ancient. I just threw in the BUILD_BUG_ON() on a whim as an extra sanity check when doing pointer arithmetics on struct edid." And he added, "Makes you wonder about all the other packed structs with enum members across the kernel."

Arnd replied with a further explanation:

"It is not the 'enum' that is special here; it's the 'union' having unpacked members, and the same thing happens when you have nested structs: both the inner and the outer aggregate need to be packed, either with __packed at the end, or on each individual member that is not fully aligned to max(sizeof(member), 4)).

"I think in general, most __packed annotations we have in the kernel are completely pointless because they do not change the structure layout on any architecture but instead just make member access slower on architectures that lack unaligned load/store instructions. There have definitely been other cases though where a __packed annotation is not needed on any sane architecture but is needed for OABI ARM.

"Overall I'm not that worried because the only machines running OABI kernels would be on really old hardware that runs a limited set of driver code.

"A completely different matter are the extraneous __packed annotations that lead to possible problems when accessed through a misaligned pointer. We ignore -Waddress-of-packed-member and -Wcast-align in the kernel, so these never get caught at build time, but we have seen bugs from gcc making incorrect assumptions about alignment even on architectures that have unaligned load/store instructions."

Further technical explanations and considerations ensued, until Russell remarked, "Sounds like we need a howto document for people to ignore and continue doing their own thing."

The conversation continued into deep dark recesses, with the main idea being to catch this sort of thing before it could become a problem next time. Following the conversation intently, Linus at one point remarked, "it would probably not be a bad idea to check that packed structures/unions don't have atomic types or locks in them. I _think_ we're all good, but who knows."

The discussion continued, and new people were brought into the CC list, as the ideas touched more and more areas of the kernel. There was no grand conclusion, except that now a lot more developers are aware of a lot more of the details of this particular data alignment issue.

Discovery and Invention

Linus Torvalds discovered open source. The GNU project may have written the General Public License (GPL), but it was never truly useful until Linus showed everyone how to use it. And this is one of the most valuable things to learn from Linux kernel development, because Linus is still discovering open source, and he is still showing the rest of us how to do it.

One interesting aspect of his approach has to do with when he'll lay down the law versus when he'll push hard versus when he'll simply nudge. Not too long ago, he laid down the law regarding patches that would cause build warnings. He wouldn't allow them in patch submissions, and he set the official kernel's build process to fail if warnings occurred.

At around the same time, Linus also started nudging developers to use cryptographic signatures (signed tags) when making pull requests. Signed tags would mean that Linus could confirm that a particular request came from a particular person; thus, he could trace specific pieces of kernel code back to their origins. Signed tags are useful for security, but they are also useful for legal reasons, in case anyone claims Linux uses code taken from a non-GPL-compliant source.

Arnd Bergmann, for example, was traveling one day and wasn't in a position to sign his pull request. He told Linus, "I'm travelling at the moment and forgot to tag it first. The top commit in the branch is tagged with my usual key, and I can send the signed tag when I'm back on Saturday." To which Linus replied, "I've tried to encourage everybody to use signed tags, but for kernel.org pulls it's not like it's a hard requirement, so no worries."

In another case, Tejun Heo sent in some pull requests, which Linus accepted – but he also said:

"Your two pulls sadly broke the perfect signed tag streak that we had this merge window up until just now.

"I'm very happy to see how people have started doing signed tags for everything, but that only makes your pull requests stand out even more.

"So yeah, despite not requiring it for kernel.org pulls, I'd _really_ like to make using signed tags just be the standard behavior for all the kernel pull requests.

"Can we try to aim for that next time? Please?"

What's really happening here? If Linus told everyone he would only accept signed pull requests, everyone would surely comply. And he has told people regarding build warnings that he will no longer accept patches that produce warnings. What's the difference between these situations?

In this particular instance, Tejun replied, "Hahaha, yeah, I lost my private key many years ago, so gotta get that sorted out first. Will do the signed pull from now on." And when Linus suggested using the same PGP key he used for his kernel.org account, Tejun added, "I don't have the private part of that pgp key anymore and have been just using the same ssh key. Nothing really requires the private key that I lost at least 5 years ago. On the plus side, the pgp key has been as secure as it gets. :) So, I gotta generate new keys, get it signed and replace the korg key and so on. I've just been really lazy."

There have been numerous other cases of Linus engaging in this form of "public shaming" to call people out for not using signed tags. It is always fairly lighthearted and nonthreatening, though we all know that Linus can be very harsh when he wants to be.

In another email thread, Al Viro submitted a pull request, and Linus replied, "Pulled. I do note that you are one of the remaining people not using signed tags. Not the only one, but _almost_. I've cajoled almost everybody else to use them…."

I think, first of all, each issue is its own special case. Whether it's build warnings, signed tags, the maximum line length in source code files, or the correct moment to break the ABI, the approach is always based on the specific situation under consideration.

In the case of signed pull requests, I think Linus considers it fundamentally outside of the kernel source tree. Warnings at compile time are another matter – in that case, the kernel code itself has problems that the developers just didn't care enough to fix.

Signed pull requests are not directly related to the kernel code. If a request is signed or unsigned, the kernel code remains the same. The exception would be if someone outside the normal process managed to sneak some hostile code into the pull request by faking the identity of the true maintainer.

However, there are also other mechanisms to ensure that only good code goes into the kernel. Presumably, each patch is looked at and tested by several developers and maintainers before being submitted to the kernel tree. And each of those reviewers adds their name to the list of people who have approved the patch. So as code wends its way into the kernel, there is some amount of history regarding who has passed judgment over it.

Changing the kernel's ABI, on the other hand, will break any compiled binaries that rely on that interface. Usually the only thing that could compel Linus to do that would be a security flaw that simply needed to be fixed. But in rare cases, if it's truly true that no one anywhere in the world is using a particular part of the ABI, and if removing that part would make the kernel more maintainable and cleaner, sometimes Linus will let it happen.

I find it fascinating trying to understand why Linus leads kernel development the way he does. I'm sure he would tell me that a lot of it is just his personal choice, and that other choices would be just as effective. But, still, I think it's extremely cool that his choices have led to Linux becoming the standard operating system for pretty much the entire known universe, with only some behind-the-times desktop users still struggling along with other systems.

The Author

The Linux kernel mailing list comprises the core of Linux development activities. Traffic volumes are immense, often reaching 10,000 messages in a week, and keeping up to date with the entire scope of development is a virtually impossible task for one person. One of the few brave souls to take on this task is Zack Brown.

Buy this article as PDF

Express-Checkout as PDF
Price $2.95
(incl. VAT)

Buy Linux Magazine

SINGLE ISSUES
 
SUBSCRIPTIONS
 
TABLET & SMARTPHONE APPS
Get it on Google Play

US / Canada

Get it on Google Play

UK / Australia

Related content

  • Kernel News

    Chronicler Zack Brown reports on the latest news, views, dilemmas, and developments within the Linux kernel community.

  • Kernel News

    Chronicler Zack Brown reports on the latest news, views, dilemmas, and developments within the Linux kernel community.

  • Kernel News

    Chronicler Zack Brown reports on the latest news, views, dilemmas, and developments within the Linux kernel community.

  • Tutorials – Build the Linux Kernel

    Get a super-customized Linux installation by configuring and compiling the kernel with just the features you need.

  • Kernel News

    The month Zack Brown reports on wonky typecasting.

comments powered by Disqus
Subscribe to our Linux Newsletters
Find Linux and Open Source Jobs
Subscribe to our ADMIN Newsletters

Support Our Work

Linux Magazine content is made possible with support from readers like you. Please consider contributing when you’ve found an article to be beneficial.

Learn More

News