scripts: Add intended executable mode and SPDX license

PATCH In review Mrinal Pandey 8 participants 14 messages Aug 27, 2020 – Sep 2, 2020

Subsystems (3)

Licenses and spdx stuff Gcc plugins Nsdeps

lore

Share & tools

lore

Mrinal Pandey

author

2020-08-27 09:24 UTC #0 lore raw

PATCH 1/1 scripts: Add intended executable mode and SPDX license

commit b72231eb7084 ("scripts: add spdxcheck.py self test") added the file spdxcheck-test.sh to the repository without the executable flag and license information.

commit eb8305aecb95 ("scripts: Coccinelle script for namespace dependencies.") added the file nsdeps, commit 313dd1b62921 ("gcc-plugins:
Add the randstruct plugin") added the file gcc-plugins/gen-random-seed.sh and commit 9b4ade226f74 ("xen: build infrastructure for generating hypercall depending symbols") added the file xen-hypercalls.sh without the executable bit.

Set to usual modes for these files and provide the SPDX license for spdxcheck-test.sh. No functional changes.

Signed-off-by Mrinal Pandey <[email protected]>

---
applies cleanly on next-20200827

Kees, Matthias, Thomas, please ack this patch.

Andrew, please pick this minor non-urgent cleanup patch once the mainainers ack.

 scripts/gcc-plugins/gen-random-seed.sh | 0
 scripts/nsdeps                         | 0
 scripts/spdxcheck-test.sh              | 1 +
 scripts/xen-hypercalls.sh              | 0
 4 files changed, 1 insertion(+)
 mode change 100644 => 100755 scripts/gcc-plugins/gen-random-seed.sh
 mode change 100644 => 100755 scripts/nsdeps
 mode change 100644 => 100755 scripts/spdxcheck-test.sh
 mode change 100644 => 100755 scripts/xen-hypercalls.sh

scripts/gcc-plugins/gen-random-seed.sh +0 -0

old mode 100644
new mode 100755

scripts/nsdeps +0 -0

old mode 100644
new mode 100755

scripts/spdxcheck-test.sh +1 -0

old mode 100644
new mode 100755
index cfea6a0d1cc0..e2902520a081
@@ -1,4 +1,5 @@
 #!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
 
 for PYTHON in python2 python3; do
 	# run check on a text and a binary file

scripts/xen-hypercalls.sh +0 -0

old mode 100644
new mode 100755

Signature · 1 line

2.25.1

Lukas Bulwahn

Acked-by on v1 1/1

2020-08-27 09:34 UTC #1 lore raw

Quoted 11 lines · Mrinal Pandey

commit b72231eb7084 ("scripts: add spdxcheck.py self test") added the file spdxcheck-test.sh to the repository without the executable flag and license information.
commit eb8305aecb95 ("scripts: Coccinelle script for namespace dependencies.") added the file nsdeps, commit 313dd1b62921 ("gcc-plugins:
Add the randstruct plugin") added the file gcc-plugins/gen-random-seed.sh and commit 9b4ade226f74 ("xen: build infrastructure for generating hypercall depending symbols") added the file xen-hypercalls.sh without the executable bit.

Quoted 4 lines

Set to usual modes for these files and provide the SPDX license for spdxcheck-test.sh. No functional changes.
Signed-off-by Mrinal Pandey <[email protected]>

This is a contribution from a candidate of the Linux Kernel Onboarding Mentorship. If successful, the candidate will work on improving checkpatch.pl so that we get closer to a state where we can run it as a bot reporting on patch submissions to mailing lists.

This was clean-up work we came across looking at SPDX license warnings, script shebangs, and executable modes of files.

Acked-by Lukas Bulwahn <[email protected]>

Quoted 43 lines

---
applies cleanly on next-20200827

Kees, Matthias, Thomas, please ack this patch.

Andrew, please pick this minor non-urgent cleanup patch once the mainainers ack.

 scripts/gcc-plugins/gen-random-seed.sh | 0
 scripts/nsdeps                         | 0
 scripts/spdxcheck-test.sh              | 1 +
 scripts/xen-hypercalls.sh              | 0
 4 files changed, 1 insertion(+)
 mode change 100644 => 100755 scripts/gcc-plugins/gen-random-seed.sh
 mode change 100644 => 100755 scripts/nsdeps
 mode change 100644 => 100755 scripts/spdxcheck-test.sh
 mode change 100644 => 100755 scripts/xen-hypercalls.sh

scripts/gcc-plugins/gen-random-seed.sh +0 -0

old mode 100644
new mode 100755

scripts/nsdeps +0 -0

old mode 100644
new mode 100755

scripts/spdxcheck-test.sh +1 -0

old mode 100644
new mode 100755
index cfea6a0d1cc0..e2902520a081
@@ -1,4 +1,5 @@
 #!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
 
 for PYTHON in python2 python3; do
 	# run check on a text and a binary file

scripts/xen-hypercalls.sh +0 -0

old mode 100644
new mode 100755

Signature · 1 line

2.25.1

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Greg KH

maintainer

2020-08-27 09:43 UTC #2 lore raw

In reply to Mrinal Pandey

Quoted 23 lines · Mrinal Pandey

commit b72231eb7084 ("scripts: add spdxcheck.py self test") added the file spdxcheck-test.sh to the repository without the executable flag and license information.
commit eb8305aecb95 ("scripts: Coccinelle script for namespace dependencies.") added the file nsdeps, commit 313dd1b62921 ("gcc-plugins:
Add the randstruct plugin") added the file gcc-plugins/gen-random-seed.sh and commit 9b4ade226f74 ("xen: build infrastructure for generating hypercall depending symbols") added the file xen-hypercalls.sh without the executable bit.
Set to usual modes for these files and provide the SPDX license for spdxcheck-test.sh. No functional changes.
Signed-off-by Mrinal Pandey <[email protected]>
---
applies cleanly on next-20200827
Kees, Matthias, Thomas, please ack this patch.
Andrew, please pick this minor non-urgent cleanup patch once the mainainers ack.

Quoted 9 lines

 scripts/gcc-plugins/gen-random-seed.sh | 0
 scripts/nsdeps                         | 0
 scripts/spdxcheck-test.sh              | 1 +
 scripts/xen-hypercalls.sh              | 0
 4 files changed, 1 insertion(+)
 mode change 100644 => 100755 scripts/gcc-plugins/gen-random-seed.sh
 mode change 100644 => 100755 scripts/nsdeps
 mode change 100644 => 100755 scripts/spdxcheck-test.sh
 mode change 100644 => 100755 scripts/xen-hypercalls.sh

This does 2 different things in one patch, shouldn't this be 2 different patches? One to change the permissions and one to add the SPDX line?

thanks,

greg k-h

Lukas Bulwahn

2020-08-27 09:49 UTC #3 lore raw

Quoted 34 lines · Greg KH

Quoted 23 lines · Mrinal Pandey
commit b72231eb7084 ("scripts: add spdxcheck.py self test") added the file spdxcheck-test.sh to the repository without the executable flag and license information.
commit eb8305aecb95 ("scripts: Coccinelle script for namespace dependencies.") added the file nsdeps, commit 313dd1b62921 ("gcc-plugins:
Add the randstruct plugin") added the file gcc-plugins/gen-random-seed.sh and commit 9b4ade226f74 ("xen: build infrastructure for generating hypercall depending symbols") added the file xen-hypercalls.sh without the executable bit.
Set to usual modes for these files and provide the SPDX license for spdxcheck-test.sh. No functional changes.
Signed-off-by Mrinal Pandey <[email protected]>
---
applies cleanly on next-20200827
Kees, Matthias, Thomas, please ack this patch.
Andrew, please pick this minor non-urgent cleanup patch once the mainainers ack.
Quoted 9 lines
 scripts/gcc-plugins/gen-random-seed.sh | 0
 scripts/nsdeps                         | 0
 scripts/spdxcheck-test.sh              | 1 +
 scripts/xen-hypercalls.sh              | 0
 4 files changed, 1 insertion(+)
 mode change 100644 => 100755 scripts/gcc-plugins/gen-random-seed.sh
 mode change 100644 => 100755 scripts/nsdeps
 mode change 100644 => 100755 scripts/spdxcheck-test.sh
 mode change 100644 => 100755 scripts/xen-hypercalls.sh

Quoted 3 lines

This does 2 different things in one patch, shouldn't this be 2 different patches? One to change the permissions and one to add the SPDX line?

For me, this was one thing: minor cleanup; and taking one minor patch is easier than taking two, but you make the call. Then, the two or even three patches would also just travel through different trees, the spdx tree and Andrew's.

Lukas

Greg KH

maintainer

2020-08-27 09:59 UTC #4 lore raw

Quoted 38 lines · Lukas Bulwahn

Quoted 34 lines · Greg KH
Quoted 32 lines · Mrinal Pandey
commit b72231eb7084 ("scripts: add spdxcheck.py self test") added the file spdxcheck-test.sh to the repository without the executable flag and license information.
commit eb8305aecb95 ("scripts: Coccinelle script for namespace dependencies.") added the file nsdeps, commit 313dd1b62921 ("gcc-plugins:
Add the randstruct plugin") added the file gcc-plugins/gen-random-seed.sh and commit 9b4ade226f74 ("xen: build infrastructure for generating hypercall depending symbols") added the file xen-hypercalls.sh without the executable bit.
Set to usual modes for these files and provide the SPDX license for spdxcheck-test.sh. No functional changes.
Signed-off-by Mrinal Pandey <[email protected]>
---
applies cleanly on next-20200827
Kees, Matthias, Thomas, please ack this patch.
Andrew, please pick this minor non-urgent cleanup patch once the mainainers ack.
 scripts/gcc-plugins/gen-random-seed.sh | 0
 scripts/nsdeps                         | 0
 scripts/spdxcheck-test.sh              | 1 +
 scripts/xen-hypercalls.sh              | 0
 4 files changed, 1 insertion(+)
 mode change 100644 => 100755 scripts/gcc-plugins/gen-random-seed.sh
 mode change 100644 => 100755 scripts/nsdeps
 mode change 100644 => 100755 scripts/spdxcheck-test.sh
 mode change 100644 => 100755 scripts/xen-hypercalls.sh

Quoted 8 lines

Quoted 3 lines
This does 2 different things in one patch, shouldn't this be 2 different patches? One to change the permissions and one to add the SPDX line?
For me, this was one thing: minor cleanup; and taking one minor patch is easier than taking two, but you make the call. Then, the two or even three patches would also just travel through different trees, the spdx tree and Andrew's.

License "additions" are good to separate from other changes, in case people have questions/comments/objections about them, they can be easier to discuss and possibly reverted.

Solar Designer

2020-08-27 13:07 UTC #5 lore raw

In reply to Mrinal Pandey

[CC list trimmed]

Quoted 1 line · Mrinal Pandey

 mode change 100644 => 100755 scripts/gcc-plugins/gen-random-seed.sh

This is basically the only change relevant to the contribution initially made via kernel-hardening, and in my opinion (and I am list admin) isn't worth bringing to the list. Now we have this bikeshed thread in here (and I'm guilty for adding to it), and would have more (which I hope this message of mine will prevent) if changes to something else in the patch(es) are requested (which Greg KH sort of already did).

I recall we previously had lots of "similar" bikeshedding in here when someone was converting the documentation to rST. The more bikeshedding we have, the less actual kernel-hardening work is going to happen, because the list gets the reputation of yet another kernel maintenance list rather than the place where actual/potential new contributions to improve the kernel's security are discussed, and because bikeshedding makes the most capable people unsubscribe or stop paying attention.

How about we remove kernel-hardening from the MAINTAINERS entries it's currently in? -

GCC PLUGINS

M:      Kees Cook <[email protected]>
R:      Emese Revfy <[email protected]>
L:      [email protected]
S:      Maintained
F:      Documentation/kbuild/gcc-plugins.rst
F:      scripts/Makefile.gcc-plugins
F:      scripts/gcc-plugin.sh
F:      scripts/gcc-plugins/

LEAKING_ADDRESSES

M:      Tobin C. Harding <[email protected]>
M:      Tycho Andersen <[email protected]>
L:      [email protected]
S:      Maintained
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/tobin/leaks.git
F:      scripts/leaking_addresses.pl

Alternatively, would this be acceptable? -

L: [email protected] (only for messages focused on core functionality, not for maintenance detail)

I think the latter would be best, if allowed.

Kees, please comment (so that we'd hopefully not need that next time), and if you agree please make a change to MAINTAINERS.

Mrinal, we appreciate your contribution, and the problem above isn't yours - it's with the way MAINTAINERS doesn't fit this group well.

Thanks,

Alexander

Kees Cook

2020-08-27 18:02 UTC #6 lore raw

Quoted 39 lines · Solar Designer

Quoted 1 line · Mrinal Pandey
 mode change 100644 => 100755 scripts/gcc-plugins/gen-random-seed.sh
This is basically the only change relevant to the contribution initially made via kernel-hardening, and in my opinion (and I am list admin) isn't worth bringing to the list. Now we have this bikeshed thread in here (and I'm guilty for adding to it), and would have more (which I hope this message of mine will prevent) if changes to something else in the patch(es) are requested (which Greg KH sort of already did).
I recall we previously had lots of "similar" bikeshedding in here when someone was converting the documentation to rST. The more bikeshedding we have, the less actual kernel-hardening work is going to happen, because the list gets the reputation of yet another kernel maintenance list rather than the place where actual/potential new contributions to improve the kernel's security are discussed, and because bikeshedding makes the most capable people unsubscribe or stop paying attention.
How about we remove kernel-hardening from the MAINTAINERS entries it's currently in? -
GCC PLUGINS
M:      Kees Cook <[email protected]>
R:      Emese Revfy <[email protected]>
L:      [email protected]
S:      Maintained
F:      Documentation/kbuild/gcc-plugins.rst
F:      scripts/Makefile.gcc-plugins
F:      scripts/gcc-plugin.sh
F:      scripts/gcc-plugins/
LEAKING_ADDRESSES
M:      Tobin C. Harding <[email protected]>
M:      Tycho Andersen <[email protected]>
L:      [email protected]
S:      Maintained
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/tobin/leaks.git
F:      scripts/leaking_addresses.pl

Quoted 8 lines

Alternatively, would this be acceptable? -
L: [email protected] (only for messages focused on core functionality, not for maintenance detail)
I think the latter would be best, if allowed.
Kees, please comment (so that we'd hopefully not need that next time), and if you agree please make a change to MAINTAINERS.

A comment isn't going to really help fix this (much of the CCing is done by scripts, etc).

I've tended to prefer more emails than missing discussions, and I think it's not unreasonable to have the list mentioned in MAINTAINERS for those things. It does, of course, mean that "maintenance" patches get directed there too, as you say.

If it's really something you'd like to avoid, I can drop those references. My instinct is to leave it as-is, but the strength of my opinion is pretty small. Let me know what you prefer...

Signature · 1 line

Kees Cook

Tycho Andersen

2020-09-01 00:15 UTC #7 lore raw

Quoted 52 lines · Kees Cook

Quoted 39 lines · Solar Designer
Quoted 1 line · Mrinal Pandey
 mode change 100644 => 100755 scripts/gcc-plugins/gen-random-seed.sh
This is basically the only change relevant to the contribution initially made via kernel-hardening, and in my opinion (and I am list admin) isn't worth bringing to the list. Now we have this bikeshed thread in here (and I'm guilty for adding to it), and would have more (which I hope this message of mine will prevent) if changes to something else in the patch(es) are requested (which Greg KH sort of already did).
I recall we previously had lots of "similar" bikeshedding in here when someone was converting the documentation to rST. The more bikeshedding we have, the less actual kernel-hardening work is going to happen, because the list gets the reputation of yet another kernel maintenance list rather than the place where actual/potential new contributions to improve the kernel's security are discussed, and because bikeshedding makes the most capable people unsubscribe or stop paying attention.
How about we remove kernel-hardening from the MAINTAINERS entries it's currently in? -
GCC PLUGINS
M:      Kees Cook <[email protected]>
R:      Emese Revfy <[email protected]>
L:      [email protected]
S:      Maintained
F:      Documentation/kbuild/gcc-plugins.rst
F:      scripts/Makefile.gcc-plugins
F:      scripts/gcc-plugin.sh
F:      scripts/gcc-plugins/
LEAKING_ADDRESSES
M:      Tobin C. Harding <[email protected]>
M:      Tycho Andersen <[email protected]>
L:      [email protected]
S:      Maintained
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/tobin/leaks.git
F:      scripts/leaking_addresses.pl
Quoted 8 lines
Alternatively, would this be acceptable? -
L: [email protected] (only for messages focused on core functionality, not for maintenance detail)
I think the latter would be best, if allowed.
Kees, please comment (so that we'd hopefully not need that next time), and if you agree please make a change to MAINTAINERS.
A comment isn't going to really help fix this (much of the CCing is done by scripts, etc).

Quoted 8 lines

I've tended to prefer more emails than missing discussions, and I think it's not unreasonable to have the list mentioned in MAINTAINERS for those things. It does, of course, mean that "maintenance" patches get directed there too, as you say.
If it's really something you'd like to avoid, I can drop those references. My instinct is to leave it as-is, but the strength of my opinion is pretty small. Let me know what you prefer...

One thing about leaking_addresses.pl is that I'm not sure anyone is actively using it at this point. I told Tobin I'd help review stuff, but I don't even have a GPG key with enough signatures to send PRs. I'm slowly working on figuring that out, but in the meantime I wonder if we couldn't move it into some self test somehow, so that at least nobody adds new leaks? Does that seem worth doing?

It would then probably go away as a separate perl script and live under selftests, which could mean we could drop the reference to the list. But that's me making it someone else's problem then, kind of :)

Also, I'm switching my e-mail address to [email protected], so future replies will be from there.

Tycho

Tobin C. Harding

2020-09-01 04:25 UTC #8 lore raw

Quoted 69 lines · Tycho Andersen

Quoted 52 lines · Kees Cook
Quoted 39 lines · Solar Designer
Quoted 1 line · Mrinal Pandey
 mode change 100644 => 100755 scripts/gcc-plugins/gen-random-seed.sh
This is basically the only change relevant to the contribution initially made via kernel-hardening, and in my opinion (and I am list admin) isn't worth bringing to the list. Now we have this bikeshed thread in here (and I'm guilty for adding to it), and would have more (which I hope this message of mine will prevent) if changes to something else in the patch(es) are requested (which Greg KH sort of already did).
I recall we previously had lots of "similar" bikeshedding in here when someone was converting the documentation to rST. The more bikeshedding we have, the less actual kernel-hardening work is going to happen, because the list gets the reputation of yet another kernel maintenance list rather than the place where actual/potential new contributions to improve the kernel's security are discussed, and because bikeshedding makes the most capable people unsubscribe or stop paying attention.
How about we remove kernel-hardening from the MAINTAINERS entries it's currently in? -
GCC PLUGINS
M:      Kees Cook <[email protected]>
R:      Emese Revfy <[email protected]>
L:      [email protected]
S:      Maintained
F:      Documentation/kbuild/gcc-plugins.rst
F:      scripts/Makefile.gcc-plugins
F:      scripts/gcc-plugin.sh
F:      scripts/gcc-plugins/
LEAKING_ADDRESSES
M:      Tobin C. Harding <[email protected]>
M:      Tycho Andersen <[email protected]>
L:      [email protected]
S:      Maintained
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/tobin/leaks.git
F:      scripts/leaking_addresses.pl
Quoted 8 lines
Alternatively, would this be acceptable? -
L: [email protected] (only for messages focused on core functionality, not for maintenance detail)
I think the latter would be best, if allowed.
Kees, please comment (so that we'd hopefully not need that next time), and if you agree please make a change to MAINTAINERS.
A comment isn't going to really help fix this (much of the CCing is done by scripts, etc).
Quoted 8 lines
I've tended to prefer more emails than missing discussions, and I think it's not unreasonable to have the list mentioned in MAINTAINERS for those things. It does, of course, mean that "maintenance" patches get directed there too, as you say.
If it's really something you'd like to avoid, I can drop those references. My instinct is to leave it as-is, but the strength of my opinion is pretty small. Let me know what you prefer...
One thing about leaking_addresses.pl is that I'm not sure anyone is actively using it at this point. I told Tobin I'd help review stuff, but I don't even have a GPG key with enough signatures to send PRs. I'm slowly working on figuring that out, but in the meantime I wonder if we couldn't move it into some self test somehow, so that at least nobody adds new leaks? Does that seem worth doing?

Quoted 6 lines

It would then probably go away as a separate perl script and live under selftests, which could mean we could drop the reference to the list. But that's me making it someone else's problem then, kind of :)
Also, I'm switching my e-mail address to [email protected], so future replies will be from there.

I don't mind if the reference to kernel-hardening is removed, if in the event that someone sends a patch that needs input from the kernel hardening community we can always mail the list.

Thanks,
Tobin

Solar Designer

2020-09-02 12:17 UTC #9 lore raw

Quoted 28 lines · Tobin C. Harding

Quoted 27 lines · Tycho Andersen
Quoted 26 lines · Kees Cook
Quoted 25 lines · Solar Designer
How about we remove kernel-hardening from the MAINTAINERS entries it's currently in? -
GCC PLUGINS
M:      Kees Cook <[email protected]>
R:      Emese Revfy <[email protected]>
L:      [email protected]
S:      Maintained
F:      Documentation/kbuild/gcc-plugins.rst
F:      scripts/Makefile.gcc-plugins
F:      scripts/gcc-plugin.sh
F:      scripts/gcc-plugins/
LEAKING_ADDRESSES
M:      Tobin C. Harding <[email protected]>
M:      Tycho Andersen <[email protected]>
L:      [email protected]
S:      Maintained
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/tobin/leaks.git
F:      scripts/leaking_addresses.pl
Alternatively, would this be acceptable? -
L: [email protected] (only for messages focused on core functionality, not for maintenance detail)

Quoted 7 lines

Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
I think the latter would be best, if allowed.
Kees, please comment (so that we'd hopefully not need that next time), and if you agree please make a change to MAINTAINERS.
A comment isn't going to really help fix this (much of the CCing is done by scripts, etc).

Understood. Maybe some other agreed-upon syntax would help - a new tag letter in place of "L" and/or e-mail address obfuscation or an https URL for further information instead of a direct posting address - but I guess this is only worth introducing if we're not unique with this wish.

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
I've tended to prefer more emails than missing discussions, and I think it's not unreasonable to have the list mentioned in MAINTAINERS for those things. It does, of course, mean that "maintenance" patches get directed there too, as you say.
If it's really something you'd like to avoid, I can drop those references. My instinct is to leave it as-is, but the strength of my opinion is pretty small. Let me know what you prefer...

Thank you for your comments, Kees.

It's not a matter of my preference, but of what works best for getting more actual work done. Unfortunately, we have to make our subjective guesses on this. FYI, when we dropped the [kernel-hardening] prefix on Subjects this appears to have resulted in some people unsubscribing. I agree we had to do that anyway because of CC'ing other lists, which is customary in Linux kernel development. Before that change, we had a slow but steady growth in the number of subscribers. When we made that change, the numbers of people joining and leaving became about the same, so we're staying at 600 to 650 subscribed addresses for a long time now. These numbers are fine by themselves; it's more relevant who is on the list, not how many. I think we might have "forced" some capable people to unsubscribe, but like I explained we kind of had to. Now I think we're doing the same with these maintenance-only threads, and I think we don't have to. This is why I think we should preferably either somehow limit the requested CC's to messages focused on core functionality, or if we can't then drop the list references from MAINTAINERS. We should also not discuss this for very long, as this discussion itself hurts actual work in a similar way.

Quoted 11 lines

Quoted 11 lines
One thing about leaking_addresses.pl is that I'm not sure anyone is actively using it at this point. I told Tobin I'd help review stuff, but I don't even have a GPG key with enough signatures to send PRs. I'm slowly working on figuring that out, but in the meantime I wonder if we couldn't move it into some self test somehow, so that at least nobody adds new leaks? Does that seem worth doing?
It would then probably go away as a separate perl script and live under selftests, which could mean we could drop the reference to the list. But that's me making it someone else's problem then, kind of :)

Quoted 6 lines

Quoted 2 lines
Also, I'm switching my e-mail address to [email protected], so future replies will be from there.
I don't mind if the reference to kernel-hardening is removed, if in the event that someone sends a patch that needs input from the kernel hardening community we can always mail the list.

Thank you for your comments as well, Tycho and Tobin.

Alexander

Kees Cook

2020-08-27 18:14 UTC #10 lore raw

In reply to Mrinal Pandey

Quoted 7 lines · Mrinal Pandey

commit eb8305aecb95 ("scripts: Coccinelle script for namespace dependencies.") added the file nsdeps, commit 313dd1b62921 ("gcc-plugins:
Add the randstruct plugin") added the file gcc-plugins/gen-random-seed.sh and commit 9b4ade226f74 ("xen: build infrastructure for generating hypercall depending symbols") added the file xen-hypercalls.sh without the executable bit.
[...]

Quoted 9 lines

 scripts/gcc-plugins/gen-random-seed.sh | 0
 scripts/nsdeps                         | 0
 scripts/spdxcheck-test.sh              | 1 +
 scripts/xen-hypercalls.sh              | 0
 4 files changed, 1 insertion(+)
 mode change 100644 => 100755 scripts/gcc-plugins/gen-random-seed.sh
 mode change 100644 => 100755 scripts/nsdeps
 mode change 100644 => 100755 scripts/spdxcheck-test.sh
 mode change 100644 => 100755 scripts/xen-hypercalls.sh

I can't find "official" guidance on this right now, but I'm pretty sure this (having execute bits set correctly) wasn't something we could depend on (i.e. regular "diff" output doesn't support it (just git's diff), and copies of the tree (or tarballs, etc) may have missed the bits). All the portions of the kernel that uses these kinds of files explicitly specify the interpreter (or universally set the execute bit)[1]. As such, is this change useful?

It might be better to _remove_ execute bits to catch the places where the build is accidentally depending on them. ;)

-Kees

[1] These all use CONFIG_SHELL:

scripts/gcc-plugins/Makefile:
$(CONFIG_SHELL) $(srctree)/$(src)/gen-random-seed.sh $@ $(objtree)/include/generated/randomize_layout_hash.h

Makefile:

       $(Q)$(CONFIG_SHELL) $(srctree)/scripts/nsdeps

arch/x86/entry/syscalls/Makefile:
quiet_cmd_hypercalls = HYPERCALLS $@

      cmd_hypercalls = $(CONFIG_SHELL) '$<' $@ $(filter-out $<,$^)
...
$(out)/xen-hypercalls.h: $(srctree)/scripts/xen-hypercalls.sh
        $(call if_changed,hypercalls)

And I can't even find anything in the kernel that calls scripts/spdxcheck-test.sh :) I think that should likely be moved into the selftests directory and wired up.

Signature · 1 line

Kees Cook

Andrew Morton

2020-08-31 00:44 UTC #11 lore raw

In reply to Mrinal Pandey

Quoted 3 lines · On Thu, 27 Aug 2020 14:54:05 +0530 Mrinal Pandey <[email protected]> wrote:

commit b72231eb7084 ("scripts: add spdxcheck.py self test") added the file spdxcheck-test.sh to the repository without the executable flag and license information.

The x bit shouldn't matter.

If someone downloads and applies patch-5.9.xz (which is a supported way of obtaining a kernel) then patch(1) will erase the x bit anyway.

Is some other script invoking spdxcheck-test.sh directly, instead of using `/bin/sh spdxcheck-test.sh'? If so, please let's fix that.

Lukas Bulwahn

2020-08-31 05:45 UTC #12 lore raw

Quoted 11 lines · Andrew Morton

Quoted 3 lines · On Thu, 27 Aug 2020 14:54:05 +0530 Mrinal Pandey <[email protected]> wrote:
commit b72231eb7084 ("scripts: add spdxcheck.py self test") added the file spdxcheck-test.sh to the repository without the executable flag and license information.
The x bit shouldn't matter.
If someone downloads and applies patch-5.9.xz (which is a supported way of obtaining a kernel) then patch(1) will erase the x bit anyway.

Andrew, Kees,

thanks for the feedback.

As his mentor, I see two valuable tasks for Mrinal to work on:

1. Document this knowledge how scripts should be called, not relying on the executable bit, probably best somewhere here:
./Documentation/kbuild/makefiles.rst, a new section on using dedicated scripts in chapter 3 ("The kbuild files").

https://www.kernel.org/doc/html/latest/kbuild/makefiles.html#the-kbuild-files

2. Determine if there are places in the build Makefiles that do rely on the executable bit and fix those script invocations. (Kees' idea of remove all executable bits and see...)

Lukas
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Kees Cook

2020-08-31 19:20 UTC #13 lore raw

Quoted 20 lines · Lukas Bulwahn

Quoted 11 lines · Andrew Morton
Quoted 3 lines · On Thu, 27 Aug 2020 14:54:05 +0530 Mrinal Pandey <[email protected]> wrote:
commit b72231eb7084 ("scripts: add spdxcheck.py self test") added the file spdxcheck-test.sh to the repository without the executable flag and license information.
The x bit shouldn't matter.
If someone downloads and applies patch-5.9.xz (which is a supported way of obtaining a kernel) then patch(1) will erase the x bit anyway.
Andrew, Kees,
thanks for the feedback.

Quoted 8 lines

As his mentor, I see two valuable tasks for Mrinal to work on:
1. Document this knowledge how scripts should be called, not relying on the executable bit, probably best somewhere here:
./Documentation/kbuild/makefiles.rst, a new section on using dedicated scripts in chapter 3 ("The kbuild files").
https://www.kernel.org/doc/html/latest/kbuild/makefiles.html#the-kbuild-files

Yes, that would be excellent.

Quoted 3 lines

2. Determine if there are places in the build Makefiles that do rely on the executable bit and fix those script invocations. (Kees' idea of remove all executable bits and see...)

I think this has value, yes. I don't think patches to remove the x bits are needed, but any cases where they are depended on need to be fixed.

And I think "3" could be "wire up or remove spdx shell script"

Signature · 1 line

Kees Cook