diff mbox series

[2/8] kbuild: prevent exported headers from including <stdlib.h>, <stdbool.h>

Message ID 20220404061948.2111820-3-masahiroy@kernel.org (mailing list archive)
State New, archived
Headers show
Series UAPI: make more exported headers self-contained, and put them into test coverage | expand

Commit Message

Masahiro Yamada April 4, 2022, 6:19 a.m. UTC 
Some UAPI headers included <stdlib.h>, like this:

  #ifndef __KERNEL__
  #include <stdlib.h>
  #endif

As it turned out, they just included it for no good reason.

After some fixes, now I can compile-test UAPI headers
(CONFIG_UAPI_HEADER_TEST=y) without including <stdlib.h> from the
system header search paths.

To avoid somebody getting it back again, this commit adds the dummy
header, usr/dummy-include/stdlib.h

I added $(srctree)/usr/dummy-include to the header search paths.
Because it is searched before the system directories, if someone
tries to include <stdlib.h>, they will see the error message.

While I am here, I also replaced $(objtree)/usr/include with $(obj),
but it has no functional change.

If we can make kernel headers self-contained (that is, none of exported
kernel headers includes system headers), we will be able to add the
-nostdinc flag, but that is much far from where we stand now.

As a realistic solution, we can ban header inclusion individually by
putting a dummy header into usr/dummy-include/.

Currently, no header include <stdbool.h>. I put it as well before somebody
attempts to use it.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

 usr/dummy-include/stdbool.h | 7 +++++++
 usr/dummy-include/stdlib.h  | 7 +++++++
 usr/include/Makefile        | 2 +-
 3 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 usr/dummy-include/stdbool.h
 create mode 100644 usr/dummy-include/stdlib.h

Comments

Christoph Hellwig April 4, 2022, 7:41 a.m. UTC | #1 
On Mon, Apr 04, 2022 at 03:19:42PM +0900, Masahiro Yamada wrote:
> If we can make kernel headers self-contained (that is, none of exported
> kernel headers includes system headers), we will be able to add the
> -nostdinc flag, but that is much far from where we stand now.

What is still missing for that?
Arnd Bergmann April 4, 2022, 8:01 a.m. UTC | #2 
On Mon, Apr 4, 2022 at 9:41 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Apr 04, 2022 at 03:19:42PM +0900, Masahiro Yamada wrote:
> > If we can make kernel headers self-contained (that is, none of exported
> > kernel headers includes system headers), we will be able to add the
> > -nostdinc flag, but that is much far from where we stand now.
>
> What is still missing for that?

One case that I don't know how to solve is

include/uapi/sound/asound.h:typedef struct { unsigned char
pad[sizeof(time_t) - sizeof(int)]; } __time_pad;

Here we define a structure layout based on a libc-provided type. There are two
possible variants (32-bit and 64-bit time_t), and the kernel interface
can handle
both versions because they get different ioctl command numbers, but user space
must see the one that matches its normal time_t.

There are a couple of similar cases like this, but I think the other ones don't
need to define architecture specific padding like this.

       Arnd
Masahiro Yamada April 4, 2022, 8:03 a.m. UTC | #3 
On Mon, Apr 4, 2022 at 4:41 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Apr 04, 2022 at 03:19:42PM +0900, Masahiro Yamada wrote:
> > If we can make kernel headers self-contained (that is, none of exported
> > kernel headers includes system headers), we will be able to add the
> > -nostdinc flag, but that is much far from where we stand now.
>
> What is still missing for that?


I changed as follows:

diff --git a/usr/include/Makefile b/usr/include/Makefile
index fa9819e022b7..169fca1a0f28 100644
--- a/usr/include/Makefile
+++ b/usr/include/Makefile
@@ -15,7 +15,7 @@ UAPI_CFLAGS += $(filter -m32 -m64 --target=%,
$(KBUILD_CFLAGS))
 # USERCFLAGS might contain sysroot location for CC.
 UAPI_CFLAGS += $(USERCFLAGS)

-override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) -I$(objtree)/usr/include
+override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile)
-I$(objtree)/usr/include -nostdinc

 # The following are excluded for now because they fail to build.
 #




I got this:



masahiro@grover:~/ref/linux$ make -j8 allyesconfig usr/include/
#
# No change to .config
#
  DESCEND objtool
  CALL    scripts/atomic/check-atomics.sh
  CALL    scripts/checksyscalls.sh
  HDRTEST usr/include/linux/wireless.h
  HDRTEST usr/include/linux/atmlec.h
  HDRTEST usr/include/linux/if_fc.h
  HDRTEST usr/include/linux/iso_fs.h
  HDRTEST usr/include/linux/sysinfo.h
  HDRTEST usr/include/linux/un.h
  HDRTEST usr/include/linux/ax25.h
  HDRTEST usr/include/linux/map_to_14segment.h
In file included from ./usr/include/linux/wireless.h:75,
                 from <command-line>:
./usr/include/linux/if.h:28:10: fatal error: sys/socket.h: No such
file or directory
   28 | #include <sys/socket.h>                 /* for struct
sockaddr.         */
      |          ^~~~~~~~~~~~~~
compilation terminated.
make[3]: *** [usr/include/Makefile:101:
usr/include/linux/wireless.hdrtest] Error 1
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:550: usr/include] Error 2
make[1]: *** [Makefile:1834: usr] Error 2
make: *** [Makefile:350: __build_one_by_one] Error 2
Nick Desaulniers April 4, 2022, 5:34 p.m. UTC | #4 
On Mon, Apr 04, 2022 at 12:41:54AM -0700, Christoph Hellwig wrote:
> On Mon, Apr 04, 2022 at 03:19:42PM +0900, Masahiro Yamada wrote:
> > If we can make kernel headers self-contained (that is, none of exported
> > kernel headers includes system headers), we will be able to add the
> > -nostdinc flag, but that is much far from where we stand now.

This is something I'd like to see done. IMO, the kernel headers should
be the independent variable of which the libc is the dependendent
variable.

Android's libc, Bionic, is making use of the UAPI headers. They are
doing some rewriting of UAPI headers, but I'd like to see what needs to
be upstreamed from there. I just noticed
include/uapi/linux/libc-compat.h, which seems like a good place for such
compat related issues.

In particular, having UAPI_HEADER_TESTS depend on CC_CAN_LINK is
something I think we can works towards removing. The header tests
themselves don't link; they force a dependency on a prebuilt libc
sysroot, and they only need the headers from the sysroot because of this
existing circular dependency between kernel headers and libc headers.

I'd be happy to be explicitly cc'ed on changes like this series, going
forward. Masahiro, if there's parts you'd like me to help with besides
just code review, please let me know how I can help.

> 
> What is still missing for that?
Masahiro Yamada April 5, 2022, 1:12 a.m. UTC | #5 
On Tue, Apr 5, 2022 at 2:34 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Mon, Apr 04, 2022 at 12:41:54AM -0700, Christoph Hellwig wrote:
> > On Mon, Apr 04, 2022 at 03:19:42PM +0900, Masahiro Yamada wrote:
> > > If we can make kernel headers self-contained (that is, none of exported
> > > kernel headers includes system headers), we will be able to add the
> > > -nostdinc flag, but that is much far from where we stand now.
>
> This is something I'd like to see done. IMO, the kernel headers should
> be the independent variable of which the libc is the dependendent
> variable.
>
> Android's libc, Bionic, is making use of the UAPI headers. They are
> doing some rewriting of UAPI headers, but I'd like to see what needs to
> be upstreamed from there. I just noticed
> include/uapi/linux/libc-compat.h, which seems like a good place for such
> compat related issues.
>
> In particular, having UAPI_HEADER_TESTS depend on CC_CAN_LINK is
> something I think we can works towards removing. The header tests
> themselves don't link; they force a dependency on a prebuilt libc
> sysroot, and they only need the headers from the sysroot because of this
> existing circular dependency between kernel headers and libc headers.
>
> I'd be happy to be explicitly cc'ed on changes like this series, going
> forward. Masahiro, if there's parts you'd like me to help with besides
> just code review, please let me know how I can help.


I wanted to make uapi headers as self-contained as possible,
but I did not see much progress.

I just fixed up some low-hanging fruits, but there are still
many remaining issues.

Thank you very much for your contribution:
https://lore.kernel.org/all/20220404175448.46200-1-ndesaulniers@google.com/

If you eliminate other issues, that would be appreciated.


> >
> > What is still missing for that?
diff mbox series

Patch

diff --git a/usr/dummy-include/stdbool.h b/usr/dummy-include/stdbool.h
new file mode 100644
index 000000000000..54ff9e9c90ac
--- /dev/null
+++ b/usr/dummy-include/stdbool.h
@@ -0,0 +1,7 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _STDBOOL_H
+#define _STDBOOL_H
+
+#error "Please do not include <stdbool.h> from exported headers"
+
+#endif /* _STDBOOL_H */
diff --git a/usr/dummy-include/stdlib.h b/usr/dummy-include/stdlib.h
new file mode 100644
index 000000000000..e8c21888e371
--- /dev/null
+++ b/usr/dummy-include/stdlib.h
@@ -0,0 +1,7 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _STDLIB_H
+#define _STDLIB_H
+
+#error "Please do not include <stdlib.h> from exported headers"
+
+#endif /* _STDLIB_H */
diff --git a/usr/include/Makefile b/usr/include/Makefile
index fa9819e022b7..7740777b49f8 100644
--- a/usr/include/Makefile
+++ b/usr/include/Makefile
@@ -15,7 +15,7 @@  UAPI_CFLAGS += $(filter -m32 -m64 --target=%, $(KBUILD_CFLAGS))
 # USERCFLAGS might contain sysroot location for CC.
 UAPI_CFLAGS += $(USERCFLAGS)
 
-override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) -I$(objtree)/usr/include
+override c_flags = $(UAPI_CFLAGS) -Wp,-MMD,$(depfile) -I $(obj) -I $(srctree)/usr/dummy-include
 
 # The following are excluded for now because they fail to build.
 #