@@ -2973,7 +2973,7 @@ sub process {}if(!defined$lines[$linenr]){WARN("BAD_SIGN_OFF",-"Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline);+"Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline);}elsif($rawlines[$linenr]!~/^\s*signed-off-by:\s*(.*)/i){WARN("BAD_SIGN_OFF","Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline."\n".$rawlines[$linenr]);
@@ -2996,8 +2996,8 @@ sub process {if(ERROR("GERRIT_CHANGE_ID","Remove Gerrit Change-Id's before submitting upstream\n".$herecurr)&&$fix){-fix_delete_line($fixlinenr,$rawline);-}+fix_delete_line($fixlinenr,$rawline);+}}#Checkifthecommitlogisinapossiblestackdump
@@ -3239,10 +3239,10 @@ sub process {nextif($start_char=~/^\S$/);nextif(index(" \t.,;?!",$end_char)==-1);-#avoidrepeatinghexoccurrenceslike'ff ff fe 09 ...'-if($first=~/\b[0-9a-f]{2,}\b/i){-nextif(!exists($allow_repeated_words{lc($first)}));-}+#avoidrepeatinghexoccurrenceslike'ff ff fe 09 ...'+if($first=~/\b[0-9a-f]{2,}\b/i){+nextif(!exists($allow_repeated_words{lc($first)}));+}if(WARN("REPEATED_WORD","Possible repeated word: '$first'\n".$herecurr)&&
@@ -4426,7 +4426,7 @@ sub process {WARN("STATIC_CONST_CHAR_ARRAY","char * array declaration might be better as static const\n".$herecurr);-}+}#checkforsizeof(foo)/sizeof(foo[0])thatcouldbeARRAY_SIZE(foo)if($line=~m@\bsizeof\s*\(\s*($Lval)\s*\)@){
@@ -5279,7 +5279,7 @@ sub process {$lines[$linenr-3]!~/^[+]\s*$Ident\s*:/){WARN("RETURN_VOID","void function return statements are not generally useful\n".$hereprev);-}+}#ifstatementsusingunnecessaryparentheses-ie:if((foo==bar))if($perl_version_ok&&
--
2.27.0
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
From: Dwaipayan Ray <dwaipayanray1@gmail.com> Date: 2021-01-04 13:26:01
strcpy() performs no bounds checking on the destination buffer.
This could result in linear overflows beyond the end of the buffer.
strlcpy() reads the entire source buffer first. This read
may exceed the destination size limit. This can be both inefficient
and lead to linear read overflows.
The safe replacement to both of these is to use strscpy() instead.
Add a new checkpatch warning which alerts the user on finding usage of
strcpy() or strlcpy().
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
scripts/checkpatch.pl | 7 +++++++
1 file changed, 7 insertions(+)
@@ -6604,6 +6604,13 @@ sub process {}}+#Checkforstrcpy/strlcpyuses+if(defined($stat)&&+$stat=~/^\+(?:.*?)\b(str[l]?cpy)\s*\(/){+WARN("PREFER_STRSCPY",+"Prefer strscpy() over $1()\n"."$here\n$stat\n");+}+#Checkformemcpy(foo,bar,ETH_ALEN)thatcouldbeether_addr_copy(foo,bar)#if($perl_version_ok&&#defined$stat&&
--
2.27.0
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
On Mon, Jan 4, 2021 at 2:25 PM Dwaipayan Ray [off-list ref] wrote:
strcpy() performs no bounds checking on the destination buffer.
This could result in linear overflows beyond the end of the buffer.
strlcpy() reads the entire source buffer first. This read
may exceed the destination size limit. This can be both inefficient
and lead to linear read overflows.
The safe replacement to both of these is to use strscpy() instead.
Add a new checkpatch warning which alerts the user on finding usage of
strcpy() or strlcpy().
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
I remember Joe has already created a patch for that over Christmas
break; check lkml before sending this.
Other than that, looks good.
Lukas
@@ -6604,6 +6604,13 @@ sub process {}}+#Checkforstrcpy/strlcpyuses+if(defined($stat)&&+$stat=~/^\+(?:.*?)\b(str[l]?cpy)\s*\(/){+WARN("PREFER_STRSCPY",+"Prefer strscpy() over $1()\n"."$here\n$stat\n");+}+#Checkformemcpy(foo,bar,ETH_ALEN)thatcouldbeether_addr_copy(foo,bar)#if($perl_version_ok&&#defined$stat&&--
On Mon, Jan 4, 2021 at 2:25 PM Dwaipayan Ray [off-list ref] wrote:
Indentations should use tabs wherever possible.
Replace spaces by tabs for indents.
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
Looks good.
Find out who is to blame for this stylistic mess up. (use git blame).
If it is Aditya, let him know to check better and how you discovered
this mess-up.
Lukas
@@ -2973,7 +2973,7 @@ sub process {}if(!defined$lines[$linenr]){WARN("BAD_SIGN_OFF",-"Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline);+"Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline);}elsif($rawlines[$linenr]!~/^\s*signed-off-by:\s*(.*)/i){WARN("BAD_SIGN_OFF","Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline."\n".$rawlines[$linenr]);
@@ -2996,8 +2996,8 @@ sub process {if(ERROR("GERRIT_CHANGE_ID","Remove Gerrit Change-Id's before submitting upstream\n".$herecurr)&&$fix){-fix_delete_line($fixlinenr,$rawline);-}+fix_delete_line($fixlinenr,$rawline);+}}#Checkifthecommitlogisinapossiblestackdump
@@ -3239,10 +3239,10 @@ sub process {nextif($start_char=~/^\S$/);nextif(index(" \t.,;?!",$end_char)==-1);-#avoidrepeatinghexoccurrenceslike'ff ff fe 09 ...'-if($first=~/\b[0-9a-f]{2,}\b/i){-nextif(!exists($allow_repeated_words{lc($first)}));-}+#avoidrepeatinghexoccurrenceslike'ff ff fe 09 ...'+if($first=~/\b[0-9a-f]{2,}\b/i){+nextif(!exists($allow_repeated_words{lc($first)}));+}if(WARN("REPEATED_WORD","Possible repeated word: '$first'\n".$herecurr)&&
@@ -4426,7 +4426,7 @@ sub process {WARN("STATIC_CONST_CHAR_ARRAY","char * array declaration might be better as static const\n".$herecurr);-}+}#checkforsizeof(foo)/sizeof(foo[0])thatcouldbeARRAY_SIZE(foo)if($line=~m@\bsizeof\s*\(\s*($Lval)\s*\)@){
@@ -5279,7 +5279,7 @@ sub process {$lines[$linenr-3]!~/^[+]\s*$Ident\s*:/){WARN("RETURN_VOID","void function return statements are not generally useful\n".$hereprev);-}+}#ifstatementsusingunnecessaryparentheses-ie:if((foo==bar))if($perl_version_ok&&--
From: Dwaipayan Ray <dwaipayanray1@gmail.com> Date: 2021-01-05 08:19:48
On Tue, Jan 5, 2021 at 1:32 PM Lukas Bulwahn [off-list ref] wrote:
On Mon, Jan 4, 2021 at 2:25 PM Dwaipayan Ray [off-list ref] wrote:
quoted
strcpy() performs no bounds checking on the destination buffer.
This could result in linear overflows beyond the end of the buffer.
strlcpy() reads the entire source buffer first. This read
may exceed the destination size limit. This can be both inefficient
and lead to linear read overflows.
The safe replacement to both of these is to use strscpy() instead.
Add a new checkpatch warning which alerts the user on finding usage of
strcpy() or strlcpy().
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
I remember Joe has already created a patch for that over Christmas
break; check lkml before sending this.
Other than that, looks good.
Lukas
From: Dwaipayan Ray <dwaipayanray1@gmail.com> Date: 2021-01-05 08:41:21
On Tue, Jan 5, 2021 at 1:34 PM Lukas Bulwahn [off-list ref] wrote:
On Mon, Jan 4, 2021 at 2:25 PM Dwaipayan Ray [off-list ref] wrote:
quoted
Indentations should use tabs wherever possible.
Replace spaces by tabs for indents.
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
Looks good.
Find out who is to blame for this stylistic mess up. (use git blame).
If it is Aditya, let him know to check better and how you discovered
this mess-up.
I think some of Aditya's earlier patches caused this. I was idly running the
regex "^[ ]+" on files which detected these misuses. Probably it's a
broken editor
which expanded tabs, maybe vim with exandtab set. Aditya could you verify your
editor config? Thanks.
Thank you,
Dwaipayan.
@@ -2973,7 +2973,7 @@ sub process {}if(!defined$lines[$linenr]){WARN("BAD_SIGN_OFF",-"Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline);+"Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline);}elsif($rawlines[$linenr]!~/^\s*signed-off-by:\s*(.*)/i){WARN("BAD_SIGN_OFF","Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline."\n".$rawlines[$linenr]);
@@ -2996,8 +2996,8 @@ sub process {if(ERROR("GERRIT_CHANGE_ID","Remove Gerrit Change-Id's before submitting upstream\n".$herecurr)&&$fix){-fix_delete_line($fixlinenr,$rawline);-}+fix_delete_line($fixlinenr,$rawline);+}}#Checkifthecommitlogisinapossiblestackdump
@@ -3239,10 +3239,10 @@ sub process {nextif($start_char=~/^\S$/);nextif(index(" \t.,;?!",$end_char)==-1);-#avoidrepeatinghexoccurrenceslike'ff ff fe 09 ...'-if($first=~/\b[0-9a-f]{2,}\b/i){-nextif(!exists($allow_repeated_words{lc($first)}));-}+#avoidrepeatinghexoccurrenceslike'ff ff fe 09 ...'+if($first=~/\b[0-9a-f]{2,}\b/i){+nextif(!exists($allow_repeated_words{lc($first)}));+}if(WARN("REPEATED_WORD","Possible repeated word: '$first'\n".$herecurr)&&
@@ -4426,7 +4426,7 @@ sub process {WARN("STATIC_CONST_CHAR_ARRAY","char * array declaration might be better as static const\n".$herecurr);-}+}#checkforsizeof(foo)/sizeof(foo[0])thatcouldbeARRAY_SIZE(foo)if($line=~m@\bsizeof\s*\(\s*($Lval)\s*\)@){
@@ -5279,7 +5279,7 @@ sub process {$lines[$linenr-3]!~/^[+]\s*$Ident\s*:/){WARN("RETURN_VOID","void function return statements are not generally useful\n".$hereprev);-}+}#ifstatementsusingunnecessaryparentheses-ie:if((foo==bar))if($perl_version_ok&&--
On Tue, Jan 5, 2021 at 9:19 AM Dwaipayan Ray [off-list ref] wrote:
On Tue, Jan 5, 2021 at 1:32 PM Lukas Bulwahn [off-list ref] wrote:
quoted
On Mon, Jan 4, 2021 at 2:25 PM Dwaipayan Ray [off-list ref] wrote:
quoted
strcpy() performs no bounds checking on the destination buffer.
This could result in linear overflows beyond the end of the buffer.
strlcpy() reads the entire source buffer first. This read
may exceed the destination size limit. This can be both inefficient
and lead to linear read overflows.
The safe replacement to both of these is to use strscpy() instead.
Add a new checkpatch warning which alerts the user on finding usage of
strcpy() or strlcpy().
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
I remember Joe has already created a patch for that over Christmas
break; check lkml before sending this.
Other than that, looks good.
Lukas
Joe pointed you already to the commit. So I think your change is obsolete.
And I fully agree with Joe. strcpy is perfectly fine, when it is clear
from the use that boundaries do not need to be checked.
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
On Tue, Jan 5, 2021 at 1:34 PM Lukas Bulwahn [off-list ref] wrote:
quoted
On Mon, Jan 4, 2021 at 2:25 PM Dwaipayan Ray [off-list ref] wrote:
quoted
Indentations should use tabs wherever possible.
Replace spaces by tabs for indents.
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
Looks good.
Find out who is to blame for this stylistic mess up. (use git blame).
If it is Aditya, let him know to check better and how you discovered
this mess-up.
I think some of Aditya's earlier patches caused this. I was idly running the
regex "^[ ]+" on files which detected these misuses. Probably it's a
broken editor
which expanded tabs, maybe vim with exandtab set. Aditya could you verify your
editor config? Thanks.
Yes, this problem was there with my earlier setup, probably because of
configured tab size and word wrap. I think my current flow works better.
Thanks
Aditya
@@ -2973,7 +2973,7 @@ sub process {}if(!defined$lines[$linenr]){WARN("BAD_SIGN_OFF",-"Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline);+"Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline);}elsif($rawlines[$linenr]!~/^\s*signed-off-by:\s*(.*)/i){WARN("BAD_SIGN_OFF","Co-developed-by: must be immediately followed by Signed-off-by:\n"."$here\n".$rawline."\n".$rawlines[$linenr]);
@@ -2996,8 +2996,8 @@ sub process {if(ERROR("GERRIT_CHANGE_ID","Remove Gerrit Change-Id's before submitting upstream\n".$herecurr)&&$fix){-fix_delete_line($fixlinenr,$rawline);-}+fix_delete_line($fixlinenr,$rawline);+}}#Checkifthecommitlogisinapossiblestackdump
@@ -3239,10 +3239,10 @@ sub process {nextif($start_char=~/^\S$/);nextif(index(" \t.,;?!",$end_char)==-1);-#avoidrepeatinghexoccurrenceslike'ff ff fe 09 ...'-if($first=~/\b[0-9a-f]{2,}\b/i){-nextif(!exists($allow_repeated_words{lc($first)}));-}+#avoidrepeatinghexoccurrenceslike'ff ff fe 09 ...'+if($first=~/\b[0-9a-f]{2,}\b/i){+nextif(!exists($allow_repeated_words{lc($first)}));+}if(WARN("REPEATED_WORD","Possible repeated word: '$first'\n".$herecurr)&&
@@ -4426,7 +4426,7 @@ sub process {WARN("STATIC_CONST_CHAR_ARRAY","char * array declaration might be better as static const\n".$herecurr);-}+}#checkforsizeof(foo)/sizeof(foo[0])thatcouldbeARRAY_SIZE(foo)if($line=~m@\bsizeof\s*\(\s*($Lval)\s*\)@){
@@ -5279,7 +5279,7 @@ sub process {$lines[$linenr-3]!~/^[+]\s*$Ident\s*:/){WARN("RETURN_VOID","void function return statements are not generally useful\n".$hereprev);-}+}#ifstatementsusingunnecessaryparentheses-ie:if((foo==bar))if($perl_version_ok&&--