Re: [RFC PATCH v2] checkpatch: add shebang check to EXECUTE_PERMISSIONS
On 14/10/20 11:16 am, Lukas Bulwahn wrote: > > > On Tue, 13 Oct 2020, Ujjwal Kumar wrote: > >> checkpatch.pl checks for invalid EXECUTE_PERMISSIONS on source >> files. The script leverages filename extensions and its path in >> the repository to decide whether to allow execute permissions on >> the file or not. >> >> Based on current check conditions, a perl script file having >> execute permissions, without '.pl' extension in its filename >> and not belonging to 'scripts/' directory is reported as ERROR >> which is a false positive. >> >> Adding a shebang check along with current conditions will make >> the check more generalised and improve checkpatch reports. >> To do so, without breaking the core design decision of checkpatch, >> we can fetch the first line from the patch itself and match it for >> a shebang pattern. >> >> There can be cases where the first line is not part of the patch. >> For instance: a patch that only changes permissions without >> changing any of the file content. >> In that case there may be a false positive report but in the end we >> will have less false positives as we will be handling some of the >> unhandled cases. >> > > I get the intent of your addition. However: > > I would bet that you only find one or two in a million commits, that would > actually benefit for this special check of a special case of a special > rule... > > So given the added complexity of yet another 19 lines in checkpatch with > little benefit of lowering false positive reports, I would be against > inclusion. Yes, it is a subtle change. > > You can provide convincing arguments with an evaluation, where you show > on how many commits this change would really make a difference... Some statistics: I aggregated commits which involved 'mode change' on script files. Totaling to 478 (looked for logs of only executable files in the repo). At current state, checkpatch reports 26 ERRORS (false positives) with 'hashbang' test we have 5 false positives. Without 'scripts/' directory test, checkpatch reports 82 ERRORS (false positives) with 'hashbang' test we have 35 false positives. > > It is probably better and simpler to just have a script checking for > execute bits on all files in the repository on linux-next (with a list of > known intended executable files) and just report to you and then to the > developers when a new file with unintentional execute bit appeared. > > Keep up the good work. I just fear this patch is a dead end. > > There is still a lot of other issues you can contribute to. > > Just one bigger project example: Comparing clang-format suggestions on > patches against checkpatch.pl suggestions are fine-tuning both of them to fit > to > the actual kernel style. > > Lukas > Thanks Ujjwal Kumar
Re: [RFC PATCH v2] checkpatch: add shebang check to EXECUTE_PERMISSIONS
On 13/10/20 5:31 pm, Ujjwal Kumar wrote:
> checkpatch.pl checks for invalid EXECUTE_PERMISSIONS on source
> files. The script leverages filename extensions and its path in
> the repository to decide whether to allow execute permissions on
> the file or not.
>
> Based on current check conditions, a perl script file having
> execute permissions, without '.pl' extension in its filename
> and not belonging to 'scripts/' directory is reported as ERROR
> which is a false positive.
>
> Adding a shebang check along with current conditions will make
> the check more generalised and improve checkpatch reports.
> To do so, without breaking the core design decision of checkpatch,
> we can fetch the first line from the patch itself and match it for
> a shebang pattern.
>
> There can be cases where the first line is not part of the patch.
> For instance: a patch that only changes permissions without
> changing any of the file content.
> In that case there may be a false positive report but in the end we
> will have less false positives as we will be handling some of the
> unhandled cases.
>
> Signed-off-by: Ujjwal Kumar
> ---
> Changes in v2:
> - Spelling correction and add example to commit
> message
> - Code style changes
> - Remove unncessary function argument
> - Use non-capturing group in regexp
>
> scripts/checkpatch.pl | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fab38b493cef..7ebbee9c3672 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1795,6 +1795,23 @@ sub get_stat_here {
> return $herectx;
> }
>
> +sub get_shebang {
> + my ($linenr) = @_;
> + my $rawline = "";
> + my $shebang = "";
> +
> + $rawline = raw_line($linenr, 3);
I'm wondering if the range information can be at a
different offset from the 'new mode line'.
> + if (defined($rawline) &&
> + $rawline =~ /^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
> + if (defined($1) && $1 == 1) {
> + $shebang = raw_line($linenr, 4);
> + $shebang = substr($shebang, 1);
> + }
> + }
> +
> + return $shebang;
> +}
> +
> sub cat_vet {
> my ($vet) = @_;
> my ($res, $coded);
> @@ -2680,7 +2697,9 @@ sub process {
> # Check for incorrect file permissions
> if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) {
> my $permhere = $here . "FILE: $realfile\n";
> + my $shebang = get_shebang($linenr);
> if ($realfile !~ m@scripts/@ &&
> + $shebang !~ /^#!\s*(?:\/\w)+.*/ &&
> $realfile !~ /\.(py|pl|awk|sh)$/) {
Consider the following case:
a python script file with '.py' filename extension but without
a shebang line. Would it be meaningful to allow execute permission
on such a file?
> ERROR("EXECUTE_PERMISSIONS",
> "do not set execute permissions for
> source files\n" . $permhere);
>
> base-commit: 148fdf990dee4efd23c1114811b205de9c966680
> --
> 2.26.2
>
Thanks
Ujjwal Kumar
[RFC PATCH v2] checkpatch: add shebang check to EXECUTE_PERMISSIONS
checkpatch.pl checks for invalid EXECUTE_PERMISSIONS on source
files. The script leverages filename extensions and its path in
the repository to decide whether to allow execute permissions on
the file or not.
Based on current check conditions, a perl script file having
execute permissions, without '.pl' extension in its filename
and not belonging to 'scripts/' directory is reported as ERROR
which is a false positive.
Adding a shebang check along with current conditions will make
the check more generalised and improve checkpatch reports.
To do so, without breaking the core design decision of checkpatch,
we can fetch the first line from the patch itself and match it for
a shebang pattern.
There can be cases where the first line is not part of the patch.
For instance: a patch that only changes permissions without
changing any of the file content.
In that case there may be a false positive report but in the end we
will have less false positives as we will be handling some of the
unhandled cases.
Signed-off-by: Ujjwal Kumar
---
Changes in v2:
- Spelling correction and add example to commit
message
- Code style changes
- Remove unncessary function argument
- Use non-capturing group in regexp
scripts/checkpatch.pl | 19 +++
1 file changed, 19 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..7ebbee9c3672 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1795,6 +1795,23 @@ sub get_stat_here {
return $herectx;
}
+sub get_shebang {
+ my ($linenr) = @_;
+ my $rawline = "";
+ my $shebang = "";
+
+ $rawline = raw_line($linenr, 3);
+ if (defined($rawline) &&
+ $rawline =~ /^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
+ if (defined($1) && $1 == 1) {
+ $shebang = raw_line($linenr, 4);
+ $shebang = substr($shebang, 1);
+ }
+ }
+
+ return $shebang;
+}
+
sub cat_vet {
my ($vet) = @_;
my ($res, $coded);
@@ -2680,7 +2697,9 @@ sub process {
# Check for incorrect file permissions
if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) {
my $permhere = $here . "FILE: $realfile\n";
+ my $shebang = get_shebang($linenr);
if ($realfile !~ m@scripts/@ &&
+ $shebang !~ /^#!\s*(?:\/\w)+.*/ &&
$realfile !~ /\.(py|pl|awk|sh)$/) {
ERROR("EXECUTE_PERMISSIONS",
"do not set execute permissions for
source files\n" . $permhere);
base-commit: 148fdf990dee4efd23c1114811b205de9c966680
--
2.26.2
Re: [PATCH v2 2/2] kbuild: use interpreters to invoke scripts
On 13/10/20 12:24 am, Bernd Petrovitsch wrote: > Hi all! > >>>> diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile >>>> index 703b1c4f6d12..86d42a2d09cb 100644 >>>> --- a/arch/ia64/Makefile >>>> +++ b/arch/ia64/Makefile >>>> @@ -27,8 +27,8 @@ cflags-y := -pipe $(EXTRA) -ffixed-r13 >>>> -mfixed-range=f12-f15,f32-f127 \ >>>> -falign-functions=32 -frename-registers >>>> -fno-optimize-sibling-calls >>>> KBUILD_CFLAGS_KERNEL := -mconstant-gp >>>> >>>> -GAS_STATUS= $(shell $(srctree)/arch/ia64/scripts/check-gas >>>> "$(CC)" "$(OBJDUMP)") >>>> -KBUILD_CPPFLAGS += $(shell $(srctree)/arch/ia64/scripts/toolchain-flags >>>> "$(CC)" "$(OBJDUMP)" "$(READELF)") >>>> +GAS_STATUS= $(shell $(CONFIG_SHELL) >>>> $(srctree)/arch/ia64/scripts/check-gas "$(CC)" "$(OBJDUMP)") >>>> +KBUILD_CPPFLAGS += $(shell $(CONFIG_SHELL) >>>> $(srctree)/arch/ia64/scripts/toolchain-flags "$(CC)" "$(OBJDUMP)" >>>> "$(READELF)") >>> >>> Here is an instance of what Masahiro-san pointed out being wrong. >>> >>> Ujjwal, will you send a v3? >> >> Following is the quoted text from the reply mail from Masahiro >> >>>> -GAS_STATUS = $(shell $(srctree)/arch/ia64/scripts/check-gas "$(CC)" >>>> "$(OBJDUMP)") >>>> -KBUILD_CPPFLAGS += $(shell $(srctree)/arch/ia64/scripts/toolchain-flags >>>> "$(CC)" "$(OBJDUMP)" "$(READELF)") >>>> +GAS_STATUS = $($(CONFIG_SHELL) $(srctree)/arch/ia64/scripts/check-gas >>>> "$(CC)" "$(OBJDUMP)") >>>> +KBUILD_CPPFLAGS += $($(CONFIG_SHELL) >>>> $(srctree)/arch/ia64/scripts/toolchain-flags "$(CC)" "$(OBJDUMP)" >>>> "$(READELF)") >>> >>> >>> >>> These changes look wrong to me. >>> >>> $($(CONFIG_SHELL)-> $(shell $(CONFIG_SHELL) >>> >> >> From the above text, I understand as follows: > > Did you actually *test* that (expecially) these lines work > afterwards as good as before? Yes, I did check my changes. TBH, I spent a considerable amount of time in doing so (given that I'm new to the community). And I explicitly mentioned the ones I couldn't test in the cover letter. But I'm afraid this particular change that Masahiro pointed must have been overlooked by me (and possibly by others involved in the process). Being the author of the patch I accept my mistake. Because this construct was new to me I read about it thoroughly in the docs. As soon as it was pointed out to me, I at once realised that the change proposed by me was wrong (i didn't have to look at the docs). > >> That my proposed change: >> $(shell $(src...)-> $($(CONFIG_SHELL) $(src...) >> >> is WRONG > > Yup, as it's in a Makefile and that's a Makefile construct> >> and in the next line he suggested the required correction. >> That being: >> $($(CONFIG_SHELL)-> $(shell $(CONFIG_SHELL) > > Such stuff should generally not be needed as the to-be-used > shell can be set in Makefiles via a "SHELL = " assignment It's not about setting shell but rather using it at required place. The 'shell function' is meant to execute provided commands in an environment outside of make; and executing commands in that environment is somewhat similar to running commands on a terminal. Invoking a script file without setting the x bits will give a permission denied error. Similar thing happens when 'shell function' tries to invoke the provided script. So the task was simply to prepend the $CONFIG_SHELL (or $SHELL whichever is configured; simple sh would also suffice) with the script file in 'shell function'. > (defaulting to /bin/sh - what else;-). > Flags for the shell can BTW set with ".SHELLFLAGS = ". setting flags might not be the solution either. > > So please > -) learn basic "Makefile" + "make" before brainlessly patching >a Makefile. > -) actually testy your changes to make sure the patch didn't >broke anything > -) and - last but not least - check if there isn't a shell >already set (and which). btw, I do agree with your points. > > MfG, > Bernd > If I said anything incorrect please correct me. Thanks Ujjwal Kumar
Re: [PATCH v2 2/2] kbuild: use interpreters to invoke scripts
On 12/10/20 11:50 pm, Lukas Bulwahn wrote: > > > On Mon, 12 Oct 2020, Ujjwal Kumar wrote: > >> We cannot rely on execute bits to be set on files in the repository. >> The build script should use the explicit interpreter when invoking any >> script from the repository. >> >> Link: >> https://lore.kernel.org/lkml/20200830174409.c24c3f67addcce0cea9a9...@linux-foundation.org/ >> Link: https://lore.kernel.org/lkml/202008271102.FEB906C88@keescook/ >> >> Suggested-by: Andrew Morton >> Suggested-by: Kees Cook >> Suggested-by: Lukas Bulwahn >> Signed-off-by: Ujjwal Kumar >> --- >> Makefile | 4 ++-- >> arch/arm64/kernel/vdso/Makefile | 2 +- >> arch/arm64/kernel/vdso32/Makefile | 2 +- >> arch/ia64/Makefile| 4 ++-- >> arch/nds32/kernel/vdso/Makefile | 2 +- >> scripts/Makefile.build| 2 +- >> scripts/Makefile.package | 4 ++-- >> 7 files changed, 10 insertions(+), 10 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index 0af7945caa61..df20e71dd7c8 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -1256,7 +1256,7 @@ include/generated/utsrelease.h: >> include/config/kernel.release FORCE >> PHONY += headerdep >> headerdep: >> $(Q)find $(srctree)/include/ -name '*.h' | xargs --max-args 1 \ >> -$(srctree)/scripts/headerdep.pl -I$(srctree)/include >> +$(PERL) $(srctree)/scripts/headerdep.pl -I$(srctree)/include >> >> # >> --- >> # Kernel headers >> @@ -1312,7 +1312,7 @@ PHONY += kselftest-merge >> kselftest-merge: >> $(if $(wildcard $(objtree)/.config),, $(error No .config exists, config >> your kernel first!)) >> $(Q)find $(srctree)/tools/testing/selftests -name config | \ >> -xargs $(srctree)/scripts/kconfig/merge_config.sh -m >> $(objtree)/.config >> +xargs $(CONFIG_SHELL) >> $(srctree)/scripts/kconfig/merge_config.sh -m $(objtree)/.config >> $(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig >> >> # >> --- >> diff --git a/arch/arm64/kernel/vdso/Makefile >> b/arch/arm64/kernel/vdso/Makefile >> index edccdb77c53e..fb07804b7fc1 100644 >> --- a/arch/arm64/kernel/vdso/Makefile >> +++ b/arch/arm64/kernel/vdso/Makefile >> @@ -65,7 +65,7 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE >> # Generate VDSO offsets using helper script >> gen-vdsosym := $(srctree)/$(src)/gen_vdso_offsets.sh >> quiet_cmd_vdsosym = VDSOSYM $@ >> - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ >> + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C >> sort > $@ >> >> include/generated/vdso-offsets.h: $(obj)/vdso.so.dbg FORCE >> $(call if_changed,vdsosym) >> diff --git a/arch/arm64/kernel/vdso32/Makefile >> b/arch/arm64/kernel/vdso32/Makefile >> index 7f96a1a9f68c..617c9ac58156 100644 >> --- a/arch/arm64/kernel/vdso32/Makefile >> +++ b/arch/arm64/kernel/vdso32/Makefile >> @@ -205,7 +205,7 @@ quiet_cmd_vdsomunge = MUNGE $@ >> gen-vdsosym := $(srctree)/$(src)/../vdso/gen_vdso_offsets.sh >> quiet_cmd_vdsosym = VDSOSYM $@ >> # The AArch64 nm should be able to read an AArch32 binary >> - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ >> + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C >> sort > $@ >> >> # Install commands for the unstripped file >> quiet_cmd_vdso_install = INSTALL32 $@ >> diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile >> index 703b1c4f6d12..86d42a2d09cb 100644 >> --- a/arch/ia64/Makefile >> +++ b/arch/ia64/Makefile >> @@ -27,8 +27,8 @@ cflags-y := -pipe $(EXTRA) -ffixed-r13 >> -mfixed-range=f12-f15,f32-f127 \ >> -falign-functions=32 -frename-registers >> -fno-optimize-sibling-calls >> KBUILD_CFLAGS_KERNEL := -mconstant-gp >> >> -GAS_STATUS = $(shell $(srctree)/arch/ia64/scripts/check-gas "$(CC)" >> "$(OBJDUMP)") >> -KBUILD_CPPFLAGS += $(shell $(srctree)/arch/ia64/scripts/toolchain-flags >> "$(CC)" "$(OBJDUMP)" "$(READELF)") >> +GAS_STATUS = $(shell $(CONFIG_SHELL) >> $(srctree)/arch/ia64/scripts/check-gas "$(CC)" "$(OBJDUMP)") >> +KBUILD_CPPFLAGS += $(shell $(CONFIG_SHELL) >> $(srctree)/arch/ia64/scripts/toolchain-flags "$(CC)" &q
[PATCH v2 2/2] kbuild: use interpreters to invoke scripts
We cannot rely on execute bits to be set on files in the repository. The build script should use the explicit interpreter when invoking any script from the repository. Link: https://lore.kernel.org/lkml/20200830174409.c24c3f67addcce0cea9a9...@linux-foundation.org/ Link: https://lore.kernel.org/lkml/202008271102.FEB906C88@keescook/ Suggested-by: Andrew Morton Suggested-by: Kees Cook Suggested-by: Lukas Bulwahn Signed-off-by: Ujjwal Kumar --- Makefile | 4 ++-- arch/arm64/kernel/vdso/Makefile | 2 +- arch/arm64/kernel/vdso32/Makefile | 2 +- arch/ia64/Makefile| 4 ++-- arch/nds32/kernel/vdso/Makefile | 2 +- scripts/Makefile.build| 2 +- scripts/Makefile.package | 4 ++-- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index 0af7945caa61..df20e71dd7c8 100644 --- a/Makefile +++ b/Makefile @@ -1256,7 +1256,7 @@ include/generated/utsrelease.h: include/config/kernel.release FORCE PHONY += headerdep headerdep: $(Q)find $(srctree)/include/ -name '*.h' | xargs --max-args 1 \ - $(srctree)/scripts/headerdep.pl -I$(srctree)/include + $(PERL) $(srctree)/scripts/headerdep.pl -I$(srctree)/include # --- # Kernel headers @@ -1312,7 +1312,7 @@ PHONY += kselftest-merge kselftest-merge: $(if $(wildcard $(objtree)/.config),, $(error No .config exists, config your kernel first!)) $(Q)find $(srctree)/tools/testing/selftests -name config | \ - xargs $(srctree)/scripts/kconfig/merge_config.sh -m $(objtree)/.config + xargs $(CONFIG_SHELL) $(srctree)/scripts/kconfig/merge_config.sh -m $(objtree)/.config $(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig # --- diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile index edccdb77c53e..fb07804b7fc1 100644 --- a/arch/arm64/kernel/vdso/Makefile +++ b/arch/arm64/kernel/vdso/Makefile @@ -65,7 +65,7 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE # Generate VDSO offsets using helper script gen-vdsosym := $(srctree)/$(src)/gen_vdso_offsets.sh quiet_cmd_vdsosym = VDSOSYM $@ - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C sort > $@ include/generated/vdso-offsets.h: $(obj)/vdso.so.dbg FORCE $(call if_changed,vdsosym) diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index 7f96a1a9f68c..617c9ac58156 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -205,7 +205,7 @@ quiet_cmd_vdsomunge = MUNGE $@ gen-vdsosym := $(srctree)/$(src)/../vdso/gen_vdso_offsets.sh quiet_cmd_vdsosym = VDSOSYM $@ # The AArch64 nm should be able to read an AArch32 binary - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C sort > $@ # Install commands for the unstripped file quiet_cmd_vdso_install = INSTALL32 $@ diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile index 703b1c4f6d12..86d42a2d09cb 100644 --- a/arch/ia64/Makefile +++ b/arch/ia64/Makefile @@ -27,8 +27,8 @@ cflags-y := -pipe $(EXTRA) -ffixed-r13 -mfixed-range=f12-f15,f32-f127 \ -falign-functions=32 -frename-registers -fno-optimize-sibling-calls KBUILD_CFLAGS_KERNEL := -mconstant-gp -GAS_STATUS = $(shell $(srctree)/arch/ia64/scripts/check-gas "$(CC)" "$(OBJDUMP)") -KBUILD_CPPFLAGS += $(shell $(srctree)/arch/ia64/scripts/toolchain-flags "$(CC)" "$(OBJDUMP)" "$(READELF)") +GAS_STATUS = $(shell $(CONFIG_SHELL) $(srctree)/arch/ia64/scripts/check-gas "$(CC)" "$(OBJDUMP)") +KBUILD_CPPFLAGS += $(shell $(CONFIG_SHELL) $(srctree)/arch/ia64/scripts/toolchain-flags "$(CC)" "$(OBJDUMP)" "$(READELF)") ifeq ($(GAS_STATUS),buggy) $(error Sorry, you need a newer version of the assember, one that is built from\ diff --git a/arch/nds32/kernel/vdso/Makefile b/arch/nds32/kernel/vdso/Makefile index 55df25ef0057..e77d4bcfa7c1 100644 --- a/arch/nds32/kernel/vdso/Makefile +++ b/arch/nds32/kernel/vdso/Makefile @@ -39,7 +39,7 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE # Generate VDSO offsets using helper script gen-vdsosym := $(srctree)/$(src)/gen_vdso_offsets.sh quiet_cmd_vdsosym = VDSOSYM $@ - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C sort > $@ include/generated/vdso-offsets.h: $(obj)/vdso.so.dbg FORCE $(call if_changed,vdsosym) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index a467b9323442..893217ee4a17 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefil
[PATCH v2 1/2] kconfig: use interpreters to invoke scripts
We cannot rely on execute bits to be set on files in the repository. The build script should use the explicit interpreter when invoking any script from the repository. Link: https://lore.kernel.org/lkml/20200830174409.c24c3f67addcce0cea9a9...@linux-foundation.org/ Link: https://lore.kernel.org/lkml/202008271102.FEB906C88@keescook/ Suggested-by: Andrew Morton Suggested-by: Kees Cook Suggested-by: Lukas Bulwahn Signed-off-by: Ujjwal Kumar --- init/Kconfig | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/init/Kconfig b/init/Kconfig index c9446911cf41..8adf3194d26f 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -30,12 +30,12 @@ config CC_IS_GCC config GCC_VERSION int - default $(shell,$(srctree)/scripts/gcc-version.sh $(CC)) if CC_IS_GCC + default $(shell,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.sh $(CC)) if CC_IS_GCC default 0 config LD_VERSION int - default $(shell,$(LD) --version | $(srctree)/scripts/ld-version.sh) + default $(shell,$(LD) --version | $(AWK) -f $(srctree)/scripts/ld-version.sh) config CC_IS_CLANG def_bool $(success,echo "$(CC_VERSION_TEXT)" | grep -q clang) @@ -45,20 +45,20 @@ config LD_IS_LLD config CLANG_VERSION int - default $(shell,$(srctree)/scripts/clang-version.sh $(CC)) + default $(shell,$(CONFIG_SHELL) $(srctree)/scripts/clang-version.sh $(CC)) config CC_CAN_LINK bool - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag)) if 64BIT - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag)) + default $(success,$(CONFIG_SHELL) $(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag)) if 64BIT + default $(success,$(CONFIG_SHELL) $(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag)) config CC_CAN_LINK_STATIC bool - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag) -static) if 64BIT - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag) -static) + default $(success,$(CONFIG_SHELL) $(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag) -static) if 64BIT + default $(success,$(CONFIG_SHELL) $(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag) -static) config CC_HAS_ASM_GOTO - def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC)) + def_bool $(success,$(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC)) config CC_HAS_ASM_GOTO_OUTPUT depends on CC_HAS_ASM_GOTO -- 2.25.1
[PATCH v2 0/2] use interpreters to invoke scripts
This patch series aims at removing the dependency on execute bit of the scripts in the kbuild system. If not working with fresh clone of linux-next, clean the srctree: make distclean make tools/clean To test the dependency on execute bits, I tried building the kernel after removing x-bits for all files in the repository. Removing execute bits: for i in $(find -executable -type f); do chmod -x $i; done Any attempts to configure (or build) the kernel fail because of 'Permission denied' on scripts with the following error: $ make allmodconfig sh: ./scripts/gcc-version.sh: Permission denied init/Kconfig:34: syntax error init/Kconfig:33: invalid statement init/Kconfig:34: invalid statement sh: ./scripts/ld-version.sh: Permission denied init/Kconfig:39: syntax error init/Kconfig:38: invalid statement sh: ./scripts/clang-version.sh: Permission denied init/Kconfig:49: syntax error init/Kconfig:48: invalid statement make[1]: *** [scripts/kconfig/Makefile:71: allmodconfig] Error 1 make: *** [Makefile:606: allmodconfig] Error 2 Changes: - Adds specific interpreters (in Kconfig) to invoke scripts. After this patch I could successfully do a kernel build without any errors. - Again, adds specific interpreters to other parts of kbuild system. I could successfully perform the following make targets after applying the PATCH 2/2: make headerdep make kselftest-merge make rpm-pkg make perf-tar-src-pkg make ARCH=ia64 defconfig ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make prepare Following changes in PATCH 2/2 are not yet tested: arch/arm64/kernel/vdso32/Makefile arch/nds32/kernel/vdso/Makefile scripts/Makefile.build --- Changes in v2: - Changes suggested by Masahiro Yamada $($(CONFIG_SHELL)-> $(shell $(CONFIG_SHELL) Ujjwal Kumar (2): kconfig: use interpreters to invoke scripts kbuild: use interpreters to invoke scripts Makefile | 4 ++-- arch/arm64/kernel/vdso/Makefile | 2 +- arch/arm64/kernel/vdso32/Makefile | 2 +- arch/ia64/Makefile| 4 ++-- arch/nds32/kernel/vdso/Makefile | 2 +- init/Kconfig | 16 scripts/Makefile.build| 2 +- scripts/Makefile.package | 4 ++-- 8 files changed, 18 insertions(+), 18 deletions(-) base-commit: 2cab4ac556258c14f6ec8d2157654e95556bbb31 -- 2.25.1
Re: [PATCH RFC 0/2] use interpreters to invoke scripts
On 12/10/20 9:48 pm, Masahiro Yamada wrote: > On Sun, Oct 4, 2020 at 12:19 AM Ujjwal Kumar > wrote: >> >> This patch series aims at removing the dependency on execute >> bit of the scripts in the kbuild system. >> >> If not working with fresh clone of linux-next, clean the srctree: >> make distclean >> make tools/clean >> >> To test the dependency on execute bits, I tried building the >> kernel after removing x-bits for all files in the repository. >> Removing execute bits: >> for i in $(find -executable -type f); do chmod -x $i; done >> >> Any attempts to configure (or build) the kernel fail because of >> 'Permission denied' on scripts with the following error: >> $ make allmodconfig >> sh: ./scripts/gcc-version.sh: Permission denied >> init/Kconfig:34: syntax error >> init/Kconfig:33: invalid statement >> init/Kconfig:34: invalid statement >> sh: ./scripts/ld-version.sh: Permission denied >> init/Kconfig:39: syntax error >> init/Kconfig:38: invalid statement >> sh: ./scripts/clang-version.sh: Permission denied >> init/Kconfig:49: syntax error >> init/Kconfig:48: invalid statement >> make[1]: *** [scripts/kconfig/Makefile:71: allmodconfig] Error 1 >> make: *** [Makefile:606: allmodconfig] Error 2 >> >> Changes: >> 1. Adds specific interpreters (in Kconfig) to invoke >> scripts. >> >> After this patch I could successfully do a kernel build >> without any errors. >> >> 2. Again, adds specific interpreters to other parts of >> kbuild system. >> >> I could successfully perform the following make targets after >> applying the PATCH 2/2: >> make headerdep >> make kselftest-merge >> make rpm-pkg >> make perf-tar-src-pkg >> make ARCH=ia64 defconfig >> ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make prepare >> >> Following changes in PATCH 2/2 are not yet tested: >> arch/arm64/kernel/vdso32/Makefile >> arch/nds32/kernel/vdso/Makefile >> scripts/Makefile.build >> >> Ujjwal Kumar (2): >> kconfig: use interpreters to invoke scripts >> kbuild: use interpreters to invoke scripts >> >> Makefile | 4 ++-- >> arch/arm64/kernel/vdso/Makefile | 2 +- >> arch/arm64/kernel/vdso32/Makefile | 2 +- >> arch/ia64/Makefile| 4 ++-- >> arch/nds32/kernel/vdso/Makefile | 2 +- >> init/Kconfig | 16 >> scripts/Makefile.build| 2 +- >> scripts/Makefile.package | 4 ++-- >> 8 files changed, 18 insertions(+), 18 deletions(-) >> >> -- >> 2.26.2 >> > > > Andrew Morton suggested and applied the doc patch > (commit e9aae7af4601688386 in linux-next), > but did not pick up this series. > > It is difficult to predict which patch he would > pick up, and which he would not. > > > I can apply this series > together with Lukas' base patch. > > > I pointed out possible mistakes in 2/2. > I can locally fix them up if you agree. I agree with the changes you pointed out. I was in the process of sending a V2 patch series (almost done). But if you prefer on locally fixing them, that is completely fine. > > > BTW, Kees Cook suggested dropping the x bit > from all scripts, but I did not agree with that part. IIRC, in the discussion Kees Cook suggestion was not to drop x bit but rather he meant to use that as a trick to catch any existing dependency on x bit. > > > In the doc change, Lukas mentioned > "further clean-up patches", but I hope > it does not mean dropping the x bits. IMO, he did not mean to drop the x bits. But rather I have many more small changes similar to these. He must be referring to these two patches and any future patches around this issue. > > > -- > Best Regards > > Masahiro Yamada > Thanks Ujjwal Kumar
[PATCH] checkpatch: improve EXECUTE_PERMISSIONS tests
1. Use group capture regexp for file mode test to improve code
readability.
2. The 'scripts/' directory test on filenames can be excluded
as it has become obsolete because there are many source
files that are not scripts in this directory and its
subdirectories.
3. Replace unnecessary group capture regexp with non-capturing
group.
Suggested-by: Joe Perches
Signed-off-by: Ujjwal Kumar
---
scripts/checkpatch.pl | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..aa8417b5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2678,10 +2678,11 @@ sub process {
}
# Check for incorrect file permissions
- if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) {
+ if ($line =~ /^new (?:file )?mode (\d+)$/) {
+ my $mode = substr($1, -3);
my $permhere = $here . "FILE: $realfile\n";
- if ($realfile !~ m@scripts/@ &&
- $realfile !~ /\.(py|pl|awk|sh)$/) {
+ if ($mode =~ /[1357]/ &&
+ $realfile !~ /\.(?:py|pl|awk|sh)$/) {
ERROR("EXECUTE_PERMISSIONS",
"do not set execute permissions for
source files\n" . $permhere);
}
base-commit: d67bc7812221606e1886620a357b13f906814af7
--
2.26.2
Re: [RFC PATCH] checkpatch: add shebang check to EXECUTE_PERMISSIONS
On 12/10/20 11:47 am, Joe Perches wrote:
> On Mon, 2020-10-12 at 11:19 +0530, Ujjwal Kumar wrote:
>> checkpatch.pl checks for invalid EXECUTE_PERMISSIONS on source
>> files. The script leverages filename extensions and its path in
>> the repository to decide whether to allow execute permissions on
>> the file or not.
>>
>> Based on current check conditions, a perl script file having
>> execute permissions, without '.pl' extension in its filename
>> and not belonging to 'scripts/' directory is reported as ERROR
>> which is a false-positive.
>>
>> Adding a shebang check along with current conditions will make
>> the check more generalised and improve checkpatch reports.
>> To do so, without breaking the core design decision of checkpatch,
>> we can fetch the first line from the patch itself and match it for
>> a shebang pattern.
>>
>> There can be cases where the first line is not part of the patch.
>
> For instance: a patch that only changes permissions
> without changing any of the file content.
>
>>
>> In that case there may be a false-positive report but in the end we
>> will have less false-positives as we will be handling some of the
>> unhandled cases.
>
>> Signed-off-by: Ujjwal Kumar
>> ---
>> Apologies, I forgot to include linux-kernel@vger.kernel.org so I'm
>> now resending.
>>
>> scripts/checkpatch.pl | 19 +++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>> @@ -1795,6 +1795,23 @@ sub get_stat_here {
>> return $herectx;
>> }
>
> First some style trivia:
>
>> +sub get_shebang {
>> +my ($linenr, $realfile) = @_;
>> +my $rawline = "";
>> +my $shebang = "";
>> +
>> +$rawline = raw_line($linenr, 3);
>> +if (defined $rawline &&
>> +$rawline =~ /^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
>
> alignment to open parenthesis please
>
>> +if (defined $1 && $1 == 1) {
>> +$shebang = raw_line($linenr, 4);
>> +$shebang = substr $shebang, 1;
>
> parentheses around substr please.
>
>> +}
>> +}
>> +
>> +return $shebang;
>> +}
>
> And some real notes:
>
> $realfile isn't used in this function so there doesn't
> seem to be a reason to have it as an function argument.
>
>> +
>> sub cat_vet {
>> my ($vet) = @_;
>> my ($res, $coded);
>> @@ -2680,7 +2697,9 @@ sub process {
>> # Check for incorrect file permissions
>> if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) {
>
> probably better here to use a capture group for the permissions
>
> if ($line =~ /^new (?:file )?mode (\d+)$/) {
> my $mode = substr($1, -3);
This
>
>> my $permhere = $here . "FILE: $realfile\n";
>> +my $shebang = get_shebang($linenr, $realfile);
>> if ($realfile !~ m@scripts/@ &&
>
> Maybe remove the $realfile directory test as
> there are many source files that are not scripts
> in this directory and its subdirectories.
this
>
>> +$shebang !~ /^#!\s*(\/\w)+.*/ &&
>
> unnecessary capture group
>
> and add
>
> $mode =~ /[1357]/ &&
this
>
>> $realfile !~ /\.(py|pl|awk|sh)$/) {
>
> No need for a a capture group here either. (existing defect)
and this.
>
>> ERROR("EXECUTE_PERMISSIONS",
>>"do not set execute permissions for
>> source files\n" . $permhere);
>
>
>
Should these new changes go as a separate patch or can they be
included in the next iteration of this patch?
Thanks
Ujjwal Kumar
[RFC PATCH] checkpatch: add shebang check to EXECUTE_PERMISSIONS
checkpatch.pl checks for invalid EXECUTE_PERMISSIONS on source
files. The script leverages filename extensions and its path in
the repository to decide whether to allow execute permissions on
the file or not.
Based on current check conditions, a perl script file having
execute permissions, without '.pl' extension in its filename
and not belonging to 'scripts/' directory is reported as ERROR
which is a false-positive.
Adding a shebang check along with current conditions will make
the check more generalised and improve checkpatch reports.
To do so, without breaking the core design decision of checkpatch,
we can fetch the first line from the patch itself and match it for
a shebang pattern.
There can be cases where the first line is not part of the patch.
In that case there may be a false-positive report but in the end we
will have less false-positives as we will be handling some of the
unhandled cases.
Signed-off-by: Ujjwal Kumar
---
Apologies, I forgot to include linux-kernel@vger.kernel.org so I'm
now resending.
scripts/checkpatch.pl | 19 +++
1 file changed, 19 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..e596d30794bf 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1795,6 +1795,23 @@ sub get_stat_here {
return $herectx;
}
+sub get_shebang {
+ my ($linenr, $realfile) = @_;
+ my $rawline = "";
+ my $shebang = "";
+
+ $rawline = raw_line($linenr, 3);
+ if (defined $rawline &&
+ $rawline =~ /^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
+ if (defined $1 && $1 == 1) {
+ $shebang = raw_line($linenr, 4);
+ $shebang = substr $shebang, 1;
+ }
+ }
+
+ return $shebang;
+}
+
sub cat_vet {
my ($vet) = @_;
my ($res, $coded);
@@ -2680,7 +2697,9 @@ sub process {
# Check for incorrect file permissions
if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) {
my $permhere = $here . "FILE: $realfile\n";
+ my $shebang = get_shebang($linenr, $realfile);
if ($realfile !~ m@scripts/@ &&
+ $shebang !~ /^#!\s*(\/\w)+.*/ &&
$realfile !~ /\.(py|pl|awk|sh)$/) {
ERROR("EXECUTE_PERMISSIONS",
"do not set execute permissions for
source files\n" . $permhere);
base-commit: d67bc7812221606e1886620a357b13f906814af7
--
2.26.2
[PATCH RFC 2/2] kbuild: use interpreters to invoke scripts
We cannot rely on execute bits to be set on files in the repository. The build script should use the explicit interpreter when invoking any script from the repository. Link: https://lore.kernel.org/lkml/20200830174409.c24c3f67addcce0cea9a9...@linux-foundation.org/ Link: https://lore.kernel.org/lkml/202008271102.FEB906C88@keescook/ Suggested-by: Andrew Morton Suggested-by: Kees Cook Suggested-by: Lukas Bulwahn Signed-off-by: Ujjwal Kumar --- Makefile | 4 ++-- arch/arm64/kernel/vdso/Makefile | 2 +- arch/arm64/kernel/vdso32/Makefile | 2 +- arch/ia64/Makefile| 4 ++-- arch/nds32/kernel/vdso/Makefile | 2 +- scripts/Makefile.build| 2 +- scripts/Makefile.package | 4 ++-- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index f93dbae71248..5f1399a576d4 100644 --- a/Makefile +++ b/Makefile @@ -1258,7 +1258,7 @@ include/generated/utsrelease.h: include/config/kernel.release FORCE PHONY += headerdep headerdep: $(Q)find $(srctree)/include/ -name '*.h' | xargs --max-args 1 \ - $(srctree)/scripts/headerdep.pl -I$(srctree)/include + $(PERL) $(srctree)/scripts/headerdep.pl -I$(srctree)/include # --- # Kernel headers @@ -1314,7 +1314,7 @@ PHONY += kselftest-merge kselftest-merge: $(if $(wildcard $(objtree)/.config),, $(error No .config exists, config your kernel first!)) $(Q)find $(srctree)/tools/testing/selftests -name config | \ - xargs $(srctree)/scripts/kconfig/merge_config.sh -m $(objtree)/.config + xargs $(CONFIG_SHELL) $(srctree)/scripts/kconfig/merge_config.sh -m $(objtree)/.config $(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig # --- diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile index de981f7b4546..30fe93bb5488 100644 --- a/arch/arm64/kernel/vdso/Makefile +++ b/arch/arm64/kernel/vdso/Makefile @@ -65,7 +65,7 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE # Generate VDSO offsets using helper script gen-vdsosym := $(srctree)/$(src)/gen_vdso_offsets.sh quiet_cmd_vdsosym = VDSOSYM $@ - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C sort > $@ include/generated/vdso-offsets.h: $(obj)/vdso.so.dbg FORCE $(call if_changed,vdsosym) diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index 572475b7b7ed..4f8fe34bc75a 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -205,7 +205,7 @@ quiet_cmd_vdsomunge = MUNGE $@ gen-vdsosym := $(srctree)/$(src)/../vdso/gen_vdso_offsets.sh quiet_cmd_vdsosym = VDSOSYM $@ # The AArch64 nm should be able to read an AArch32 binary - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C sort > $@ # Install commands for the unstripped file quiet_cmd_vdso_install = INSTALL32 $@ diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile index 2876a7df1b0a..5f6cc3c3da50 100644 --- a/arch/ia64/Makefile +++ b/arch/ia64/Makefile @@ -28,8 +28,8 @@ cflags-y := -pipe $(EXTRA) -ffixed-r13 -mfixed-range=f12-f15,f32-f127 \ -falign-functions=32 -frename-registers -fno-optimize-sibling-calls KBUILD_CFLAGS_KERNEL := -mconstant-gp -GAS_STATUS = $(shell $(srctree)/arch/ia64/scripts/check-gas "$(CC)" "$(OBJDUMP)") -KBUILD_CPPFLAGS += $(shell $(srctree)/arch/ia64/scripts/toolchain-flags "$(CC)" "$(OBJDUMP)" "$(READELF)") +GAS_STATUS = $($(CONFIG_SHELL) $(srctree)/arch/ia64/scripts/check-gas "$(CC)" "$(OBJDUMP)") +KBUILD_CPPFLAGS += $($(CONFIG_SHELL) $(srctree)/arch/ia64/scripts/toolchain-flags "$(CC)" "$(OBJDUMP)" "$(READELF)") ifeq ($(GAS_STATUS),buggy) $(error Sorry, you need a newer version of the assember, one that is built from\ diff --git a/arch/nds32/kernel/vdso/Makefile b/arch/nds32/kernel/vdso/Makefile index 55df25ef0057..e77d4bcfa7c1 100644 --- a/arch/nds32/kernel/vdso/Makefile +++ b/arch/nds32/kernel/vdso/Makefile @@ -39,7 +39,7 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE # Generate VDSO offsets using helper script gen-vdsosym := $(srctree)/$(src)/gen_vdso_offsets.sh quiet_cmd_vdsosym = VDSOSYM $@ - cmd_vdsosym = $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ + cmd_vdsosym = $(NM) $< | $(CONFIG_SHELL) $(gen-vdsosym) | LC_ALL=C sort > $@ include/generated/vdso-offsets.h: $(obj)/vdso.so.dbg FORCE $(call if_changed,vdsosym) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index a467b9323442..893217ee4a17 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile
[PATCH RFC 1/2] kconfig: use interpreters to invoke scripts
We cannot rely on execute bits to be set on files in the repository. The build script should use the explicit interpreter when invoking any script from the repository. Link: https://lore.kernel.org/lkml/20200830174409.c24c3f67addcce0cea9a9...@linux-foundation.org/ Link: https://lore.kernel.org/lkml/202008271102.FEB906C88@keescook/ Suggested-by: Andrew Morton Suggested-by: Kees Cook Suggested-by: Lukas Bulwahn Signed-off-by: Ujjwal Kumar --- init/Kconfig | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/init/Kconfig b/init/Kconfig index 91456ac0ef20..524f6b555945 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -30,12 +30,12 @@ config CC_IS_GCC config GCC_VERSION int - default $(shell,$(srctree)/scripts/gcc-version.sh $(CC)) if CC_IS_GCC + default $(shell,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.sh $(CC)) if CC_IS_GCC default 0 config LD_VERSION int - default $(shell,$(LD) --version | $(srctree)/scripts/ld-version.sh) + default $(shell,$(LD) --version | $(AWK) -f $(srctree)/scripts/ld-version.sh) config CC_IS_CLANG def_bool $(success,echo "$(CC_VERSION_TEXT)" | grep -q clang) @@ -45,20 +45,20 @@ config LD_IS_LLD config CLANG_VERSION int - default $(shell,$(srctree)/scripts/clang-version.sh $(CC)) + default $(shell,$(CONFIG_SHELL) $(srctree)/scripts/clang-version.sh $(CC)) config CC_CAN_LINK bool - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag)) if 64BIT - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag)) + default $(success,$(CONFIG_SHELL) $(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag)) if 64BIT + default $(success,$(CONFIG_SHELL) $(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag)) config CC_CAN_LINK_STATIC bool - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag) -static) if 64BIT - default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag) -static) + default $(success,$(CONFIG_SHELL) $(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m64-flag) -static) if 64BIT + default $(success,$(CONFIG_SHELL) $(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(m32-flag) -static) config CC_HAS_ASM_GOTO - def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC)) + def_bool $(success,$(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC)) config CC_HAS_ASM_GOTO_OUTPUT depends on CC_HAS_ASM_GOTO -- 2.26.2
[PATCH RFC 0/2] use interpreters to invoke scripts
This patch series aims at removing the dependency on execute bit of the scripts in the kbuild system. If not working with fresh clone of linux-next, clean the srctree: make distclean make tools/clean To test the dependency on execute bits, I tried building the kernel after removing x-bits for all files in the repository. Removing execute bits: for i in $(find -executable -type f); do chmod -x $i; done Any attempts to configure (or build) the kernel fail because of 'Permission denied' on scripts with the following error: $ make allmodconfig sh: ./scripts/gcc-version.sh: Permission denied init/Kconfig:34: syntax error init/Kconfig:33: invalid statement init/Kconfig:34: invalid statement sh: ./scripts/ld-version.sh: Permission denied init/Kconfig:39: syntax error init/Kconfig:38: invalid statement sh: ./scripts/clang-version.sh: Permission denied init/Kconfig:49: syntax error init/Kconfig:48: invalid statement make[1]: *** [scripts/kconfig/Makefile:71: allmodconfig] Error 1 make: *** [Makefile:606: allmodconfig] Error 2 Changes: 1. Adds specific interpreters (in Kconfig) to invoke scripts. After this patch I could successfully do a kernel build without any errors. 2. Again, adds specific interpreters to other parts of kbuild system. I could successfully perform the following make targets after applying the PATCH 2/2: make headerdep make kselftest-merge make rpm-pkg make perf-tar-src-pkg make ARCH=ia64 defconfig ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make prepare Following changes in PATCH 2/2 are not yet tested: arch/arm64/kernel/vdso32/Makefile arch/nds32/kernel/vdso/Makefile scripts/Makefile.build Ujjwal Kumar (2): kconfig: use interpreters to invoke scripts kbuild: use interpreters to invoke scripts Makefile | 4 ++-- arch/arm64/kernel/vdso/Makefile | 2 +- arch/arm64/kernel/vdso32/Makefile | 2 +- arch/ia64/Makefile| 4 ++-- arch/nds32/kernel/vdso/Makefile | 2 +- init/Kconfig | 16 scripts/Makefile.build| 2 +- scripts/Makefile.package | 4 ++-- 8 files changed, 18 insertions(+), 18 deletions(-) -- 2.26.2
