Detect an opencoded expression that is used before or after
array_size()/array3_size()/struct_size() to compute the same size.
Cc: Kees Cook <[email protected]>
Signed-off-by: Denis Efremov <[email protected]>
---
scripts/coccinelle/misc/array_size_dup.cocci | 347 +++++++++++++++++++
1 file changed, 347 insertions(+)
create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci
diff --git a/scripts/coccinelle/misc/array_size_dup.cocci b/scripts/coccinelle/misc/array_size_dup.cocci
new file mode 100644
index 000000000000..08919a938754
--- /dev/null
+++ b/scripts/coccinelle/misc/array_size_dup.cocci
@@ -0,0 +1,347 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for array_size(), array3_size(), struct_size() duplicates.
+/// Three types of patterns for these functions:
+/// 1. An opencoded expression is used before array_size() to compute the same size
+/// 2. An opencoded expression is used after array_size() to compute the same size
+/// 3. Consecutive calls of array_size() with the same values
+/// From security point of view only first case is relevant. These functions
+/// perform arithmetic overflow check. Thus, if we use an opencoded expression
+/// before a call to the *_size() function we can miss an overflow.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers --no-loops
+
+virtual context
+virtual report
+virtual org
+
+@as@
+expression E1, E2;
+@@
+
+array_size(E1, E2)
+
+@as_next@
+expression subE1 <= as.E1;
+expression as.E1;
+expression subE2 <= as.E2;
+expression as.E2;
+expression E3;
+position p1, p2;
+@@
+
+* E1 * E2@p1
+ ... when != \(E1\|E2\|subE1\|subE2\)=E3
+ when != \(E1\|E2\|subE1\|subE2\)+=E3
+ when != \(E1\|E2\|subE1\|subE2\)-=E3
+ when != \(E1\|E2\|subE1\|subE2\)*=E3
+ when != \(&E1\|&E2\|&subE1\|&subE2\)
+* array_size(E1, E2)@p2
+
+@as_prev@
+expression subE1 <= as.E1;
+expression as.E1;
+expression subE2 <= as.E2;
+expression as.E2;
+expression E3;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\)=E3
+ when != \(E1\|E2\|subE1\|subE2\)+=E3
+ when != \(E1\|E2\|subE1\|subE2\)-=E3
+ when != \(E1\|E2\|subE1\|subE2\)*=E3
+ when != \(&E1\|&E2\|&subE1\|&subE2\)
+* E1 * E2@p2
+
+@as_dup@
+expression subE1 <= as.E1;
+expression as.E1;
+expression subE2 <= as.E2;
+expression as.E2;
+expression E3;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\)=E3
+ when != \(E1\|E2\|subE1\|subE2\)+=E3
+ when != \(E1\|E2\|subE1\|subE2\)-=E3
+ when != \(E1\|E2\|subE1\|subE2\)*=E3
+ when != \(&E1\|&E2\|&subE1\|&subE2\)
+* array_size(E1, E2)@p2
+
+@as3@
+expression E1, E2, E3;
+@@
+
+array3_size(E1, E2, E3)
+
+@as3_next@
+expression subE1 <= as3.E1;
+expression as3.E1;
+expression subE2 <= as3.E2;
+expression as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E3;
+expression E4;
+position p1, p2;
+@@
+
+* E1 * E2 * E3@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
+ when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
+* array3_size(E1, E2, E3)@p2
+
+@as3_prev@
+expression subE1 <= as3.E1;
+expression as3.E1;
+expression subE2 <= as3.E2;
+expression as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E3;
+expression E4;
+position p1, p2;
+@@
+
+* array3_size(E1, E2, E3)@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
+ when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
+* E1 * E2 * E3@p2
+
+@as3_dup@
+expression subE1 <= as3.E1;
+expression as3.E1;
+expression subE2 <= as3.E2;
+expression as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E3;
+expression E4;
+position p1, p2;
+@@
+
+* array3_size(E1, E2, E3)@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
+ when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
+* array3_size(E1, E2, E3)@p2
+
+@ss@
+expression E1, E2, E3;
+@@
+
+struct_size(E1, E2, E3)
+
+@ss_next@
+expression subE1 <= ss.E1;
+expression ss.E1;
+expression subE2 <= ss.E2;
+expression ss.E2;
+expression subE3 <= ss.E3;
+expression ss.E3;
+expression E4;
+position p1, p2;
+@@
+
+* E1 * E2 + E3@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
+ when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
+* struct_size(E1, E2, E3)@p2
+
+@ss_prev@
+expression subE1 <= ss.E1;
+expression ss.E1;
+expression subE2 <= ss.E2;
+expression ss.E2;
+expression subE3 <= ss.E3;
+expression ss.E3;
+expression E4;
+position p1, p2;
+@@
+
+* struct_size(E1, E2, E3)@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
+ when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
+* E1 * E2 + E3@p2
+
+@ss_dup@
+expression subE1 <= ss.E1;
+expression ss.E1;
+expression subE2 <= ss.E2;
+expression ss.E2;
+expression subE3 <= ss.E3;
+expression ss.E3;
+expression E4;
+position p1, p2;
+@@
+
+* struct_size(E1, E2, E3)@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
+ when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
+ when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
+* struct_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+msg = "WARNING: array_size is used down the code (line %s) to compute the same size" % (p2[0].line)
+coccilib.report.print_report(p1[0], msg)
+
+@script:python depends on org@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+msg = "WARNING: array_size is used down the code (line %s) to compute the same size" % (p2[0].line)
+coccilib.org.print_todo(p1[0], msg)
+
+@script:python depends on report@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.report.print_report(p2[0], msg)
+
+@script:python depends on org@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.org.print_todo(p2[0], msg)
+
+@script:python depends on report@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+msg = "WARNING: same array_size (line %s)" % (p1[0].line)
+coccilib.report.print_report(p2[0], msg)
+
+@script:python depends on org@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+msg = "WARNING: same array_size (line %s)" % (p1[0].line)
+coccilib.org.print_todo(p2[0], msg)
+
+
+@script:python depends on report@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+msg = "WARNING: array3_size is used down the code (line %s) to compute the same size" % (p2[0].line)
+coccilib.report.print_report(p1[0], msg)
+
+@script:python depends on org@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+msg = "WARNING: array3_size is used down the code (line %s) to compute the same size" % (p2[0].line)
+coccilib.org.print_todo(p1[0], msg)
+
+@script:python depends on report@
+p1 << as3_prev.p1;
+p2 << as3_prev.p2;
+@@
+
+msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.report.print_report(p2[0], msg)
+
+@script:python depends on org@
+p1 << as3_prev.p1;
+p2 << as3_prev.p2;
+@@
+
+msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.org.print_todo(p2[0], msg)
+
+@script:python depends on report@
+p1 << as3_dup.p1;
+p2 << as3_dup.p2;
+@@
+
+msg = "WARNING: same array3_size (line %s)" % (p1[0].line)
+coccilib.report.print_report(p2[0], msg)
+
+@script:python depends on org@
+p1 << as3_dup.p1;
+p2 << as3_dup.p2;
+@@
+
+msg = "WARNING: same array3_size (line %s)" % (p1[0].line)
+coccilib.org.print_todo(p2[0], msg)
+
+
+@script:python depends on report@
+p1 << ss_next.p1;
+p2 << ss_next.p2;
+@@
+
+msg = "WARNING: struct_size is used down the code (line %s) to compute the same size" % (p2[0].line)
+coccilib.report.print_report(p1[0], msg)
+
+@script:python depends on org@
+p1 << ss_next.p1;
+p2 << ss_next.p2;
+@@
+
+msg = "WARNING: struct_size is used down the code (line %s) to compute the same size" % (p2[0].line)
+coccilib.org.print_todo(p1[0], msg)
+
+@script:python depends on report@
+p1 << ss_prev.p1;
+p2 << ss_prev.p2;
+@@
+
+msg = "WARNING: struct_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.report.print_report(p2[0], msg)
+
+@script:python depends on org@
+p1 << ss_prev.p1;
+p2 << ss_prev.p2;
+@@
+
+msg = "WARNING: struct_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.org.print_todo(p2[0], msg)
+
+@script:python depends on report@
+p1 << ss_dup.p1;
+p2 << ss_dup.p2;
+@@
+
+msg = "WARNING: same struct_size (line %s)" % (p1[0].line)
+coccilib.report.print_report(p2[0], msg)
+
+@script:python depends on org@
+p1 << ss_dup.p1;
+p2 << ss_dup.p2;
+@@
+
+msg = "WARNING: same struct_size (line %s)" % (p1[0].line)
+coccilib.org.print_todo(p2[0], msg)
--
2.26.2
On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote:
> Detect an opencoded expression that is used before or after
> array_size()/array3_size()/struct_size() to compute the same size.
>
> Cc: Kees Cook <[email protected]>
> Signed-off-by: Denis Efremov <[email protected]>
Oh, very cool! How much does this find currently?
--
Kees Cook
On 6/15/20 9:23 PM, Kees Cook wrote:
> On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote:
>> Detect an opencoded expression that is used before or after
>> array_size()/array3_size()/struct_size() to compute the same size.
>>
>> Cc: Kees Cook <[email protected]>
>> Signed-off-by: Denis Efremov <[email protected]>
>
> Oh, very cool! How much does this find currently?
>
opencoded expression before the function call:
./drivers/net/ethernet/cavium/liquidio/request_manager.c:98:34-59: WARNING: array_size is used down the code (line 103) to compute the same size
./drivers/media/test-drivers/vivid/vivid-core.c:1120:26-34: WARNING: array_size is used down the code (line 1122) to compute the same size
./drivers/scsi/megaraid/megaraid_sas_fusion.c:5184:11-31: WARNING: array_size is used down the code (line 5191) to compute the same size
./drivers/scsi/megaraid/megaraid_sas_fusion.c:5200:2-37: WARNING: array_size is used down the code (line 5207) to compute the same size
./fs/cifs/misc.c:853:17-39: WARNING: array_size is used down the code (line 858) to compute the same size
./fs/cifs/misc.c:863:17-38: WARNING: array_size is used down the code (line 868) to compute the same size
./drivers/scsi/fnic/fnic_trace.c:562:25-48: WARNING: array_size is used down the code (line 566) to compute the same size
opencoded expression after the function call:
./net/ethtool/ioctl.c:1976:55-66: WARNING: array_size is already used (line 1957) to compute the same size
./net/ethtool/ioctl.c:1921:55-66: WARNING: array_size is already used (line 1909) to compute the same size
./drivers/net/ethernet/cavium/liquidio/request_manager.c:111:29-54: WARNING: array_size is already used (line 103) to compute the same size
./drivers/staging/rts5208/ms.c:2309:55-56: WARNING: array_size is already used (line 2305) to compute the same size
./drivers/video/fbdev/core/fbcon.c:642:52-53: WARNING: array3_size is already used (line 638) to compute the same size
./drivers/video/fbdev/core/fbcon.c:679:47-48: WARNING: array3_size is already used (line 638) to compute the same size
./drivers/usb/misc/sisusbvga/sisusb_con.c:1229:54-56: WARNING: array_size is already used (line 1226) to compute the same size
./fs/afs/cmservice.c:271:45-46: WARNING: array3_size is already used (line 267) to compute the same size
./drivers/mtd/ftl.c:270:49-65: WARNING: array_size is already used (line 266) to compute the same size
./drivers/scsi/qla2xxx/tcm_qla2xxx.c:1608:6-42: WARNING: array_size is already used (line 1605) to compute the same size
./drivers/scsi/qla2xxx/tcm_qla2xxx.c:1613:8-44: WARNING: array_size is already used (line 1605) to compute the same size
./drivers/net/ppp/bsd_comp.c:439:13-37: WARNING: array_size is already used (line 409) to compute the same size
./drivers/net/wireless/ath/ath5k/debug.c:957:20-21: WARNING: array_size is already used (line 934) to compute the same size
./drivers/scsi/fnic/fnic_trace.c:575:3-26: WARNING: array_size is already used (line 566) to compute the same size
./drivers/scsi/fnic/fnic_trace.c:592:32-53: WARNING: array_size is already used (line 580) to compute the same size
./drivers/scsi/fnic/fnic_trace.c:504:30-51: WARNING: array_size is already used (line 492) to compute the same size
./drivers/staging/rts5208/rtsx_chip.c:1475:17-18: WARNING: array_size is already used (line 1458) to compute the same size
./kernel/kexec_file.c:917:8-25: WARNING: array_size is already used (line 913) to compute the same size
./drivers/rapidio/devices/rio_mport_cdev.c:984:8-25: WARNING: array_size is already used (line 978) to compute the same size
./fs/reiserfs/bitmap.c:1463:22-37: WARNING: array_size is already used (line 1459) to compute the same size
duplicate calls:
./drivers/media/test-drivers/vivid/vivid-core.c:1125:59-60: WARNING: same array_size (line 1122)
./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:142:36-37: WARNING: same array_size (line 138)
./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:129:41-42: WARNING: same array3_size (line 123)
./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:146:40-41: WARNING: same array3_size (line 123)
./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:146:40-41: WARNING: same array3_size (line 129)
./drivers/net/ethernet/cavium/liquidio/octeon_droq.c:289:27-28: WARNING: same array_size (line 284)
./drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c:857:59-60: WARNING: same struct_size (line 854)
./fs/f2fs/super.c:3478:34-35: WARNING: same array_size (line 3478)
./drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1637:45-46: WARNING: same struct_size (line 1634)
./drivers/net/ethernet/netronome/nfp/flower/cmsg.c:221:49-50: WARNING: same struct_size (line 219)
./drivers/staging/rts5208/rtsx_chip.c:1458:36-37: WARNING: same array_size (line 1454)
./drivers/net/ethernet/neterion/vxge/vxge-config.c:2664:59-60: WARNING: same array_size (line 2654)
On 6/15/20 13:35, Denis Efremov wrote:
>
>
> On 6/15/20 9:23 PM, Kees Cook wrote:
>> On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote:
>>> Detect an opencoded expression that is used before or after
>>> array_size()/array3_size()/struct_size() to compute the same size.
>>>
>>> Cc: Kees Cook <[email protected]>
>>> Signed-off-by: Denis Efremov <[email protected]>
>>
>> Oh, very cool! How much does this find currently?
>>
>
> opencoded expression before the function call:
> ./drivers/net/ethernet/cavium/liquidio/request_manager.c:98:34-59: WARNING: array_size is used down the code (line 103) to compute the same size
> ./drivers/media/test-drivers/vivid/vivid-core.c:1120:26-34: WARNING: array_size is used down the code (line 1122) to compute the same size
> ./drivers/scsi/megaraid/megaraid_sas_fusion.c:5184:11-31: WARNING: array_size is used down the code (line 5191) to compute the same size
> ./drivers/scsi/megaraid/megaraid_sas_fusion.c:5200:2-37: WARNING: array_size is used down the code (line 5207) to compute the same size
> ./fs/cifs/misc.c:853:17-39: WARNING: array_size is used down the code (line 858) to compute the same size
> ./fs/cifs/misc.c:863:17-38: WARNING: array_size is used down the code (line 868) to compute the same size
> ./drivers/scsi/fnic/fnic_trace.c:562:25-48: WARNING: array_size is used down the code (line 566) to compute the same size
>
> opencoded expression after the function call:
> ./net/ethtool/ioctl.c:1976:55-66: WARNING: array_size is already used (line 1957) to compute the same size
> ./net/ethtool/ioctl.c:1921:55-66: WARNING: array_size is already used (line 1909) to compute the same size
> ./drivers/net/ethernet/cavium/liquidio/request_manager.c:111:29-54: WARNING: array_size is already used (line 103) to compute the same size
> ./drivers/staging/rts5208/ms.c:2309:55-56: WARNING: array_size is already used (line 2305) to compute the same size
> ./drivers/video/fbdev/core/fbcon.c:642:52-53: WARNING: array3_size is already used (line 638) to compute the same size
> ./drivers/video/fbdev/core/fbcon.c:679:47-48: WARNING: array3_size is already used (line 638) to compute the same size
> ./drivers/usb/misc/sisusbvga/sisusb_con.c:1229:54-56: WARNING: array_size is already used (line 1226) to compute the same size
> ./fs/afs/cmservice.c:271:45-46: WARNING: array3_size is already used (line 267) to compute the same size
> ./drivers/mtd/ftl.c:270:49-65: WARNING: array_size is already used (line 266) to compute the same size
> ./drivers/scsi/qla2xxx/tcm_qla2xxx.c:1608:6-42: WARNING: array_size is already used (line 1605) to compute the same size
> ./drivers/scsi/qla2xxx/tcm_qla2xxx.c:1613:8-44: WARNING: array_size is already used (line 1605) to compute the same size
> ./drivers/net/ppp/bsd_comp.c:439:13-37: WARNING: array_size is already used (line 409) to compute the same size
> ./drivers/net/wireless/ath/ath5k/debug.c:957:20-21: WARNING: array_size is already used (line 934) to compute the same size
> ./drivers/scsi/fnic/fnic_trace.c:575:3-26: WARNING: array_size is already used (line 566) to compute the same size
> ./drivers/scsi/fnic/fnic_trace.c:592:32-53: WARNING: array_size is already used (line 580) to compute the same size
> ./drivers/scsi/fnic/fnic_trace.c:504:30-51: WARNING: array_size is already used (line 492) to compute the same size
> ./drivers/staging/rts5208/rtsx_chip.c:1475:17-18: WARNING: array_size is already used (line 1458) to compute the same size
> ./kernel/kexec_file.c:917:8-25: WARNING: array_size is already used (line 913) to compute the same size
> ./drivers/rapidio/devices/rio_mport_cdev.c:984:8-25: WARNING: array_size is already used (line 978) to compute the same size
> ./fs/reiserfs/bitmap.c:1463:22-37: WARNING: array_size is already used (line 1459) to compute the same size
>
> duplicate calls:
> ./drivers/media/test-drivers/vivid/vivid-core.c:1125:59-60: WARNING: same array_size (line 1122)
> ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:142:36-37: WARNING: same array_size (line 138)
> ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:129:41-42: WARNING: same array3_size (line 123)
> ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:146:40-41: WARNING: same array3_size (line 123)
> ./drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:146:40-41: WARNING: same array3_size (line 129)
> ./drivers/net/ethernet/cavium/liquidio/octeon_droq.c:289:27-28: WARNING: same array_size (line 284)
> ./drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c:857:59-60: WARNING: same struct_size (line 854)
> ./fs/f2fs/super.c:3478:34-35: WARNING: same array_size (line 3478)
> ./drivers/net/wireless/zydas/zd1211rw/zd_usb.c:1637:45-46: WARNING: same struct_size (line 1634)
> ./drivers/net/ethernet/netronome/nfp/flower/cmsg.c:221:49-50: WARNING: same struct_size (line 219)
> ./drivers/staging/rts5208/rtsx_chip.c:1458:36-37: WARNING: same array_size (line 1454)
> ./drivers/net/ethernet/neterion/vxge/vxge-config.c:2664:59-60: WARNING: same array_size (line 2654)
Awesome! I'll take a look into this. :)
Thanks, Denis!
--
Gustavo
>
>
> Awesome! I'll take a look into this. :)
>
It would be helpful to get a feedback from you after.
What kind of warnings are helpful and what are not?
"duplicate calls" and "opencoded expression after array_size()" look doubtful to me.
I think that maintainers will not like these patches.
Thanks,
Denis
>
> Awesome! I'll take a look into this. :)
>
Here is another script for your #83 ticket.
Currently, it issues 598 warnings.
// SPDX-License-Identifier: GPL-2.0-only
///
/// Check for missing overflow checks in allocation functions.
/// Low confidence because it's pointless to check for overflow
/// relatively small allocations.
///
// Confidence: Low
// Copyright: (C) 2020 Denis Efremov ISPRAS
// Options: --no-includes --include-headers
virtual patch
virtual context
virtual org
virtual report
@depends on patch@
expression E1, E2, E3, E4, size;
@@
(
- size = E1 * E2;
+ size = array_size(E1, E2);
|
- size = E1 * E2 * E3;
+ size = array3_size(E1, E2, E3);
|
- size = E1 * E2 + E3;
+ size = struct_size(E1, E2, E3);
)
... when != size = E4
when != size += E4
when != size -= E4
when != size *= E4
when != &size
\(kmalloc\|krealloc\|kzalloc\|kzalloc_node\|
vmalloc\|vzalloc\|vzalloc_node\|
kvmalloc\|kvzalloc\|kvzalloc_node\|
sock_kmalloc\|
f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\|
devm_kmalloc\|devm_kzalloc\)
(..., size, ...)
@r depends on !patch@
expression E1, E2, E3, E4, size;
position p;
@@
(
* size = E1 * E2;@p
|
* size = E1 * E2 * E3;@p
|
* size = E1 * E2 + E3;@p
)
... when != size = E4
when != size += E4
when != size -= E4
when != size *= E4
when != &size
* \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\|
vmalloc\|vzalloc\|vzalloc_node\|
kvmalloc\|kvzalloc\|kvzalloc_node\|
sock_kmalloc\|
f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\|
devm_kmalloc\|devm_kzalloc\)
(..., size, ...)
@script:python depends on report@
p << r.p;
@@
coccilib.report.print_report(p[0], "WARNING: missing overflow check")
@script:python depends on org@
p << r.p;
@@
coccilib.org.print_todo(p[0], "WARNING: missing overflow check")
On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote:
> +@as@
> +expression E1, E2;
> +@@
> +
> +array_size(E1, E2)
BTW, is there a way yet in Coccinelle to match a fully qualified (?)
identifier? For example, if I have two lines in C:
A)
array_size(variable, 5);
B)
array_size(instance->member.size, 5);
C)
array_size(instance->member.size + 1, 5);
D)
array_size(function_call(variable), 5);
This matches A, B, C, and D:
@@
expression ARG1;
expression ARG2;
@@
array_size(ARG1, ARG2);
This matches only A:
@@
identifier ARG1;
expression ARG2;
@@
array_size(ARG1, ARG2);
How do I get something to match A and B but not C and D (i.e. I do not
want to match any operations, function calls, etc, only a variable,
which may be identified through dereference, array index, or struct
member access.)
--
Kees Cook
On Wed, 17 Jun 2020, Denis Efremov wrote:
>
> >
> > Awesome! I'll take a look into this. :)
> >
> Here is another script for your #83 ticket.
> Currently, it issues 598 warnings.
>
> // SPDX-License-Identifier: GPL-2.0-only
> ///
> /// Check for missing overflow checks in allocation functions.
> /// Low confidence because it's pointless to check for overflow
> /// relatively small allocations.
> ///
> // Confidence: Low
> // Copyright: (C) 2020 Denis Efremov ISPRAS
> // Options: --no-includes --include-headers
>
> virtual patch
> virtual context
> virtual org
> virtual report
>
> @depends on patch@
> expression E1, E2, E3, E4, size;
> @@
>
> (
> - size = E1 * E2;
> + size = array_size(E1, E2);
> |
> - size = E1 * E2 * E3;
> + size = array3_size(E1, E2, E3);
> |
> - size = E1 * E2 + E3;
> + size = struct_size(E1, E2, E3);
Should the arguments be checked to see if they have something to do with
arrays and structures?
> )
> ... when != size = E4
> when != size += E4
> when != size -= E4
> when != size *= E4
Here you can have a metavariable
assignment operator aop;
and then say size aop E4
It doesn't really look like an assignment any more, but it could be a
little safer.
julia
> when != &size
> \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\|
> vmalloc\|vzalloc\|vzalloc_node\|
> kvmalloc\|kvzalloc\|kvzalloc_node\|
> sock_kmalloc\|
> f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\|
> devm_kmalloc\|devm_kzalloc\)
> (..., size, ...)
>
> @r depends on !patch@
> expression E1, E2, E3, E4, size;
> position p;
> @@
>
> (
> * size = E1 * E2;@p
> |
> * size = E1 * E2 * E3;@p
> |
> * size = E1 * E2 + E3;@p
> )
> ... when != size = E4
> when != size += E4
> when != size -= E4
> when != size *= E4
> when != &size
> * \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\|
> vmalloc\|vzalloc\|vzalloc_node\|
> kvmalloc\|kvzalloc\|kvzalloc_node\|
> sock_kmalloc\|
> f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\|
> devm_kmalloc\|devm_kzalloc\)
> (..., size, ...)
>
> @script:python depends on report@
> p << r.p;
> @@
>
> coccilib.report.print_report(p[0], "WARNING: missing overflow check")
>
> @script:python depends on org@
> p << r.p;
> @@
>
> coccilib.org.print_todo(p[0], "WARNING: missing overflow check")
> _______________________________________________
> Cocci mailing list
> [email protected]
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
On Wed, 17 Jun 2020, Julia Lawall wrote:
>
>
> On Wed, 17 Jun 2020, Denis Efremov wrote:
>
> >
> > >
> > > Awesome! I'll take a look into this. :)
> > >
> > Here is another script for your #83 ticket.
> > Currently, it issues 598 warnings.
> >
> > // SPDX-License-Identifier: GPL-2.0-only
> > ///
> > /// Check for missing overflow checks in allocation functions.
> > /// Low confidence because it's pointless to check for overflow
> > /// relatively small allocations.
> > ///
> > // Confidence: Low
> > // Copyright: (C) 2020 Denis Efremov ISPRAS
> > // Options: --no-includes --include-headers
> >
> > virtual patch
> > virtual context
> > virtual org
> > virtual report
> >
> > @depends on patch@
> > expression E1, E2, E3, E4, size;
> > @@
> >
> > (
> > - size = E1 * E2;
> > + size = array_size(E1, E2);
> > |
> > - size = E1 * E2 * E3;
> > + size = array3_size(E1, E2, E3);
> > |
> > - size = E1 * E2 + E3;
> > + size = struct_size(E1, E2, E3);
>
> Should the arguments be checked to see if they have something to do with
> arrays and structures?
Sorry for the noise, I see that this comment makes no sense.
julia
>
> > )
> > ... when != size = E4
> > when != size += E4
> > when != size -= E4
> > when != size *= E4
>
> Here you can have a metavariable
>
> assignment operator aop;
>
> and then say size aop E4
>
> It doesn't really look like an assignment any more, but it could be a
> little safer.
>
> julia
>
> > when != &size
> > \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\|
> > vmalloc\|vzalloc\|vzalloc_node\|
> > kvmalloc\|kvzalloc\|kvzalloc_node\|
> > sock_kmalloc\|
> > f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\|
> > devm_kmalloc\|devm_kzalloc\)
> > (..., size, ...)
> >
> > @r depends on !patch@
> > expression E1, E2, E3, E4, size;
> > position p;
> > @@
> >
> > (
> > * size = E1 * E2;@p
> > |
> > * size = E1 * E2 * E3;@p
> > |
> > * size = E1 * E2 + E3;@p
> > )
> > ... when != size = E4
> > when != size += E4
> > when != size -= E4
> > when != size *= E4
> > when != &size
> > * \(kmalloc\|krealloc\|kzalloc\|kzalloc_node\|
> > vmalloc\|vzalloc\|vzalloc_node\|
> > kvmalloc\|kvzalloc\|kvzalloc_node\|
> > sock_kmalloc\|
> > f2fs_kmalloc\|f2fs_kzalloc\|f2fs_kvmalloc\|f2fs_kvzalloc\|
> > devm_kmalloc\|devm_kzalloc\)
> > (..., size, ...)
> >
> > @script:python depends on report@
> > p << r.p;
> > @@
> >
> > coccilib.report.print_report(p[0], "WARNING: missing overflow check")
> >
> > @script:python depends on org@
> > p << r.p;
> > @@
> >
> > coccilib.org.print_todo(p[0], "WARNING: missing overflow check")
> > _______________________________________________
> > Cocci mailing list
> > [email protected]
> > https://systeme.lip6.fr/mailman/listinfo/cocci
> >
> _______________________________________________
> Cocci mailing list
> [email protected]
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
On Mon, 15 Jun 2020, Denis Efremov wrote:
> Detect an opencoded expression that is used before or after
> array_size()/array3_size()/struct_size() to compute the same size.
This would benefit from the assignemnt operator metavariables as well.
Also, it could be better to put the python rules up next the SmPL pattern
matching rules that they are associated with.
julia
>
> Cc: Kees Cook <[email protected]>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
> scripts/coccinelle/misc/array_size_dup.cocci | 347 +++++++++++++++++++
> 1 file changed, 347 insertions(+)
> create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci
>
> diff --git a/scripts/coccinelle/misc/array_size_dup.cocci b/scripts/coccinelle/misc/array_size_dup.cocci
> new file mode 100644
> index 000000000000..08919a938754
> --- /dev/null
> +++ b/scripts/coccinelle/misc/array_size_dup.cocci
> @@ -0,0 +1,347 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Check for array_size(), array3_size(), struct_size() duplicates.
> +/// Three types of patterns for these functions:
> +/// 1. An opencoded expression is used before array_size() to compute the same size
> +/// 2. An opencoded expression is used after array_size() to compute the same size
> +/// 3. Consecutive calls of array_size() with the same values
> +/// From security point of view only first case is relevant. These functions
> +/// perform arithmetic overflow check. Thus, if we use an opencoded expression
> +/// before a call to the *_size() function we can miss an overflow.
> +///
> +// Confidence: High
> +// Copyright: (C) 2020 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers --no-loops
> +
> +virtual context
> +virtual report
> +virtual org
> +
> +@as@
> +expression E1, E2;
> +@@
> +
> +array_size(E1, E2)
> +
> +@as_next@
> +expression subE1 <= as.E1;
> +expression as.E1;
> +expression subE2 <= as.E2;
> +expression as.E2;
> +expression E3;
> +position p1, p2;
> +@@
> +
> +* E1 * E2@p1
> + ... when != \(E1\|E2\|subE1\|subE2\)=E3
> + when != \(E1\|E2\|subE1\|subE2\)+=E3
> + when != \(E1\|E2\|subE1\|subE2\)-=E3
> + when != \(E1\|E2\|subE1\|subE2\)*=E3
> + when != \(&E1\|&E2\|&subE1\|&subE2\)
> +* array_size(E1, E2)@p2
> +
> +@as_prev@
> +expression subE1 <= as.E1;
> +expression as.E1;
> +expression subE2 <= as.E2;
> +expression as.E2;
> +expression E3;
> +position p1, p2;
> +@@
> +
> +* array_size(E1, E2)@p1
> + ... when != \(E1\|E2\|subE1\|subE2\)=E3
> + when != \(E1\|E2\|subE1\|subE2\)+=E3
> + when != \(E1\|E2\|subE1\|subE2\)-=E3
> + when != \(E1\|E2\|subE1\|subE2\)*=E3
> + when != \(&E1\|&E2\|&subE1\|&subE2\)
> +* E1 * E2@p2
> +
> +@as_dup@
> +expression subE1 <= as.E1;
> +expression as.E1;
> +expression subE2 <= as.E2;
> +expression as.E2;
> +expression E3;
> +position p1, p2;
> +@@
> +
> +* array_size(E1, E2)@p1
> + ... when != \(E1\|E2\|subE1\|subE2\)=E3
> + when != \(E1\|E2\|subE1\|subE2\)+=E3
> + when != \(E1\|E2\|subE1\|subE2\)-=E3
> + when != \(E1\|E2\|subE1\|subE2\)*=E3
> + when != \(&E1\|&E2\|&subE1\|&subE2\)
> +* array_size(E1, E2)@p2
> +
> +@as3@
> +expression E1, E2, E3;
> +@@
> +
> +array3_size(E1, E2, E3)
> +
> +@as3_next@
> +expression subE1 <= as3.E1;
> +expression as3.E1;
> +expression subE2 <= as3.E2;
> +expression as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* E1 * E2 * E3@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> + when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* array3_size(E1, E2, E3)@p2
> +
> +@as3_prev@
> +expression subE1 <= as3.E1;
> +expression as3.E1;
> +expression subE2 <= as3.E2;
> +expression as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* array3_size(E1, E2, E3)@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> + when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* E1 * E2 * E3@p2
> +
> +@as3_dup@
> +expression subE1 <= as3.E1;
> +expression as3.E1;
> +expression subE2 <= as3.E2;
> +expression as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* array3_size(E1, E2, E3)@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> + when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* array3_size(E1, E2, E3)@p2
> +
> +@ss@
> +expression E1, E2, E3;
> +@@
> +
> +struct_size(E1, E2, E3)
> +
> +@ss_next@
> +expression subE1 <= ss.E1;
> +expression ss.E1;
> +expression subE2 <= ss.E2;
> +expression ss.E2;
> +expression subE3 <= ss.E3;
> +expression ss.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* E1 * E2 + E3@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> + when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* struct_size(E1, E2, E3)@p2
> +
> +@ss_prev@
> +expression subE1 <= ss.E1;
> +expression ss.E1;
> +expression subE2 <= ss.E2;
> +expression ss.E2;
> +expression subE3 <= ss.E3;
> +expression ss.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* struct_size(E1, E2, E3)@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> + when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* E1 * E2 + E3@p2
> +
> +@ss_dup@
> +expression subE1 <= ss.E1;
> +expression ss.E1;
> +expression subE2 <= ss.E2;
> +expression ss.E2;
> +expression subE3 <= ss.E3;
> +expression ss.E3;
> +expression E4;
> +position p1, p2;
> +@@
> +
> +* struct_size(E1, E2, E3)@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)+=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)-=E4
> + when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\)*=E4
> + when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
> +* struct_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << as_next.p1;
> +p2 << as_next.p2;
> +@@
> +
> +msg = "WARNING: array_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p1 << as_next.p1;
> +p2 << as_next.p2;
> +@@
> +
> +msg = "WARNING: array_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.org.print_todo(p1[0], msg)
> +
> +@script:python depends on report@
> +p1 << as_prev.p1;
> +p2 << as_prev.p2;
> +@@
> +
> +msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << as_prev.p1;
> +p2 << as_prev.p2;
> +@@
> +
> +msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +@script:python depends on report@
> +p1 << as_dup.p1;
> +p2 << as_dup.p2;
> +@@
> +
> +msg = "WARNING: same array_size (line %s)" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << as_dup.p1;
> +p2 << as_dup.p2;
> +@@
> +
> +msg = "WARNING: same array_size (line %s)" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +
> +@script:python depends on report@
> +p1 << as3_next.p1;
> +p2 << as3_next.p2;
> +@@
> +
> +msg = "WARNING: array3_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p1 << as3_next.p1;
> +p2 << as3_next.p2;
> +@@
> +
> +msg = "WARNING: array3_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.org.print_todo(p1[0], msg)
> +
> +@script:python depends on report@
> +p1 << as3_prev.p1;
> +p2 << as3_prev.p2;
> +@@
> +
> +msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << as3_prev.p1;
> +p2 << as3_prev.p2;
> +@@
> +
> +msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +@script:python depends on report@
> +p1 << as3_dup.p1;
> +p2 << as3_dup.p2;
> +@@
> +
> +msg = "WARNING: same array3_size (line %s)" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << as3_dup.p1;
> +p2 << as3_dup.p2;
> +@@
> +
> +msg = "WARNING: same array3_size (line %s)" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +
> +@script:python depends on report@
> +p1 << ss_next.p1;
> +p2 << ss_next.p2;
> +@@
> +
> +msg = "WARNING: struct_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p1 << ss_next.p1;
> +p2 << ss_next.p2;
> +@@
> +
> +msg = "WARNING: struct_size is used down the code (line %s) to compute the same size" % (p2[0].line)
> +coccilib.org.print_todo(p1[0], msg)
> +
> +@script:python depends on report@
> +p1 << ss_prev.p1;
> +p2 << ss_prev.p2;
> +@@
> +
> +msg = "WARNING: struct_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << ss_prev.p1;
> +p2 << ss_prev.p2;
> +@@
> +
> +msg = "WARNING: struct_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +@script:python depends on report@
> +p1 << ss_dup.p1;
> +p2 << ss_dup.p2;
> +@@
> +
> +msg = "WARNING: same struct_size (line %s)" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << ss_dup.p1;
> +p2 << ss_dup.p2;
> +@@
> +
> +msg = "WARNING: same struct_size (line %s)" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> --
> 2.26.2
>
> _______________________________________________
> Cocci mailing list
> [email protected]
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
On 6/17/20 11:30 PM, Julia Lawall wrote:
>
>
> On Mon, 15 Jun 2020, Denis Efremov wrote:
>
>> Detect an opencoded expression that is used before or after
>> array_size()/array3_size()/struct_size() to compute the same size.
>
> This would benefit from the assignemnt operator metavariables as well.
>
> Also, it could be better to put the python rules up next the SmPL pattern
> matching rules that they are associated with.
>
Thanks, I will send v2.
Here is the KSPP ticket with patches https://github.com/KSPP/linux/issues/83
On Wed, 17 Jun 2020, Denis Efremov wrote:
>
>
> On 6/17/20 11:30 PM, Julia Lawall wrote:
> >
> >
> > On Mon, 15 Jun 2020, Denis Efremov wrote:
> >
> >> Detect an opencoded expression that is used before or after
> >> array_size()/array3_size()/struct_size() to compute the same size.
> >
> > This would benefit from the assignemnt operator metavariables as well.
> >
> > Also, it could be better to put the python rules up next the SmPL pattern
> > matching rules that they are associated with.
> >
>
> Thanks, I will send v2.
> Here is the KSPP ticket with patches https://github.com/KSPP/linux/issues/83
Thanks!
julia
Detect an opencoded expression that is used before or after
array_size()/array3_size()/struct_size() to compute the same size.
Cc: Gustavo A. R. Silva <[email protected]>
Cc: Kees Cook <[email protected]>
Signed-off-by: Denis Efremov <[email protected]>
---
Changes in v2:
- python rules moved next to SmPL patterns
- assignment operator used
- struct_size patterns fixed to check only E3, since
E1, E2 are sizeofs of a structure and a member
of a structure
scripts/coccinelle/misc/array_size_dup.cocci | 309 +++++++++++++++++++
1 file changed, 309 insertions(+)
create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci
diff --git a/scripts/coccinelle/misc/array_size_dup.cocci b/scripts/coccinelle/misc/array_size_dup.cocci
new file mode 100644
index 000000000000..c5214310285c
--- /dev/null
+++ b/scripts/coccinelle/misc/array_size_dup.cocci
@@ -0,0 +1,309 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for array_size(), array3_size(), struct_size() duplicates.
+/// Three types of patterns for these functions:
+/// 1. An opencoded expression is used before array_size() to compute the same size
+/// 2. An opencoded expression is used after array_size() to compute the same size
+/// 3. Consecutive calls of array_size() with the same values
+/// From security point of view only first case is relevant. These functions
+/// perform arithmetic overflow check. Thus, if we use an opencoded expression
+/// before a call to the *_size() function we can miss an overflow.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers --no-loops
+
+virtual context
+virtual report
+virtual org
+
+@as@
+expression E1, E2;
+@@
+
+array_size(E1, E2)
+
+@as_next@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != \(&E1\|&E2\|&subE1\|&subE2\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: array_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: array_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@as_prev@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != \(&E1\|&E2\|&subE1\|&subE2\)
+* E1 * E2@p2
+
+@script:python depends on report@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: array_size is already used (line {p1[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: array_size is already used (line {p1[0].line}) to compute \
+the same size")
+
+@as_dup@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != \(&E1\|&E2\|&subE1\|&subE2\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: same array_size (line {p1[0].line})")
+
+@script:python depends on org@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: same array_size (line {p1[0].line})")
+
+@as3@
+expression E1, E2, E3;
+@@
+
+array3_size(E1, E2, E3)
+
+@as3_next@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2 * E3@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
+* array3_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@as3_prev@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array3_size(E1, E2, E3)@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
+* E1 * E2 * E3@p2
+
+@script:python depends on report@
+p1 << as3_prev.p1;
+p2 << as3_prev.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: array3_size is already used (line {p1[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << as3_prev.p1;
+p2 << as3_prev.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: array3_size is already used (line {p1[0].line}) to compute \
+the same size")
+
+@as3_dup@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array3_size(E1, E2, E3)@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != \(&E1\|&E2\|&E3\|&subE1\|&subE2\|&subE3\)
+* array3_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << as3_dup.p1;
+p2 << as3_dup.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: same array3_size (line {p1[0].line})")
+
+@script:python depends on org@
+p1 << as3_dup.p1;
+p2 << as3_dup.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: same array3_size (line {p1[0].line})")
+
+@ss@
+expression E1, E2, E3;
+@@
+
+struct_size(E1, E2, E3)
+
+@ss_next@
+expression subE3 <= ss.E3;
+expression ss.E1, ss.E2, ss.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2 + E3@p1
+ ... when != \(E3\|subE3\) aop E4
+ when != \(&E3\|&subE3\)
+* struct_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << ss_next.p1;
+p2 << ss_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: struct_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << ss_next.p1;
+p2 << ss_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: struct_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@ss_prev@
+expression subE3 <= ss.E3;
+expression ss.E1, ss.E2, ss.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* struct_size(E1, E2, E3)@p1
+ ... when != \(E3\|subE3\) aop E4
+ when != \(&E3\|&subE3\)
+* E1 * E2 + E3@p2
+
+@script:python depends on report@
+p1 << ss_prev.p1;
+p2 << ss_prev.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: struct_size is already used (line {p1[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << ss_prev.p1;
+p2 << ss_prev.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: struct_size is already used (line {p1[0].line}) to compute \
+the same size")
+
+@ss_dup@
+expression subE3 <= ss.E3;
+expression ss.E1, ss.E2, ss.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* struct_size(E1, E2, E3)@p1
+ ... when != \(E3\|subE3\) aop E4
+ when != \(&E3\|&subE3\)
+* struct_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << ss_dup.p1;
+p2 << ss_dup.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: same struct_size (line {p1[0].line})")
+
+@script:python depends on org@
+p1 << ss_dup.p1;
+p2 << ss_dup.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: same struct_size (line {p1[0].line})")
--
2.26.2
Detect an opencoded expression that is used before or after
array_size()/array3_size()/struct_size() to compute the same size.
Cc: Gustavo A. R. Silva <[email protected]>
Cc: Kees Cook <[email protected]>
Signed-off-by: Denis Efremov <[email protected]>
---
Changes in v2:
- python rules moved next to SmPL patterns
- assignment operator used
- struct_size patterns fixed to check only E3, since
E1, E2 are sizeofs of a structure and a member
of a structure
Changes in v3:
- s/overlow/overflow/ typo fixed (thanks, Markus)
- \(&E1\|&E2\) changed to &\(E1\|E2\)
- print strings breaks removed
scripts/coccinelle/misc/array_size_dup.cocci | 297 +++++++++++++++++++
1 file changed, 297 insertions(+)
create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci
diff --git a/scripts/coccinelle/misc/array_size_dup.cocci b/scripts/coccinelle/misc/array_size_dup.cocci
new file mode 100644
index 000000000000..d03740257e97
--- /dev/null
+++ b/scripts/coccinelle/misc/array_size_dup.cocci
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for array_size(), array3_size(), struct_size() duplicates.
+/// Three types of patterns for these functions:
+/// 1. An opencoded expression is used before array_size() to compute the same size
+/// 2. An opencoded expression is used after array_size() to compute the same size
+/// 3. Consecutive calls of array_size() with the same values
+/// From security point of view only first case is relevant. These functions
+/// perform arithmetic overflow check. Thus, if we use an opencoded expression
+/// before a call to the *_size() function we can miss an overflow.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers --no-loops
+
+virtual context
+virtual report
+virtual org
+
+@as@
+expression E1, E2;
+@@
+
+array_size(E1, E2)
+
+@as_next@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != &\(E1\|E2\|subE1\|subE2\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: array_size is used down the code (line {p2[0].line}) to compute the same size")
+
+@script:python depends on org@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: array_size is used down the code (line {p2[0].line}) to compute the same size")
+
+@as_prev@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != &\(E1\|E2\|subE1\|subE2\)
+* E1 * E2@p2
+
+@script:python depends on report@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: array_size is already used (line {p1[0].line}) to compute the same size")
+
+@script:python depends on org@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: array_size is already used (line {p1[0].line}) to compute the same size")
+
+@as_dup@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != &\(E1\|E2\|subE1\|subE2\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: same array_size (line {p1[0].line})")
+
+@script:python depends on org@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: same array_size (line {p1[0].line})")
+
+@as3@
+expression E1, E2, E3;
+@@
+
+array3_size(E1, E2, E3)
+
+@as3_next@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2 * E3@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != &\(E1\|E2\|E3\|subE1\|subE2\|subE3\)
+* array3_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute the same size")
+
+@script:python depends on org@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute the same size")
+
+@as3_prev@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array3_size(E1, E2, E3)@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != &\(E1\|E2\|E3\|subE1\|subE2\|subE3\)
+* E1 * E2 * E3@p2
+
+@script:python depends on report@
+p1 << as3_prev.p1;
+p2 << as3_prev.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: array3_size is already used (line {p1[0].line}) to compute the same size")
+
+@script:python depends on org@
+p1 << as3_prev.p1;
+p2 << as3_prev.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: array3_size is already used (line {p1[0].line}) to compute the same size")
+
+@as3_dup@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array3_size(E1, E2, E3)@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != &\(E1\|E2\|E3\|subE1\|subE2\|subE3\)
+* array3_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << as3_dup.p1;
+p2 << as3_dup.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: same array3_size (line {p1[0].line})")
+
+@script:python depends on org@
+p1 << as3_dup.p1;
+p2 << as3_dup.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: same array3_size (line {p1[0].line})")
+
+@ss@
+expression E1, E2, E3;
+@@
+
+struct_size(E1, E2, E3)
+
+@ss_next@
+expression subE3 <= ss.E3;
+expression ss.E1, ss.E2, ss.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2 + E3@p1
+ ... when != \(E3\|subE3\) aop E4
+ when != &\(E3\|subE3\)
+* struct_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << ss_next.p1;
+p2 << ss_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: struct_size is used down the code (line {p2[0].line}) to compute the same size")
+
+@script:python depends on org@
+p1 << ss_next.p1;
+p2 << ss_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: struct_size is used down the code (line {p2[0].line}) to compute the same size")
+
+@ss_prev@
+expression subE3 <= ss.E3;
+expression ss.E1, ss.E2, ss.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* struct_size(E1, E2, E3)@p1
+ ... when != \(E3\|subE3\) aop E4
+ when != &\(E3\|subE3\)
+* E1 * E2 + E3@p2
+
+@script:python depends on report@
+p1 << ss_prev.p1;
+p2 << ss_prev.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: struct_size is already used (line {p1[0].line}) to compute the same size")
+
+@script:python depends on org@
+p1 << ss_prev.p1;
+p2 << ss_prev.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: struct_size is already used (line {p1[0].line}) to compute the same size")
+
+@ss_dup@
+expression subE3 <= ss.E3;
+expression ss.E1, ss.E2, ss.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* struct_size(E1, E2, E3)@p1
+ ... when != \(E3\|subE3\) aop E4
+ when != &\(E3\|subE3\)
+* struct_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << ss_dup.p1;
+p2 << ss_dup.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: same struct_size (line {p1[0].line})")
+
+@script:python depends on org@
+p1 << ss_dup.p1;
+p2 << ss_dup.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: same struct_size (line {p1[0].line})")
--
2.26.2
> Changes in v2:
…
> - assignment operator used
I would prefer the distinction for the application of corresponding metavariables.
> Changes in v3:
…
> - \(&E1\|&E2\) changed to &\(E1\|E2\)
Would it be more helpful to mention the movement of the ampersand
before SmPL disjunctions?
…
> +virtual context
> +virtual report
> +virtual org
Can the following SmPL code variant become more attractive?
+virtual context, report, org
…
> +expression subE1 <= as.E1;
> +expression subE2 <= as.E2;
> +expression as.E1, as.E2, E3;
How do you think about the following SmPL code variant?
+expression subE1 <= as.E1,
+ subE2 <= as.E2,
+ as.E1,
+ as.E2,
+ E3;
…
> +coccilib.report.print_report(p2[0],
> +f"WARNING: same struct_size (line {p1[0].line})")
Please align such function parameters.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=5e857ce6eae7ca21b2055cca4885545e29228fe2#n93
+coccilib.report.print_report(p2[0],
+ f"WARNING: same struct_size (line {p1[0].line})")
Regards,
Markus
> +@as_next@
> +expression subE1 <= as.E1;
> +expression subE2 <= as.E2;
> +expression as.E1, as.E2, E3;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* E1 * E2@p1
> + ... when != \(E1\|E2\|subE1\|subE2\) aop E3
> + when != &\(E1\|E2\|subE1\|subE2\)
You don't need E1 and E2 in the above two lines. subE1 and subE2 will
match them. Likewise for the other rules.
> +* array_size(E1, E2)@p2
> +
> +@script:python depends on report@
> +p1 << as_next.p1;
> +p2 << as_next.p2;
> +@@
> +
> +coccilib.report.print_report(p1[0],
> +f"WARNING: array_size is used down the code (line {p2[0].line}) to compute the same size")
I don't think that "down the code" is very understandable. "Later in the
code" would be fine. Even just "later" would be fine.
julia
> +@script:python depends on org@
> +p1 << as_next.p1;
> +p2 << as_next.p2;
> +@@
> +
> +coccilib.org.print_todo(p1[0],
> +f"WARNING: array_size is used down the code (line {p2[0].line}) to compute the same size")
> +
> +@as_prev@
> +expression subE1 <= as.E1;
> +expression subE2 <= as.E2;
> +expression as.E1, as.E2, E3;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* array_size(E1, E2)@p1
> + ... when != \(E1\|E2\|subE1\|subE2\) aop E3
> + when != &\(E1\|E2\|subE1\|subE2\)
> +* E1 * E2@p2
> +
> +@script:python depends on report@
> +p1 << as_prev.p1;
> +p2 << as_prev.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: array_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@script:python depends on org@
> +p1 << as_prev.p1;
> +p2 << as_prev.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: array_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@as_dup@
> +expression subE1 <= as.E1;
> +expression subE2 <= as.E2;
> +expression as.E1, as.E2, E3;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* array_size(E1, E2)@p1
> + ... when != \(E1\|E2\|subE1\|subE2\) aop E3
> + when != &\(E1\|E2\|subE1\|subE2\)
> +* array_size(E1, E2)@p2
> +
> +@script:python depends on report@
> +p1 << as_dup.p1;
> +p2 << as_dup.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: same array_size (line {p1[0].line})")
> +
> +@script:python depends on org@
> +p1 << as_dup.p1;
> +p2 << as_dup.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: same array_size (line {p1[0].line})")
> +
> +@as3@
> +expression E1, E2, E3;
> +@@
> +
> +array3_size(E1, E2, E3)
> +
> +@as3_next@
> +expression subE1 <= as3.E1;
> +expression subE2 <= as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E1, as3.E2, as3.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* E1 * E2 * E3@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
> + when != &\(E1\|E2\|E3\|subE1\|subE2\|subE3\)
> +* array3_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << as3_next.p1;
> +p2 << as3_next.p2;
> +@@
> +
> +coccilib.report.print_report(p1[0],
> +f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute the same size")
> +
> +@script:python depends on org@
> +p1 << as3_next.p1;
> +p2 << as3_next.p2;
> +@@
> +
> +coccilib.org.print_todo(p1[0],
> +f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute the same size")
> +
> +@as3_prev@
> +expression subE1 <= as3.E1;
> +expression subE2 <= as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E1, as3.E2, as3.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* array3_size(E1, E2, E3)@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
> + when != &\(E1\|E2\|E3\|subE1\|subE2\|subE3\)
> +* E1 * E2 * E3@p2
> +
> +@script:python depends on report@
> +p1 << as3_prev.p1;
> +p2 << as3_prev.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: array3_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@script:python depends on org@
> +p1 << as3_prev.p1;
> +p2 << as3_prev.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: array3_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@as3_dup@
> +expression subE1 <= as3.E1;
> +expression subE2 <= as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E1, as3.E2, as3.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* array3_size(E1, E2, E3)@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
> + when != &\(E1\|E2\|E3\|subE1\|subE2\|subE3\)
> +* array3_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << as3_dup.p1;
> +p2 << as3_dup.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: same array3_size (line {p1[0].line})")
> +
> +@script:python depends on org@
> +p1 << as3_dup.p1;
> +p2 << as3_dup.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: same array3_size (line {p1[0].line})")
> +
> +@ss@
> +expression E1, E2, E3;
> +@@
> +
> +struct_size(E1, E2, E3)
> +
> +@ss_next@
> +expression subE3 <= ss.E3;
> +expression ss.E1, ss.E2, ss.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* E1 * E2 + E3@p1
> + ... when != \(E3\|subE3\) aop E4
> + when != &\(E3\|subE3\)
> +* struct_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << ss_next.p1;
> +p2 << ss_next.p2;
> +@@
> +
> +coccilib.report.print_report(p1[0],
> +f"WARNING: struct_size is used down the code (line {p2[0].line}) to compute the same size")
> +
> +@script:python depends on org@
> +p1 << ss_next.p1;
> +p2 << ss_next.p2;
> +@@
> +
> +coccilib.org.print_todo(p1[0],
> +f"WARNING: struct_size is used down the code (line {p2[0].line}) to compute the same size")
> +
> +@ss_prev@
> +expression subE3 <= ss.E3;
> +expression ss.E1, ss.E2, ss.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* struct_size(E1, E2, E3)@p1
> + ... when != \(E3\|subE3\) aop E4
> + when != &\(E3\|subE3\)
> +* E1 * E2 + E3@p2
> +
> +@script:python depends on report@
> +p1 << ss_prev.p1;
> +p2 << ss_prev.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: struct_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@script:python depends on org@
> +p1 << ss_prev.p1;
> +p2 << ss_prev.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: struct_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@ss_dup@
> +expression subE3 <= ss.E3;
> +expression ss.E1, ss.E2, ss.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* struct_size(E1, E2, E3)@p1
> + ... when != \(E3\|subE3\) aop E4
> + when != &\(E3\|subE3\)
> +* struct_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << ss_dup.p1;
> +p2 << ss_dup.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: same struct_size (line {p1[0].line})")
> +
> +@script:python depends on org@
> +p1 << ss_dup.p1;
> +p2 << ss_dup.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: same struct_size (line {p1[0].line})")
> --
> 2.26.2
>
> _______________________________________________
> Cocci mailing list
> [email protected]
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
> +@script:python depends on report@
> +p1 << as_next.p1;
> +p2 << as_next.p2;
> +@@
> +
> +coccilib.report.print_report(p1[0],
> +f"WARNING: array_size is used down the code (line {p2[0].line}) to compute the same size")
I get python failures for all of these messages. I know nothing about
python. How is this supposed to work? Is it a python 2 vs python 3
thing?
julia
> +
> +@script:python depends on org@
> +p1 << as_next.p1;
> +p2 << as_next.p2;
> +@@
> +
> +coccilib.org.print_todo(p1[0],
> +f"WARNING: array_size is used down the code (line {p2[0].line}) to compute the same size")
> +
> +@as_prev@
> +expression subE1 <= as.E1;
> +expression subE2 <= as.E2;
> +expression as.E1, as.E2, E3;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* array_size(E1, E2)@p1
> + ... when != \(E1\|E2\|subE1\|subE2\) aop E3
> + when != &\(E1\|E2\|subE1\|subE2\)
> +* E1 * E2@p2
> +
> +@script:python depends on report@
> +p1 << as_prev.p1;
> +p2 << as_prev.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: array_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@script:python depends on org@
> +p1 << as_prev.p1;
> +p2 << as_prev.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: array_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@as_dup@
> +expression subE1 <= as.E1;
> +expression subE2 <= as.E2;
> +expression as.E1, as.E2, E3;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* array_size(E1, E2)@p1
> + ... when != \(E1\|E2\|subE1\|subE2\) aop E3
> + when != &\(E1\|E2\|subE1\|subE2\)
> +* array_size(E1, E2)@p2
> +
> +@script:python depends on report@
> +p1 << as_dup.p1;
> +p2 << as_dup.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: same array_size (line {p1[0].line})")
> +
> +@script:python depends on org@
> +p1 << as_dup.p1;
> +p2 << as_dup.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: same array_size (line {p1[0].line})")
> +
> +@as3@
> +expression E1, E2, E3;
> +@@
> +
> +array3_size(E1, E2, E3)
> +
> +@as3_next@
> +expression subE1 <= as3.E1;
> +expression subE2 <= as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E1, as3.E2, as3.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* E1 * E2 * E3@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
> + when != &\(E1\|E2\|E3\|subE1\|subE2\|subE3\)
> +* array3_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << as3_next.p1;
> +p2 << as3_next.p2;
> +@@
> +
> +coccilib.report.print_report(p1[0],
> +f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute the same size")
> +
> +@script:python depends on org@
> +p1 << as3_next.p1;
> +p2 << as3_next.p2;
> +@@
> +
> +coccilib.org.print_todo(p1[0],
> +f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute the same size")
> +
> +@as3_prev@
> +expression subE1 <= as3.E1;
> +expression subE2 <= as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E1, as3.E2, as3.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* array3_size(E1, E2, E3)@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
> + when != &\(E1\|E2\|E3\|subE1\|subE2\|subE3\)
> +* E1 * E2 * E3@p2
> +
> +@script:python depends on report@
> +p1 << as3_prev.p1;
> +p2 << as3_prev.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: array3_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@script:python depends on org@
> +p1 << as3_prev.p1;
> +p2 << as3_prev.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: array3_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@as3_dup@
> +expression subE1 <= as3.E1;
> +expression subE2 <= as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E1, as3.E2, as3.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* array3_size(E1, E2, E3)@p1
> + ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
> + when != &\(E1\|E2\|E3\|subE1\|subE2\|subE3\)
> +* array3_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << as3_dup.p1;
> +p2 << as3_dup.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: same array3_size (line {p1[0].line})")
> +
> +@script:python depends on org@
> +p1 << as3_dup.p1;
> +p2 << as3_dup.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: same array3_size (line {p1[0].line})")
> +
> +@ss@
> +expression E1, E2, E3;
> +@@
> +
> +struct_size(E1, E2, E3)
> +
> +@ss_next@
> +expression subE3 <= ss.E3;
> +expression ss.E1, ss.E2, ss.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* E1 * E2 + E3@p1
> + ... when != \(E3\|subE3\) aop E4
> + when != &\(E3\|subE3\)
> +* struct_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << ss_next.p1;
> +p2 << ss_next.p2;
> +@@
> +
> +coccilib.report.print_report(p1[0],
> +f"WARNING: struct_size is used down the code (line {p2[0].line}) to compute the same size")
> +
> +@script:python depends on org@
> +p1 << ss_next.p1;
> +p2 << ss_next.p2;
> +@@
> +
> +coccilib.org.print_todo(p1[0],
> +f"WARNING: struct_size is used down the code (line {p2[0].line}) to compute the same size")
> +
> +@ss_prev@
> +expression subE3 <= ss.E3;
> +expression ss.E1, ss.E2, ss.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* struct_size(E1, E2, E3)@p1
> + ... when != \(E3\|subE3\) aop E4
> + when != &\(E3\|subE3\)
> +* E1 * E2 + E3@p2
> +
> +@script:python depends on report@
> +p1 << ss_prev.p1;
> +p2 << ss_prev.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: struct_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@script:python depends on org@
> +p1 << ss_prev.p1;
> +p2 << ss_prev.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: struct_size is already used (line {p1[0].line}) to compute the same size")
> +
> +@ss_dup@
> +expression subE3 <= ss.E3;
> +expression ss.E1, ss.E2, ss.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* struct_size(E1, E2, E3)@p1
> + ... when != \(E3\|subE3\) aop E4
> + when != &\(E3\|subE3\)
> +* struct_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << ss_dup.p1;
> +p2 << ss_dup.p2;
> +@@
> +
> +coccilib.report.print_report(p2[0],
> +f"WARNING: same struct_size (line {p1[0].line})")
> +
> +@script:python depends on org@
> +p1 << ss_dup.p1;
> +p2 << ss_dup.p2;
> +@@
> +
> +coccilib.org.print_todo(p2[0],
> +f"WARNING: same struct_size (line {p1[0].line})")
> --
> 2.26.2
>
> _______________________________________________
> Cocci mailing list
> [email protected]
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
On 6/21/20 11:56 PM, Julia Lawall wrote:
> Is it a python 2 vs python 3 thing?
Yes, python2 is no longer supported and I
thought it would be safe to use this syntax.
Ok, I will make it portable in v4.
Denis
What do you think about removing duplicates warning from the rule?
I mean this kind of warnings: "WARNING: same array_size (line {p1[0].line})"
As for now, I think it's better to not disturb developers with this kind
of things.
Thanks,
Denis
>> +@as_dup@
>> +expression subE1 <= as.E1;
>> +expression subE2 <= as.E2;
>> +expression as.E1, as.E2, E3;
>> +assignment operator aop;
>> +position p1, p2;
>> +@@
>> +
>> +* array_size(E1, E2)@p1
>> + ... when != \(E1\|E2\|subE1\|subE2\) aop E3
>> + when != &\(E1\|E2\|subE1\|subE2\)
>> +* array_size(E1, E2)@p2
>> +
>> +@script:python depends on report@
>> +p1 << as_dup.p1;
>> +p2 << as_dup.p2;
>> +@@
>> +
>> +coccilib.report.print_report(p2[0],
>> +f"WARNING: same array_size (line {p1[0].line})")
>> +
>> +@script:python depends on org@
>> +p1 << as_dup.p1;
>> +p2 << as_dup.p2;
>> +@@
>> +
>> +coccilib.org.print_todo(p2[0],
>> +f"WARNING: same array_size (line {p1[0].line})")
>> +
On Mon, 22 Jun 2020, Denis Efremov wrote:
> What do you think about removing duplicates warning from the rule?
>
> I mean this kind of warnings: "WARNING: same array_size (line {p1[0].line})"
>
> As for now, I think it's better to not disturb developers with this kind
> of things.
I agree. I guess there is a small overhead for the test, but if the code
is not performance critical, being simpler is probably better.
julia
>
> Thanks,
> Denis
>
> >> +@as_dup@
> >> +expression subE1 <= as.E1;
> >> +expression subE2 <= as.E2;
> >> +expression as.E1, as.E2, E3;
> >> +assignment operator aop;
> >> +position p1, p2;
> >> +@@
> >> +
> >> +* array_size(E1, E2)@p1
> >> + ... when != \(E1\|E2\|subE1\|subE2\) aop E3
> >> + when != &\(E1\|E2\|subE1\|subE2\)
> >> +* array_size(E1, E2)@p2
> >> +
> >> +@script:python depends on report@
> >> +p1 << as_dup.p1;
> >> +p2 << as_dup.p2;
> >> +@@
> >> +
> >> +coccilib.report.print_report(p2[0],
> >> +f"WARNING: same array_size (line {p1[0].line})")
> >> +
> >> +@script:python depends on org@
> >> +p1 << as_dup.p1;
> >> +p2 << as_dup.p2;
> >> +@@
> >> +
> >> +coccilib.org.print_todo(p2[0],
> >> +f"WARNING: same array_size (line {p1[0].line})")
> >> +
>
>
Detect an opencoded expression that is used before or after
array_size()/array3_size()/struct_size() to compute the same size.
Signed-off-by: Denis Efremov <[email protected]>
---
Changes in v2:
- python rules moved next to SmPL patterns
- assignment operator used
- struct_size patterns fixed to check only E3, since
E1, E2 are sizeofs of a structure and a member
of a structure
Changes in v3:
- s/overlow/overflow/ typo fixed (thanks, Markus)
- \(&E1\|&E2\) changed to &\(E1\|E2\)
- print strings breaks removed
Changes in v4:
- duplicates warning removed
- python2 compatability in report&&org modes added
- s/down the code/later/ warning changed
- \(E1\|E2\|subE1\|subE2\) patterns simplified to \(subE1\|subE2\)
scripts/coccinelle/misc/array_size_dup.cocci | 209 +++++++++++++++++++
1 file changed, 209 insertions(+)
create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci
diff --git a/scripts/coccinelle/misc/array_size_dup.cocci b/scripts/coccinelle/misc/array_size_dup.cocci
new file mode 100644
index 000000000000..d3d635b2d4fc
--- /dev/null
+++ b/scripts/coccinelle/misc/array_size_dup.cocci
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for array_size(), array3_size(), struct_size() duplicates.
+/// Three types of patterns for these functions:
+/// 1. An opencoded expression is used before array_size() to compute the same size
+/// 2. An opencoded expression is used after array_size() to compute the same size
+/// From security point of view only first case is relevant. These functions
+/// perform arithmetic overflow check. Thus, if we use an opencoded expression
+/// before a call to the *_size() function we can miss an overflow.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers --no-loops
+
+virtual context
+virtual report
+virtual org
+
+@as@
+expression E1, E2;
+@@
+
+array_size(E1, E2)
+
+@as_next@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2@p1
+ ... when != \(subE1\|subE2\) aop E3
+ when != &\(subE1\|subE2\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+msg = "WARNING: array_size is used later (line %s) to compute the same size" % (p2[0].line)
+coccilib.report.print_report(p1[0], msg)
+
+@script:python depends on org@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+msg = "WARNING: array_size is used later (line %s) to compute the same size" % (p2[0].line)
+coccilib.org.print_todo(p1[0], msg)
+
+@as_prev@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(subE1\|subE2\) aop E3
+ when != &\(subE1\|subE2\)
+* E1 * E2@p2
+
+@script:python depends on report@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.report.print_report(p2[0], msg)
+
+@script:python depends on org@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.org.print_todo(p2[0], msg)
+
+@as3@
+expression E1, E2, E3;
+@@
+
+array3_size(E1, E2, E3)
+
+@as3_next@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2 * E3@p1
+ ... when != \(subE1\|subE2\|subE3\) aop E4
+ when != &\(subE1\|subE2\|subE3\)
+* array3_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+msg = "WARNING: array3_size is used later (line %s) to compute the same size" % (p2[0].line)
+coccilib.report.print_report(p1[0], msg)
+
+@script:python depends on org@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+msg = "WARNING: array3_size is used later (line %s) to compute the same size" % (p2[0].line)
+coccilib.org.print_todo(p1[0], msg)
+
+@as3_prev@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array3_size(E1, E2, E3)@p1
+ ... when != \(subE1\|subE2\|subE3\) aop E4
+ when != &\(subE1\|subE2\|subE3\)
+* E1 * E2 * E3@p2
+
+@script:python depends on report@
+p1 << as3_prev.p1;
+p2 << as3_prev.p2;
+@@
+
+msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.report.print_report(p2[0], msg)
+
+@script:python depends on org@
+p1 << as3_prev.p1;
+p2 << as3_prev.p2;
+@@
+
+msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.org.print_todo(p2[0], msg)
+
+@ss@
+expression E1, E2, E3;
+@@
+
+struct_size(E1, E2, E3)
+
+@ss_next@
+expression subE3 <= ss.E3;
+expression ss.E1, ss.E2, ss.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2 + E3@p1
+ ... when != subE3 aop E4
+ when != &subE3
+* struct_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << ss_next.p1;
+p2 << ss_next.p2;
+@@
+
+msg = "WARNING: struct_size is used later (line %s) to compute the same size" % (p2[0].line)
+coccilib.report.print_report(p1[0], msg)
+
+@script:python depends on org@
+p1 << ss_next.p1;
+p2 << ss_next.p2;
+@@
+
+msg = "WARNING: struct_size is used later (line %s) to compute the same size" % (p2[0].line)
+coccilib.org.print_todo(p1[0], msg)
+
+@ss_prev@
+expression subE3 <= ss.E3;
+expression ss.E1, ss.E2, ss.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* struct_size(E1, E2, E3)@p1
+ ... when != subE3 aop E4
+ when != &subE3
+* E1 * E2 + E3@p2
+
+@script:python depends on report@
+p1 << ss_prev.p1;
+p2 << ss_prev.p2;
+@@
+
+msg = "WARNING: struct_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.report.print_report(p2[0], msg)
+
+@script:python depends on org@
+p1 << ss_prev.p1;
+p2 << ss_prev.p2;
+@@
+
+msg = "WARNING: struct_size is already used (line %s) to compute the same size" % (p1[0].line)
+coccilib.org.print_todo(p2[0], msg)
--
2.26.2
> Changes in v2:
…
> - assignment operator used
I prefer the distinction for the application of corresponding metavariables.
> Changes in v3:
…
> - \(&E1\|&E2\) changed to &\(E1\|E2\)
Would it be more helpful to mention the movement of the ampersand
before SmPL disjunctions?
…
>+/// Three types of patterns for these functions:
Will another adjustment be needed according to your information “duplicates warning removed”?
…
> +virtual context
> +virtual report
> +virtual org
Can the following SmPL code variant ever become more attractive?
+virtual context, report, org
…
> +expression subE1 <= as.E1;
> +expression subE2 <= as.E2;
> +expression as.E1, as.E2, E3;
How do you think about the following SmPL code variant?
+expression subE1 <= as.E1,
+ subE2 <= as.E2,
+ as.E1, as.E2, E3;
…
> +msg = "WARNING: array_size is used later (line %s) to compute the same size" % (p2[0].line)
> +coccilib.report.print_report(p1[0], msg)
Please omit the extra Python variable “msg” for the passing of such simple message objects.
What does hinder you to take the proposed script variants better into account?
Regards,
Markus
I don't agree with any of these comments.
julia
On Tue, 23 Jun 2020, Markus Elfring wrote:
> > Changes in v2:
> …
> > - assignment operator used
>
> I prefer the distinction for the application of corresponding metavariables.
>
>
> > Changes in v3:
> …
> > - \(&E1\|&E2\) changed to &\(E1\|E2\)
>
> Would it be more helpful to mention the movement of the ampersand
> before SmPL disjunctions?
>
>
> …
> >+/// Three types of patterns for these functions:
>
> Will another adjustment be needed according to your information “duplicates warning removed”?
>
>
> …
> > +virtual context
> > +virtual report
> > +virtual org
>
> Can the following SmPL code variant ever become more attractive?
>
> +virtual context, report, org
>
>
> …
> > +expression subE1 <= as.E1;
> > +expression subE2 <= as.E2;
> > +expression as.E1, as.E2, E3;
>
> How do you think about the following SmPL code variant?
>
> +expression subE1 <= as.E1,
> + subE2 <= as.E2,
> + as.E1, as.E2, E3;
>
>
> …
> > +msg = "WARNING: array_size is used later (line %s) to compute the same size" % (p2[0].line)
> > +coccilib.report.print_report(p1[0], msg)
>
> Please omit the extra Python variable “msg” for the passing of such simple message objects.
>
> What does hinder you to take the proposed script variants better into account?
>
> Regards,
> Markus
>
> I don't agree with any of these comments.
Would we like to clarify each of the disagreements in more detail
for a more constructive patch review?
Regards,
Markus
On Tue, 23 Jun 2020, Denis Efremov wrote:
> Detect an opencoded expression that is used before or after
> array_size()/array3_size()/struct_size() to compute the same size.
>
> Signed-off-by: Denis Efremov <[email protected]>
Applied, thanks.
julia
> ---
> Changes in v2:
> - python rules moved next to SmPL patterns
> - assignment operator used
> - struct_size patterns fixed to check only E3, since
> E1, E2 are sizeofs of a structure and a member
> of a structure
> Changes in v3:
> - s/overlow/overflow/ typo fixed (thanks, Markus)
> - \(&E1\|&E2\) changed to &\(E1\|E2\)
> - print strings breaks removed
> Changes in v4:
> - duplicates warning removed
> - python2 compatability in report&&org modes added
> - s/down the code/later/ warning changed
> - \(E1\|E2\|subE1\|subE2\) patterns simplified to \(subE1\|subE2\)
>
> scripts/coccinelle/misc/array_size_dup.cocci | 209 +++++++++++++++++++
> 1 file changed, 209 insertions(+)
> create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci
>
> diff --git a/scripts/coccinelle/misc/array_size_dup.cocci b/scripts/coccinelle/misc/array_size_dup.cocci
> new file mode 100644
> index 000000000000..d3d635b2d4fc
> --- /dev/null
> +++ b/scripts/coccinelle/misc/array_size_dup.cocci
> @@ -0,0 +1,209 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// Check for array_size(), array3_size(), struct_size() duplicates.
> +/// Three types of patterns for these functions:
> +/// 1. An opencoded expression is used before array_size() to compute the same size
> +/// 2. An opencoded expression is used after array_size() to compute the same size
> +/// From security point of view only first case is relevant. These functions
> +/// perform arithmetic overflow check. Thus, if we use an opencoded expression
> +/// before a call to the *_size() function we can miss an overflow.
> +///
> +// Confidence: High
> +// Copyright: (C) 2020 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers --no-loops
> +
> +virtual context
> +virtual report
> +virtual org
> +
> +@as@
> +expression E1, E2;
> +@@
> +
> +array_size(E1, E2)
> +
> +@as_next@
> +expression subE1 <= as.E1;
> +expression subE2 <= as.E2;
> +expression as.E1, as.E2, E3;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* E1 * E2@p1
> + ... when != \(subE1\|subE2\) aop E3
> + when != &\(subE1\|subE2\)
> +* array_size(E1, E2)@p2
> +
> +@script:python depends on report@
> +p1 << as_next.p1;
> +p2 << as_next.p2;
> +@@
> +
> +msg = "WARNING: array_size is used later (line %s) to compute the same size" % (p2[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p1 << as_next.p1;
> +p2 << as_next.p2;
> +@@
> +
> +msg = "WARNING: array_size is used later (line %s) to compute the same size" % (p2[0].line)
> +coccilib.org.print_todo(p1[0], msg)
> +
> +@as_prev@
> +expression subE1 <= as.E1;
> +expression subE2 <= as.E2;
> +expression as.E1, as.E2, E3;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* array_size(E1, E2)@p1
> + ... when != \(subE1\|subE2\) aop E3
> + when != &\(subE1\|subE2\)
> +* E1 * E2@p2
> +
> +@script:python depends on report@
> +p1 << as_prev.p1;
> +p2 << as_prev.p2;
> +@@
> +
> +msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << as_prev.p1;
> +p2 << as_prev.p2;
> +@@
> +
> +msg = "WARNING: array_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +@as3@
> +expression E1, E2, E3;
> +@@
> +
> +array3_size(E1, E2, E3)
> +
> +@as3_next@
> +expression subE1 <= as3.E1;
> +expression subE2 <= as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E1, as3.E2, as3.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* E1 * E2 * E3@p1
> + ... when != \(subE1\|subE2\|subE3\) aop E4
> + when != &\(subE1\|subE2\|subE3\)
> +* array3_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << as3_next.p1;
> +p2 << as3_next.p2;
> +@@
> +
> +msg = "WARNING: array3_size is used later (line %s) to compute the same size" % (p2[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p1 << as3_next.p1;
> +p2 << as3_next.p2;
> +@@
> +
> +msg = "WARNING: array3_size is used later (line %s) to compute the same size" % (p2[0].line)
> +coccilib.org.print_todo(p1[0], msg)
> +
> +@as3_prev@
> +expression subE1 <= as3.E1;
> +expression subE2 <= as3.E2;
> +expression subE3 <= as3.E3;
> +expression as3.E1, as3.E2, as3.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* array3_size(E1, E2, E3)@p1
> + ... when != \(subE1\|subE2\|subE3\) aop E4
> + when != &\(subE1\|subE2\|subE3\)
> +* E1 * E2 * E3@p2
> +
> +@script:python depends on report@
> +p1 << as3_prev.p1;
> +p2 << as3_prev.p2;
> +@@
> +
> +msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << as3_prev.p1;
> +p2 << as3_prev.p2;
> +@@
> +
> +msg = "WARNING: array3_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> +
> +@ss@
> +expression E1, E2, E3;
> +@@
> +
> +struct_size(E1, E2, E3)
> +
> +@ss_next@
> +expression subE3 <= ss.E3;
> +expression ss.E1, ss.E2, ss.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* E1 * E2 + E3@p1
> + ... when != subE3 aop E4
> + when != &subE3
> +* struct_size(E1, E2, E3)@p2
> +
> +@script:python depends on report@
> +p1 << ss_next.p1;
> +p2 << ss_next.p2;
> +@@
> +
> +msg = "WARNING: struct_size is used later (line %s) to compute the same size" % (p2[0].line)
> +coccilib.report.print_report(p1[0], msg)
> +
> +@script:python depends on org@
> +p1 << ss_next.p1;
> +p2 << ss_next.p2;
> +@@
> +
> +msg = "WARNING: struct_size is used later (line %s) to compute the same size" % (p2[0].line)
> +coccilib.org.print_todo(p1[0], msg)
> +
> +@ss_prev@
> +expression subE3 <= ss.E3;
> +expression ss.E1, ss.E2, ss.E3, E4;
> +assignment operator aop;
> +position p1, p2;
> +@@
> +
> +* struct_size(E1, E2, E3)@p1
> + ... when != subE3 aop E4
> + when != &subE3
> +* E1 * E2 + E3@p2
> +
> +@script:python depends on report@
> +p1 << ss_prev.p1;
> +p2 << ss_prev.p2;
> +@@
> +
> +msg = "WARNING: struct_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.report.print_report(p2[0], msg)
> +
> +@script:python depends on org@
> +p1 << ss_prev.p1;
> +p2 << ss_prev.p2;
> +@@
> +
> +msg = "WARNING: struct_size is already used (line %s) to compute the same size" % (p1[0].line)
> +coccilib.org.print_todo(p2[0], msg)
> --
> 2.26.2
>
>