linux-kernel - Re: [PATCH] tools: hv: Fix cross-compilation

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: <20241213040102.GA28827@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Date: Thu, 12 Dec 2024 20:01:02 -0800
From: Saurabh Singh Sengar <ssengar@...ux.microsoft.com>
To: Wei Liu <wei.liu@...nel.org>
Cc: kys@...rosoft.com, haiyangz@...rosoft.com, decui@...rosoft.com,
	linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
	ssengar@...rosoft.com, avladu@...udbasesolutions.com
Subject: Re: [PATCH] tools: hv: Fix cross-compilation

On Thu, Dec 12, 2024 at 11:06:16PM +0000, Wei Liu wrote:
> On Thu, Dec 12, 2024 at 12:28:34AM -0800, Saurabh Sengar wrote:
> > Use the native ARCH only incase it is not set, this will allow
> > the cross complilation where ARCH is explicitly set. Add few
> > info prints as well to know what arch and toolchain is getting
> > used to build it.
> > 
> > Additionally, simplify the check for ARCH so that fcopy daemon
> > is build only for x86_64.
> > 
> > Fixes: 82b0945ce2c2 ("tools: hv: Add new fcopy application based on uio driver")
> > Reported-by: Adrian Vladu <avladu@...udbasesolutions.com>
> > Closes: https://lore.kernel.org/linux-hyperv/Z1Y9ZkAt9GPjQsGi@liuwe-devbox-debian-v2/
> > Signed-off-by: Saurabh Sengar <ssengar@...ux.microsoft.com>
> > ---
> >  tools/hv/Makefile | 14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/hv/Makefile b/tools/hv/Makefile
> > index 34ffcec264ab..d29e6be6309b 100644
> > --- a/tools/hv/Makefile
> > +++ b/tools/hv/Makefile
> > @@ -2,7 +2,7 @@
> >  # Makefile for Hyper-V tools
> >  include ../scripts/Makefile.include
> >  
> > -ARCH := $(shell uname -m 2>/dev/null)
> > +ARCH ?= $(shell uname -m 2>/dev/null)
> >  sbindir ?= /usr/sbin
> >  libexecdir ?= /usr/libexec
> >  sharedstatedir ?= /var/lib
> > @@ -20,18 +20,26 @@ override CFLAGS += -O2 -Wall -g -D_GNU_SOURCE -I$(OUTPUT)include
> >  override CFLAGS += -Wno-address-of-packed-member
> >  
> >  ALL_TARGETS := hv_kvp_daemon hv_vss_daemon
> > -ifneq ($(ARCH), aarch64)
> > +ifeq ($(ARCH), x86_64)
> 
> Technically speaking, you can also build this for x86 (32bit). Whether
> anybody uses it is another question.

My intention is to allow fcopy daemon build only for the arch it has
been tested. IMO its better than restricting only for arm64/aarch64.

I tried with gcc '-m32' switch which I believe is for 32 bit x86 compilation
I see problems with it on other (kvp daemon) daemons too. I think we never
cared about 32 bit.

saurabh@...rabh:/work/linux-next/tools/hv$ make ARCH=x86 CFLAGS=-m32
make[1]: Entering directory '/work/linux-next/tools/hv'
  CC      hv_kvp_daemon.o
hv_kvp_daemon.c:25:10: fatal error: sys/poll.h: No such file or directory
   25 | #include <sys/poll.h>
      |          ^~~~~~~~~~~~
compilation terminated.
make[1]: *** [/work/linux-next/tools/build/Makefile.build:106: hv_kvp_daemon.o] Error 1
make[1]: Leaving directory '/work/linux-next/tools/hv'
make: *** [Makefile:37: hv_kvp_daemon-in.o] Error 2


I don't have any strong opinion here, if you want I can allow x86 compilation
for fcopy daemon as well.

Please let me know what is your preference.

> 
> >  ALL_TARGETS += hv_fcopy_uio_daemon
> >  endif
> >  ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
> >  
> >  ALL_SCRIPTS := hv_get_dhcp_info.sh hv_get_dns_info.sh hv_set_ifconfig.sh
> >  
> > -all: $(ALL_PROGRAMS)
> > +all: info $(ALL_PROGRAMS)
> >  
> >  export srctree OUTPUT CC LD CFLAGS
> >  include $(srctree)/tools/build/Makefile.include
> >  
> > +info:
> > +	@echo "---------------------"
> > +	@echo "Building for:"
> > +	@echo "CC $(CC)"
> > +	@echo "LD $(LD)"
> > +	@echo "ARCH $(ARCH)"
> > +	@echo "---------------------"
> 
> I don't think this is needed. Anyone who's building the kernel source
> should know what tool chain they are using and architecture they're
> building for.

I am fine removing it in V2.

- Saurabh