linux-kernel - Re: [PATCH] kbuild: add extra-y to targets-for-modules

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 16 Dec 2020 18:04:44 -0500
From:   Joe Lawrence <joe.lawrence@...hat.com>
To:     Masahiro Yamada <masahiroy@...nel.org>,
        Artem Savkov <artem.savkov@...il.com>
Cc:     WANG Chao <chao.wang@...oud.cn>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>
Subject: Re: [PATCH] kbuild: add extra-y to targets-for-modules

On 12/16/20 1:14 AM, Masahiro Yamada wrote:
> On Tue, Dec 8, 2020 at 11:31 PM Artem Savkov <artem.savkov@...il.com> wrote:
>>
>> On Tue, Dec 08, 2020 at 05:20:35PM +0800, WANG Chao wrote:
>>> Sorry for the late reply.
>>>
>>> On 11/25/20 at 10:42P, Masahiro Yamada wrote:
>>>> On Tue, Nov 24, 2020 at 12:05 AM WANG Chao <chao.wang@...oud.cn> wrote:
>>>>>
>>>>> On 11/23/20 at 02:23P, Masahiro Yamada wrote:
>>>>>> On Tue, Nov 3, 2020 at 3:23 PM WANG Chao <chao.wang@...oud.cn> wrote:
>>>>>>>
>>>>>>> extra-y target doesn't build for 'make M=...' since commit 6212804f2d78
>>>>>>> ("kbuild: do not create built-in objects for external module builds").
>>>>>>>
>>>>>>> This especially breaks kpatch, which is using 'extra-y := kpatch.lds'
>>>>>>> and 'make M=...' to build livepatch patch module.
>>>>>>>
>>>>>>> Add extra-y to targets-for-modules so that such kind of build works
>>>>>>> properly.
>>>>>>>
>>>>>>> Signed-off-by: WANG Chao <chao.wang@...oud.cn>
>>>>>>> ---
>>>>>>>   scripts/Makefile.build | 2 +-
>>>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
>>>>>>> index ae647379b579..0113a042d643 100644
>>>>>>> --- a/scripts/Makefile.build
>>>>>>> +++ b/scripts/Makefile.build
>>>>>>> @@ -86,7 +86,7 @@ ifdef need-builtin
>>>>>>>   targets-for-builtin += $(obj)/built-in.a
>>>>>>>   endif
>>>>>>>
>>>>>>> -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
>>>>>>> +targets-for-modules := $(extra-y) $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
>>>>>>>
>>>>>>>   ifdef need-modorder
>>>>>>>   targets-for-modules += $(obj)/modules.order
>>>>>>> --
>>>>>>> 2.29.1
>>>>>>>
>>>>>>
>>>>>> NACK.
>>>>>>
>>>>>> Please fix your Makefile.
>>>>>>
>>>>>> Hint:
>>>>>> https://patchwork.kernel.org/project/linux-kbuild/patch/20201123045403.63402-6-masahiroy@kernel.org/
>>>>>>
>>>>>>
>>>>>> Probably what you should use is 'targets'.
>>>>>
>>>>> I tried with 'targets' and 'always-y'. Both doesn't work for me.
>>>>>
>>>>> I narraw it down to the following example:
>>>>>
>>>>> cat > Makefile << _EOF_
>>>>> obj-m += foo.o
>>>>>
>>>>> ldflags-y += -T $(src)/kpatch.lds
>>>>> always-y += kpatch.lds
>>>>>
>>>>> foo-objs += bar.o
>>>>>
>>>>> all:
>>>>>          make -C /lib/modules/$(shell uname -r)/build M=$(PWD)
>>>>> clean:
>>>>>          make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
>>>>> _EOF_
>>>>>
>>>>> Take a look into scripts/Makefile.build:488:
>>>>>
>>>>> __build: $(if $(KBUILD_BUILTIN), $(targets-for-builtin)) \
>>>>>           $(if $(KBUILD_MODULES), $(targets-for-modules)) \
>>>>>           $(subdir-ym) $(always-y)
>>>>>          @:
>>>>>
>>>>> 'always-y' is built after 'targets-for-modules'. This makes
>>>>> 'targets-for-modules' fails because kpatch.lds isn't there.
>>>>
>>>>
>>>> Heh, you rely on the targets built from left to right,
>>>> and you have never thought Make supports the parallel option -j.
>>>
>>> You're right. I missed that.
>>>
>>>>
>>>>
>>>> You need to specify the dependency if you expect objects
>>>> are built in the particular order.
>>>>
>>>> However, in this case, using ldflags-y looks wrong
>>>> in the first place.
>>>>
>>>> The linker script is used when combining the object
>>>> as well as the final link of *.ko
>>
>> We want linker script to be used on both those steps, otherwise modpost
>> fails.
> 
> 
> In that case, does the following work?
> (untested)
> 
> 
> 
> diff --git a/kmod/patch/Makefile b/kmod/patch/Makefile
> index e017b17..02d4c66 100644
> --- a/kmod/patch/Makefile
> +++ b/kmod/patch/Makefile
> @@ -12,7 +12,9 @@ endif
> 
>   obj-m += $(KPATCH_NAME).o
>   ldflags-y += -T $(src)/kpatch.lds
> -extra-y := kpatch.lds
> +targets += kpatch.lds
> +
> +$(obj)/$(KPATCH_NAME).o: $(obj)/kpatch.lds
> 
>   $(KPATCH_NAME)-objs += patch-hook.o output.o
> 

Hi Masahiro,

Yeah this is more or less what Artem came up with:
https://github.com/dynup/kpatch/pull/1149

though we hadn't added kpatch.lds to targets.  Is there documentation 
somewhere on what effect "targets" has for out-of-tree builds?

Thanks,

-- Joe