From: Ujjwal Kumar <ujjwalkumar0501@gmail.com>
To: Joe Perches <joe@perches.com>, Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: linux-kernel-mentees@lists.linuxfoundation.org,
linux-kernel@vger.kernel.org
Subject: Re: [Linux-kernel-mentees] [RFC PATCH v2] checkpatch: add shebang check to EXECUTE_PERMISSIONS
Date: Tue, 13 Oct 2020 17:43:59 +0530 [thread overview]
Message-ID: <da3053be-f27e-f99c-0a8a-447adb9733d2@gmail.com> (raw)
In-Reply-To: <20201013120129.1304101-1-ujjwalkumar0501@gmail.com>
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 <ujjwalkumar0501@gmail.com>
> ---
> 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
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
next prev parent reply other threads:[~2020-10-13 12:14 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-13 12:01 [Linux-kernel-mentees] [RFC PATCH v2] checkpatch: add shebang check to EXECUTE_PERMISSIONS Ujjwal Kumar
2020-10-13 12:13 ` Ujjwal Kumar [this message]
2020-10-13 15:27 ` Joe Perches
2020-10-14 5:46 ` Lukas Bulwahn
2020-10-14 6:01 ` Joe Perches
2020-10-14 6:21 ` Lukas Bulwahn
2020-10-14 6:27 ` Joe Perches
2020-10-14 6:36 ` Lukas Bulwahn
2020-10-14 6:39 ` Joe Perches
2020-10-14 6:47 ` Lukas Bulwahn
2020-10-14 6:58 ` Joe Perches
2020-10-14 7:17 ` Lukas Bulwahn
2020-10-14 7:35 ` Joe Perches
2020-10-14 17:45 ` Miguel Ojeda
2020-10-14 18:05 ` Joe Perches
2020-10-14 18:32 ` Miguel Ojeda
2020-10-14 18:37 ` Joe Perches
2020-10-14 11:14 ` Ujjwal Kumar
2020-10-16 9:05 ` Ujjwal Kumar
2020-10-16 10:10 ` Lukas Bulwahn
2020-10-16 10:24 ` Dwaipayan Ray
2020-10-16 10:33 ` Lukas Bulwahn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=da3053be-f27e-f99c-0a8a-447adb9733d2@gmail.com \
--to=ujjwalkumar0501@gmail.com \
--cc=joe@perches.com \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lukas.bulwahn@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).