linux-kernel - Re: [PATCH v2] coccinelle: assign signed result to unsigned variable

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] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 28 Sep 2015 13:32:27 +0200 (CEST)
From:	Julia Lawall <julia.lawall@...6.fr>
To:	Andrzej Hajda <a.hajda@...sung.com>
cc:	julia.lawall@...6.fr,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Gilles Muller <Gilles.Muller@...6.fr>,
	Michal Marek <mmarek@...e.com>,
	Nicolas Palix <nicolas.palix@...g.fr>,
	kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org,
	cocci@...teme.lip6.fr, elfring@...rs.sourceforge.net
Subject: Re: [PATCH v2] coccinelle: assign signed result to unsigned
 variable



On Mon, 28 Sep 2015, Andrzej Hajda wrote:

> Assigning signed function result to unsigned variable can indicate error.
> To decrease number of false positives patch looks if after assignment
> there is also check for negative values of the result.
>
> Signed-off-by: Andrzej Hajda <a.hajda@...sung.com>
> ---
> Hi Julia,
>
> Thanks for the hint. Now it looks much better.
> Summarizing this patch has found 20 problems and has 22 false positives [1][2].

Do you have some examples of the false positives?

julia

> unsigned_lesser_than_zero.cocci patch posted earlier has found
> 40 problems [3][4], and about 80 false positives if I remember correctly.
> Few patches were rejected, as developers likes code for testing variable range,
> even if its result is always true/false [5][6], but most of kernel patches are
> real bug fixes.
>
> Both patches tries to address similar issues, maybe it would be good to merge
> them? Especially as their results overlap.
> Additionally I thought about adding detecting range checks in
> unsigned_lesser_than_zero.cocci, to decrease number of false positives.
> Of course it could then miss real bugs. What do you think about it?
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2046131
> [2]: http://permalink.gmane.org/gmane.linux.kernel/2048070
> [3]: http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/70031
> [4]: http://permalink.gmane.org/gmane.linux.power-management.general/66143
> [5]: http://permalink.gmane.org/gmane.linux.kernel.mm/138902
> [6]: http://libdivecomputer.org/pipermail/devel/2014-July/000329.html
>
> Regards
> Andrzej
>
> ---
>  .../tests/assign_signed_to_unsigned.cocci          | 45 ++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 scripts/coccinelle/tests/assign_signed_to_unsigned.cocci
>
> diff --git a/scripts/coccinelle/tests/assign_signed_to_unsigned.cocci b/scripts/coccinelle/tests/assign_signed_to_unsigned.cocci
> new file mode 100644
> index 0000000..efa4e83
> --- /dev/null
> +++ b/scripts/coccinelle/tests/assign_signed_to_unsigned.cocci
> @@ -0,0 +1,45 @@
> +/// Assigning signed function result to unsigned variable can indicate error.
> +/// To decrease number of false positives patch looks if after assignment
> +/// there is also check for negative values of the result.
> +///
> +// Confidence: High
> +// Copyright: (C) 2015 Andrzej Hajda, Samsung Electronics Co., Ltd. GPLv2.
> +// URL: http://coccinelle.lip6.fr/
> +// Options: --include-headers --all-includes
> +
> +virtual context
> +virtual org
> +virtual report
> +
> +@r@
> +typedef bool, u8, u16, u32, u64, s8, s16, s32, s64;
> +{char, short, int, long, long long, s8, s16, s32, s64} vs;
> +{unsigned char, unsigned short, unsigned int, unsigned long, unsigned long long,
> +    size_t, bool, u8, u16, u32, u64} vu;
> +position p;
> +identifier f;
> +statement S1, S2;
> +expression e;
> +@@
> +
> +*vu@p = f(...)@vs;
> +... when != vu = e;
> +if ( \( vu < 0 \| vu <= 0 \) ) S1 else S2
> +
> +@...ipt:python depends on r && org@
> +p << r.p;
> +f << r.f;
> +vu << r.vu;
> +@@
> +
> +msg = "WARNING: Assigning signed result to unsigned variable: %s = %s(...)" % (vu, f)
> +coccilib.org.print_todo(p[0], msg)
> +
> +@...ipt:python depends on r && report@
> +p << r.p;
> +f << r.f;
> +vu << r.vu;
> +@@
> +
> +msg = "WARNING: Assigning signed result to unsigned variable: %s = %s(...)" % (vu, f)
> +coccilib.report.print_report(p[0], msg)
> --
> 1.9.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/