From: Christian Schoenebeck <linux_oss@crudebyte.com>
To: v9fs@lists.linux.dev, Dominique Martinet <asmadeus@codewreck.org>
Cc: ericvh@kernel.org, lucho@ionkov.net,
linux-kernel@vger.kernel.org,
Dominique Martinet <asmadeus@codewreck.org>
Subject: Re: [PATCH v2 3/3] 9p/net: xen: fix false positive printf format overflow warning
Date: Wed, 25 Oct 2023 19:24:11 +0200 [thread overview]
Message-ID: <1940104.uIXT7tB08N@silver> (raw)
In-Reply-To: <20231025103445.1248103-3-asmadeus@codewreck.org>
On Wednesday, October 25, 2023 12:34:45 PM CEST Dominique Martinet wrote:
> Use the constant to make the compiler happy about this warning:
> net/9p/trans_xen.c: In function ‘xen_9pfs_front_changed’:
> net/9p/trans_xen.c:444:39: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 8 [-Wformat-overflow=]
> 444 | sprintf(str, "ring-ref%d", i);
> | ^~
> In function ‘xen_9pfs_front_init’,
> inlined from ‘xen_9pfs_front_changed’ at net/9p/trans_xen.c:516:8,
> inlined from ‘xen_9pfs_front_changed’ at net/9p/trans_xen.c:504:13:
> net/9p/trans_xen.c:444:30: note: directive argument in the range [-2147483644, 2147483646]
> 444 | sprintf(str, "ring-ref%d", i);
> | ^~~~~~~~~~~~
> net/9p/trans_xen.c:444:17: note: ‘sprintf’ output between 10 and 20 bytes into a destination of size 16
> 444 | sprintf(str, "ring-ref%d", i);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> net/9p/trans_xen.c: In function ‘xen_9pfs_front_changed’:
> net/9p/trans_xen.c:450:45: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 2 [-Wformat-overflow=]
> 450 | sprintf(str, "event-channel-%d", i);
> | ^~
> In function ‘xen_9pfs_front_init’,
> inlined from ‘xen_9pfs_front_changed’ at net/9p/trans_xen.c:516:8,
> inlined from ‘xen_9pfs_front_changed’ at net/9p/trans_xen.c:504:13:
> net/9p/trans_xen.c:450:30: note: directive argument in the range [-2147483644, 2147483646]
> 450 | sprintf(str, "event-channel-%d", i);
> | ^~~~~~~~~~~~~~~~~~
> net/9p/trans_xen.c:450:17: note: ‘sprintf’ output between 16 and 26 bytes into a destination of size 16
> 450 | sprintf(str, "event-channel-%d", i);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> There is no change in logic: there only are a constant number of rings,
> and there also already is a BUILD_BUG_ON that checks if that constant
> goes over 9 as anything bigger would no longer fit the event-channel-%d
> destination size.
>
> In theory having that size as part of the struct means it could be
> modified by another thread and makes the compiler lose track of possible
> values for 'i' here, using the constant directly here makes it work.
>
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
> Message-ID: <20231023233704.1185154-4-asmadeus@codewreck.org>
> ---
Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> v1->v2:
> - use constant directly instead of going through a local variable
>
> net/9p/trans_xen.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> index 1fffe2bed5b0..dfdbe1ca5338 100644
> --- a/net/9p/trans_xen.c
> +++ b/net/9p/trans_xen.c
> @@ -54,7 +54,6 @@ struct xen_9pfs_front_priv {
> char *tag;
> struct p9_client *client;
>
> - int num_rings;
> struct xen_9pfs_dataring *rings;
> };
>
> @@ -131,7 +130,7 @@ static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
> if (list_entry_is_head(priv, &xen_9pfs_devs, list))
> return -EINVAL;
>
> - num = p9_req->tc.tag % priv->num_rings;
> + num = p9_req->tc.tag % XEN_9PFS_NUM_RINGS;
> ring = &priv->rings[num];
>
> again:
> @@ -279,7 +278,7 @@ static void xen_9pfs_front_free(struct xen_9pfs_front_priv *priv)
> list_del(&priv->list);
> write_unlock(&xen_9pfs_lock);
>
> - for (i = 0; i < priv->num_rings; i++) {
> + for (i = 0; i < XEN_9PFS_NUM_RINGS; i++) {
> struct xen_9pfs_dataring *ring = &priv->rings[i];
>
> cancel_work_sync(&ring->work);
> @@ -408,15 +407,14 @@ static int xen_9pfs_front_init(struct xenbus_device *dev)
> if (p9_xen_trans.maxsize > XEN_FLEX_RING_SIZE(max_ring_order))
> p9_xen_trans.maxsize = XEN_FLEX_RING_SIZE(max_ring_order) / 2;
>
> - priv->num_rings = XEN_9PFS_NUM_RINGS;
> - priv->rings = kcalloc(priv->num_rings, sizeof(*priv->rings),
> + priv->rings = kcalloc(XEN_9PFS_NUM_RINGS, sizeof(*priv->rings),
> GFP_KERNEL);
> if (!priv->rings) {
> kfree(priv);
> return -ENOMEM;
> }
>
> - for (i = 0; i < priv->num_rings; i++) {
> + for (i = 0; i < XEN_9PFS_NUM_RINGS; i++) {
> priv->rings[i].priv = priv;
> ret = xen_9pfs_front_alloc_dataring(dev, &priv->rings[i],
> max_ring_order);
> @@ -434,10 +432,11 @@ static int xen_9pfs_front_init(struct xenbus_device *dev)
> if (ret)
> goto error_xenbus;
> ret = xenbus_printf(xbt, dev->nodename, "num-rings", "%u",
> - priv->num_rings);
> + XEN_9PFS_NUM_RINGS);
> if (ret)
> goto error_xenbus;
> - for (i = 0; i < priv->num_rings; i++) {
> +
> + for (i = 0; i < XEN_9PFS_NUM_RINGS; i++) {
> char str[16];
>
> BUILD_BUG_ON(XEN_9PFS_NUM_RINGS > 9);
>
prev parent reply other threads:[~2023-10-25 17:24 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-25 10:34 [PATCH v2 1/3] 9p/trans_fd: Annotate data-racy writes to file::f_flags Dominique Martinet
2023-10-25 10:34 ` [PATCH v2 2/3] 9p: v9fs_listxattr: fix %s null argument warning Dominique Martinet
2023-10-25 10:34 ` [PATCH v2 3/3] 9p/net: xen: fix false positive printf format overflow warning Dominique Martinet
2023-10-25 17:24 ` Christian Schoenebeck [this message]
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=1940104.uIXT7tB08N@silver \
--to=linux_oss@crudebyte.com \
--cc=asmadeus@codewreck.org \
--cc=ericvh@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lucho@ionkov.net \
--cc=v9fs@lists.linux.dev \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).