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: <CAGsJ_4y+J9uaj=h6JfmKeq5SM5ic9ZKwgDONBP+JbUm6SJtzNg@mail.gmail.com>
Date: Fri, 29 Aug 2025 09:22:21 +1200
From: Barry Song <21cnbao@...il.com>
To: Qinxin Xia <xiaqinxin@...wei.com>
Cc: m.szyprowski@...sung.com, robin.murphy@....com, 
	jonathan.cameron@...wei.com, prime.zeng@...wei.com, fanghao11@...wei.com, 
	linux-kernel@...r.kernel.org, linuxarm@...wei.com, yangyicong@...wei.com
Subject: Re: [PATCH 1/2] tools/dma: move dma_map_benchmark from selftests to tools/dma

On Thu, Aug 28, 2025 at 12:07 AM Qinxin Xia <xiaqinxin@...wei.com> wrote:
>
>
>
> On 2025/8/22 09:12:07, Barry Song <21cnbao@...il.com> wrote:
> >>
> >> Does usr/include have header files? Did you run make headers_install
> >> before make?
> >> [xiaqinxin@...alhost linux]$ make headers_install
> >>     HOSTCC  scripts/basic/fixdep
> >>     HOSTCC  scripts/unifdef
> >>     WRAP    arch/arm64/include/generated/uapi/asm/socket.h
> >>     SYSHDR  arch/arm64/include/generated/uapi/asm/unistd_64.h
> >>     HDRINST usr/include/asm-generic/mman.h
> >>     HDRINST usr/include/asm-generic/stat.h
> >>     HDRINST usr/include/asm-generic/ucontext.h
> >>     HDRINST usr/include/asm-generic/int-ll64.h
> >>     HDRINST usr/include/asm-generic/unistd.h
> >>     HDRINST usr/include/asm-generic/kvm_para.h
> >>     HDRINST usr/include/asm-generic/types.h
> >>     HDRINST usr/include/asm-generic/ipcbuf.h
> >>     HDRINST usr/include/asm-generic/termbits-common.h
> >> ...
> >> [xiaqinxin@...alhost linux]$ cd tools/dma/
> >> [xiaqinxin@...alhost dma]$ make
> >> cc -I../../usr/include -I../../include dma_map_benchmark.c -o
> >> dma_map_benchmark
> >
> > This is really frustrating. Why do other parts not need this, but
> > dma_map_benchmark does? It is also not acceptable to hardcode the
> > path to usr/include.
> >
> > It is also not good practice to access a kernel header directly from a
> > userspace tool - such as -I../../include.
> >
> > Shouldn't map_benchmark.h be a proper UAPI header that gets installed
> > into the toolchain like the others?
> >
> Hello Barry :
>
> This include file is inherited from the original version, and there are
> similar
>
> method in other parts :
>
> pcmcia/Makefile:CFLAGS := -I../../usr/include
> laptop/dslm/Makefile:CFLAGS := -I../../usr/include
> accounting/Makefile:CFLAGS := -I../../usr/include
>
> During compilation, the system searches for header files from
> ../../usr/include first.
>
> If no header file is found in ../../usr/include, the system attempts to
> get header files

The difference is that, for them, the build still passes even without
running `header_install` (and thus without `../../usr/include`).

tools/laptop/dslm$ make
gcc -I../../usr/include    dslm.c   -o dslm

tools/pcmcia$ make
gcc -I../../usr/include    crc32hash.c   -o crc32hash

For tools/accounting, the build fails mainly because the UAPI header
files in the toolchain may be older than those in the latest kernel. so
we need make headers_install to update.

But I don’t really understand why we need it. My guess is that the
"-I../../include" option overrides the system header files, which then
causes type conflicts.

cc -I../../include dma_map_benchmark.c -o dma_map_benchmark
In file included from dma_map_benchmark.c:13:
../../include/linux/types.h:20:33: error: conflicting types for
‘fd_set’; have ‘__kernel_fd_set’
   20 | typedef __kernel_fd_set         fd_set;
      |                                 ^~~~~~
In file included from /usr/include/x86_64-linux-gnu/sys/types.h:179,
                 from /usr/include/stdlib.h:395,
                 from dma_map_benchmark.c:8:
/usr/include/x86_64-linux-gnu/sys/select.h:70:5: note: previous
declaration of ‘fd_set’ with type ‘fd_set’
   70 |   } fd_set;
      |     ^~~~~~
In file included from dma_map_benchmark.c:13:
../../include/linux/types.h:21:33: error: conflicting types for
‘dev_t’; have ‘__kernel_dev_t’ {aka ‘unsigned int’}
   21 | typedef __kernel_dev_t          dev_t;
      |                                 ^~~~~

You should be referring to the system types.h, but the -I../../include
option makes you pick up the wrong kernel types.h. However, when you
do have "../../usr/include", you end up with the correct types.h from UAPI.

I really dislike all this *mess*. You can fix it by doing the following:

diff --git a/tools/dma/Makefile b/tools/dma/Makefile
index 841b54896288..cec09abf47cd 100644
--- a/tools/dma/Makefile
+++ b/tools/dma/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 bindir ?= /usr/bin

-CFLAGS += -I../../include -I../../usr/include
+CFLAGS += -idirafter ../../include

>
> from the system directory of the compilation environment. So maybe in
> some compilation
>
> environments, compiling these modules might have the same problem...
>
> 'struct map_benchmark' is defined in map_benchmark.h which is used by
> map_benchmark.c
>
> Do we need to define them separately in the kernel and uapi header files?>>

Ideally, yes—then the -I../../include option would no longer be necessary.

Thanks
Barry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ