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: <CAK7LNAQThGkgtKgquRPv8Ysi_omedRthF1_++apKda-xWeWcbA@mail.gmail.com>
Date: Wed, 2 Apr 2025 03:46:34 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Jani Nikula <jani.nikula@...ux.intel.com>, Jason Gunthorpe <jgg@...dia.com>, 
	Dave Airlie <airlied@...il.com>, dri-devel <dri-devel@...ts.freedesktop.org>, 
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [git pull] drm for 6.15-rc1

On Wed, Apr 2, 2025 at 1:12 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Tue, 1 Apr 2025 at 05:21, Jani Nikula <jani.nikula@...ux.intel.com> wrote:
> >
> > The header checks have existed for uapi headers before, including the,
> > uh, turds, but apparently adding them in drm broke the camel's back.
>
> The uapi header test things never caused any problems for me [*],
> because they don't actually pollute the source tree.
>
> Why? Because they all end up in that generated 'usr/include/' directory.
>
> So when I look at the source files, filename completion is entirely
> unaffected, and it all works fine.
>
> Look, I can complete something like
>
>     include/uapi/asm-generic/poll.h
>
> perfectly fine, because there is *not* some generated turd that affects it all.
>
> Because for the uapi files those hdrtest files end up being in
>
>     ./usr/include/asm-generic/poll.hdrtest
>
> and I never have any reason to really look at that subdirectory at
> all, since it's all generated.
>
> Or put another way - if I _were_ to look at it, it would be exactly
> because I want to see some generated file, in which case the 'hdrtest'
> turd would be part of it.
>
> (Although I cannot recall that ever having actually happened, to be
> honest - but looking at various header files is common, and I hit the
> drm case immediately)
>
> Would you mind taking more of that uapi approach than creating that
> hidden directory specific to the drm tree? Maybe this could *all* be
> generalized?
>
>            Linus
>
> [*] I say "never caused any problems for me", but maybe it did way in
> the past and it was fixed and I just don't recall. I have definitely
> complained about pathname completion issues to people before.

I know you dislike this, and I NACKed this before (but too late):
https://lore.kernel.org/dri-devel/CAK7LNATHXwEkjJHP7b-ZmhzLfyyuOdsyimna-=r-sJk+DxigrA@mail.gmail.com/

Compile-testing UAPI headers is useful
(and I believe this is the only case where it is useful)
because userspace is independent of any CONFIG option,
and we have no control over the include order in
userspace programs.

In contrast, kernel-space headers are conditionally parsed,
depending on CONFIG options.

You dislike this feature for the reason of TAB-completion,
but I am negative about this feature from another perspective.

We cannot avoid a false-positive:
https://lore.kernel.org/all/20190718130835.GA28520@lst.de/

It is not <drm/*.h> but <linux/*.h>
However, it is annoying to make every header self-contained
"just because we are checking this".
Actually, it is not a real problem when the relevant
CONFIG option is disabled.
I did not interrupt him because he was doing this
checkunder drivers/gpu/drm/i915/.
()
But, I am opposed to expanding it to more-global include/drm/,
or any other subsystem.

In my opinion, the right fix is
"git revert 62ae45687e43"


If we continue this, maybe leave a caution
in capital letters?


diff --git a/include/Kbuild b/include/Kbuild
index 5e76a599e2dd..4dff23ae51a4 100644
--- a/include/Kbuild
+++ b/include/Kbuild
@@ -1 +1,3 @@
+# The drm subsystem is strict about the self-containedness of header files.
+# OTHER SUBSYSTEMS SHOULD NOT FOLLOW THIS PRACTICE.
 obj-$(CONFIG_DRM_HEADER_TEST)  += drm/



--
Best Regards

Masahiro Yamada

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ