linux-kernel - Re: 回复: [PATCH] coccinelle: adjust the confidence of fen.cocci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.22.394.2112151026590.2946@hadrien>
Date:   Wed, 15 Dec 2021 10:36:17 +0100 (CET)
From:   Julia Lawall <julia.lawall@...ia.fr>
To:     王擎 <wangqing@...o.com>
cc:     Nicolas Palix <nicolas.palix@...g.fr>,
        "cocci@...teme.lip6.fr" <cocci@...teme.lip6.fr>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: 回复: [PATCH] coccinelle: adjust the confidence of fen.cocci



On Wed, 15 Dec 2021, 王擎 wrote:

>
> >> From: Wang Qing <wangqing@...o.com>
> >>
> >> We scan by coccinelle according to the confidence value, then check for
> >> potential problems. But the accuracy of this cocci is very low,
> >> the scan results are all wrong.
> >
> >Can you give some examples of cases where the results are wrong?
> >
> For example:
> for_each_node_by_name(np, "global-utilities") {
> 	if ((of_get_property(np, "fsl,has-rstcr", NULL))) {
> 		rstcr = of_iomap(np, 0) + 0xb0;
> 		if (!rstcr) {
> 			printk (KERN_ERR "Error: reset control "
> 					"register not mapped!\n");
> 		} else {
> 			register_restart_handler(&restart_handler);
> 		}
> 		break; // break here
> 	}
> }
>
> of_node_put(np);  // Is there any problem here?

This looks quite strange.  If the loop exits normally, then there is a put
on something on which there was no get.  Is the value NULL in tihs case,
and so the of_node_put does nothing?  I guess that then there is no
problem, but it's not clear that the code is very nice either.

But I think that the real of_node_put problems are covered by another
rule, so we can drop this one, as you suggested.

julia

>
> And there are many others:
> >>>>>>>>>>>>>>>>>  iterators/fen.cocci <<<<<<<<<<<<<<<<<<<<
> drivers/edac/cell_edac.c:165:1-12: ERROR: of_node_put not needed after iterator on line 137
> arch/powerpc/platforms/powermac/udbg_adb.c:204:1-12: ERROR: of_node_put not needed after iterator on line 195
> arch/powerpc/platforms/cell/iommu.c:744:1-12: ERROR: of_node_put not needed after iterator on line 737
> arch/powerpc/kexec/file_load_64.c:1037:1-12: ERROR: of_node_put not needed after iterator on line 1023
> drivers/cpufreq/pmac64-cpufreq.c:640:1-12: ERROR: of_node_put not needed after iterator on line 501
> arch/powerpc/kexec/ranges.c:271:1-12: ERROR: of_node_put not needed after iterator on line 248
> arch/powerpc/platforms/chrp/setup.c:479:1-12: ERROR: of_node_put not needed after iterator on line 470
> arch/powerpc/platforms/chrp/setup.c:524:1-12: ERROR: of_node_put not needed after iterator on line 521
> arch/powerpc/platforms/4xx/uic.c:293:1-12: ERROR: of_node_put not needed after iterator on line 280
> arch/powerpc/mm/numa.c:1317:1-12: ERROR: of_node_put not needed after iterator on line 1289
> sound/ppc/pmac.c:970:2-13: ERROR: of_node_put not needed after iterator on line 951
> drivers/tty/serial/cpm_uart/cpm_uart_core.c:1336:1-12: ERROR: of_node_put not needed after iterator on line 1316
> arch/powerpc/sysdev/fsl_soc.c:190:1-12: ERROR: of_node_put not needed after iterator on line 177
>
> Thanks,
> Qing
>
> >thanks,
> >julia
> >
> >>
> >> So, lower its confidence to low.
> >>
> >> Signed-off-by: Wang Qing <wangqing@...o.com>
> >> ---
> >>  scripts/coccinelle/iterators/fen.cocci | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/scripts/coccinelle/iterators/fen.cocci b/scripts/coccinelle/iterators/fen.cocci
> >> index b69f966..7b71207
> >> --- a/scripts/coccinelle/iterators/fen.cocci
> >> +++ b/scripts/coccinelle/iterators/fen.cocci
> >> @@ -2,7 +2,7 @@
> >>  /// These iterators only exit normally when the loop cursor is NULL, so there
> >>  /// is no point to call of_node_put on the final value.
> >>  ///
> >> -// Confidence: High
> >> +// Confidence: Low
> >>  // Copyright: (C) 2010-2012 Nicolas Palix.
> >>  // Copyright: (C) 2010-2012 Julia Lawall, INRIA/LIP6.
> >>  // Copyright: (C) 2010-2012 Gilles Muller, INRIA/LiP6.
> >> --
> >> 2.7.4
> >>
> >>