All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	aleksander.lobakin@intel.com,
	nex.sw.ncis.osdt.itp.upstreaming@intel.com,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [net-next v2] pfcp: avoid copy warning by simplifing code
Date: Mon, 8 Apr 2024 09:18:29 +0100	[thread overview]
Message-ID: <20240408081829.GC26556@kernel.org> (raw)
In-Reply-To: <20240405063605.649744-1-michal.swiatkowski@linux.intel.com>

On Fri, Apr 05, 2024 at 08:36:05AM +0200, Michal Swiatkowski wrote:
> >From Arnd comments:
> "The memcpy() in the ip_tunnel_info_opts_set() causes
> a string.h fortification warning, with at least gcc-13:
> 
>     In function 'fortify_memcpy_chk',
>         inlined from 'ip_tunnel_info_opts_set' at include/net/ip_tunnels.h:619:3,
>         inlined from 'pfcp_encap_recv' at drivers/net/pfcp.c:84:2:
>     include/linux/fortify-string.h:553:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>       553 |                         __write_overflow_field(p_size_field, size);"
> 
> It is a false-positivie caused by ambiguity of the union.
> 
> However, as Arnd noticed, copying here is unescessary. The code can be
> simplified to avoid calling ip_tunnel_info_opts_set(), which is doing
> copying, setting flags and options_len.
> 
> Set correct flags and options_len directly on tun_info.
> 
> Fixes: 6dd514f48110 ("pfcp: always set pfcp metadata")
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Closes: https://lore.kernel.org/netdev/701f8f93-f5fb-408b-822a-37a1d5c424ba@app.fastmail.com/
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

I agree that it's nice to avoid a copy.
But I do wonder where else this pattern may exist.
And if it might be worth introducing a helper for it.

Regardless, this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

...

  reply	other threads:[~2024-04-08  8:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-05  6:36 [net-next v2] pfcp: avoid copy warning by simplifing code Michal Swiatkowski
2024-04-08  8:18 ` Simon Horman [this message]
2024-04-08 10:46   ` Michal Swiatkowski
2024-04-08 10:50     ` Alexander Lobakin
2024-04-08 11:08       ` Michal Swiatkowski
2024-04-08 10:50 ` patchwork-bot+netdevbpf

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=20240408081829.GC26556@kernel.org \
    --to=horms@kernel.org \
    --cc=aleksander.lobakin@intel.com \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=michal.swiatkowski@linux.intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=nex.sw.ncis.osdt.itp.upstreaming@intel.com \
    --cc=pabeni@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.