From: Himanshu Jha <himanshujha199640@gmail.com>
To: Dominique Martinet <asmadeus@codewreck.org>
Cc: "Michal Marek" <michal.lkml@markovi.net>,
"Nicolas Palix" <nicolas.palix@imag.fr>,
linux-kernel@vger.kernel.org, cocci@systeme.lip6.fr,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Subject: Re: [Cocci] [PATCH 01/18] coccinelle: change strncpy+truncation to strlcpy
Date: Fri, 13 Jul 2018 13:14:55 +0530 [thread overview]
Message-ID: <20180713074455.GA6859@himanshu-Vostro-3559> (raw)
In-Reply-To: <1531444483-17338-1-git-send-email-asmadeus@codewreck.org>
On Fri, Jul 13, 2018 at 03:14:43AM +0200, Dominique Martinet wrote:
> Besides being simpler, using strlcpy instead of strncpy+truncation
> fixes part of the following class of new gcc warnings:
>
> drivers/gpu/drm/i915/intel_tv.c: In function ‘intel_tv_get_modes’:
> drivers/gpu/drm/i915/intel_tv.c:1358:3: error: ‘strncpy’ specified bound 32 equals
> destination size [-Werror=stringop-truncation]
> strncpy(mode_ptr->name, input->name, DRM_DISPLAY_MODE_LEN);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Note that this is not a proper fix for this warning (and not all of the
> occurences give the warning either - the strings are not always static).
> The warning was intended to have developers check the return code of
> strncpy and act in case of truncation (print a warning, abort the
> function or something similar if the original string was not nul
> terminated); the change to strlcpy only works because gcc does not
> handle the function the same way.
>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
> ---
>
> Running this fixes 30 occurences of the problem in 17 different
> components of the kernel, and while the produced patches are fairly
> straight-forward I'm not sure who I should expect to pick this up as
> it is sent as a series.
> I expect each maintainer will pick their share of the patchs if they
> agree with it and the rest will just be dropped?
Masahiro Yamada <yamada.masahiro@socionext.com> takes coccinelle patches,
so please cc him or your patch would be lost.
> .../coccinelle/misc/strncpy_truncation.cocci | 41 +++++++++++++++++++
> 1 file changed, 41 insertions(+)
> create mode 100644 scripts/coccinelle/misc/strncpy_truncation.cocci
>
> diff --git a/scripts/coccinelle/misc/strncpy_truncation.cocci b/scripts/coccinelle/misc/strncpy_truncation.cocci
> new file mode 100644
> index 000000000000..28b5c2a290ac
> --- /dev/null
> +++ b/scripts/coccinelle/misc/strncpy_truncation.cocci
> @@ -0,0 +1,41 @@
> +/// Use strlcpy rather than strncpy(dest,..,sz) + dest[sz-1] = '\0'
> +///
> +// Confidence: High
> +// Comments:
> +// Options: --no-includes --include-headers
> +
> +virtual patch
> +virtual context
You might consider adding context rule or remove this line perhaps ?
> +virtual report
> +virtual org
> +
> +@r@
> +expression dest, src, sz;
> +position p;
> +@@
> +
> +strncpy@p(dest, src, sz);
> +dest[sz - 1] = '\0';
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +cocci.print_main("strncpy followed by truncation can be strlcpy",p)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +msg = "SUGGESTION: strncpy followed by truncation can be strlcpy"
> +coccilib.report.print_report(p[0],msg)
> +
> +@ok depends on patch@
> +expression r.dest, r.src, r.sz;
> +position r.p;
> +@@
> +
> +-strncpy@p(
> ++strlcpy(
> + dest, src, sz);
> +-dest[sz - 1] = '\0';
The above rule produces an output that I think is not correct:
--------------------------------------------------------------
diff =
diff -u -p a//ti/wl1251/acx.c b//ti/wl1251/acx.c
--- a//ti/wl1251/acx.c
+++ b//ti/wl1251/acx.c
@@ -150,14 +150,7 @@ int wl1251_acx_fw_version(struct wl1251
}
/* be careful with the buffer sizes */
- strncpy(buf, rev->fw_version, min(len, sizeof(rev->fw_version)));
-
- /*
- * if the firmware version string is exactly
- * sizeof(rev->fw_version) long or fw_len is less than
- * sizeof(rev->fw_version) it won't be null terminated
- */
- buf[min(len, sizeof(rev->fw_version)) - 1] = '\0';
+ strlcpy(buf, rev->fw_version, min(len, sizeof(rev->fw_version)));
-----------------------------------------------------------------
I think the comment is useful and should not be removed. Also, consider
changing Confidence level appropriately.
Thanks.
--
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology
next prev parent reply other threads:[~2018-07-13 7:45 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-13 1:14 [PATCH 01/18] coccinelle: change strncpy+truncation to strlcpy Dominique Martinet
2018-07-13 1:25 ` [PATCH 02/18] block/aoenet: " Dominique Martinet
2018-07-13 14:16 ` Jens Axboe
2018-07-13 15:31 ` Dominique Martinet
2018-07-13 1:25 ` [PATCH 03/18] drm_property: " Dominique Martinet
2018-07-13 1:25 ` [PATCH 04/18] nouveau: " Dominique Martinet
2018-07-13 1:25 ` [PATCH 05/18] iio: " Dominique Martinet
2018-07-15 10:39 ` Jonathan Cameron
2018-07-16 11:42 ` Dominique Martinet
2018-07-22 8:13 ` Jonathan Cameron
2018-07-13 1:25 ` [PATCH 06/18] mptctl: " Dominique Martinet
2018-07-13 1:25 ` [PATCH 07/18] hisilicon: " Dominique Martinet
2018-07-13 1:25 ` [PATCH 08/18] myricom: " Dominique Martinet
2018-07-13 1:25 ` [PATCH 09/18] qlogic/qed: " Dominique Martinet
2018-07-13 1:25 ` [PATCH 10/18] brcmsmac: " Dominique Martinet
2018-07-13 7:19 ` Arend van Spriel
2018-07-13 1:25 ` [PATCH 11/18] wireless/ti: " Dominique Martinet
2018-07-13 7:38 ` Greg Kroah-Hartman
2018-07-13 7:47 ` Arend van Spriel
2018-07-13 8:13 ` Dominique Martinet
2018-07-13 18:56 ` Rustad, Mark D
2018-07-27 9:19 ` Kalle Valo
2018-07-13 1:25 ` [PATCH 12/18] test_power: " Dominique Martinet
2018-07-13 1:25 ` [PATCH 13/18] ibmvscsi: " Dominique Martinet
2018-07-13 1:25 ` [PATCH 14/18] kdb_support: " Dominique Martinet
2018-07-13 10:33 ` Daniel Thompson
2018-07-13 15:18 ` Dominique Martinet
2018-07-16 8:23 ` Daniel Thompson
2018-07-13 1:26 ` [PATCH 15/18] blktrace: " Dominique Martinet
2019-03-15 1:37 ` Steven Rostedt
2019-03-15 2:01 ` Jens Axboe
2019-03-15 6:30 ` Dominique Martinet
2019-03-15 14:29 ` Jens Axboe
2018-07-13 1:26 ` [PATCH 16/18] tools/accounting: " Dominique Martinet
2018-07-13 1:26 ` [PATCH 17/18] perf: " Dominique Martinet
2018-07-13 1:26 ` [PATCH 18/18] cpupower: " Dominique Martinet
2018-07-24 16:31 ` Shuah Khan
2018-08-14 15:45 ` Daniel Díaz
2018-08-14 19:27 ` Dominique Martinet
2018-08-20 14:27 ` Shuah Khan
2018-07-13 7:44 ` Himanshu Jha [this message]
2018-07-13 8:00 ` [Cocci] [PATCH 01/18] coccinelle: " Dominique Martinet
2018-07-13 9:14 ` Himanshu Jha
2018-07-13 9:44 ` Julia Lawall
2018-07-13 10:21 ` Himanshu Jha
2018-07-13 10:50 ` Julia Lawall
2018-07-13 16:11 ` Dominique Martinet
2018-07-14 8:12 ` [PATCH v2] coccinelle: strncpy+truncation by strscpy Dominique Martinet
2018-07-14 11:54 ` Julia Lawall
[not found] ` <alpine.DEB.2.20.1807140743550.3356@hadrien>
2018-07-14 13:08 ` Dominique Martinet
2018-07-14 20:36 ` Julia Lawall
2018-07-14 14:34 ` [v2] Coccinelle: Replace strncpy() + truncation by strscpy() SF Markus Elfring
2018-07-20 0:36 ` [PATCH v3] coccinelle: suggest replacing strncpy+truncation by strscpy Dominique Martinet
2018-07-20 5:33 ` Julia Lawall
2018-07-20 5:40 ` Dominique Martinet
2018-07-20 5:49 ` Julia Lawall
2018-07-20 5:57 ` Dominique Martinet
2018-07-20 6:03 ` Julia Lawall
2018-07-20 11:00 ` [v3] Coccinelle: " SF Markus Elfring
2018-07-20 9:40 ` SF Markus Elfring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180713074455.GA6859@himanshu-Vostro-3559 \
--to=himanshujha199640@gmail.com \
--cc=asmadeus@codewreck.org \
--cc=cocci@systeme.lip6.fr \
--cc=linux-kernel@vger.kernel.org \
--cc=michal.lkml@markovi.net \
--cc=nicolas.palix@imag.fr \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).