* [PATCH bpf-next] libbpf: fix GCC8 warning for strncpy
@ 2019-07-02 5:46 Andrii Nakryiko
2019-07-02 6:09 ` Y Song
0 siblings, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2019-07-02 5:46 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel
Cc: Andrii Nakryiko, Magnus Karlsson
GCC8 started emitting warning about using strncpy with number of bytes
exactly equal destination size, which is generally unsafe, as can lead
to non-zero terminated string being copied. Use IFNAMSIZ - 1 as number
of bytes to ensure name is always zero-terminated.
Cc: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/xsk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index bf15a80a37c2..9588e7f87d0b 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -327,7 +327,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
channels.cmd = ETHTOOL_GCHANNELS;
ifr.ifr_data = (void *)&channels;
- strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ);
+ strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
err = ioctl(fd, SIOCETHTOOL, &ifr);
if (err && errno != EOPNOTSUPP) {
ret = -errno;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] libbpf: fix GCC8 warning for strncpy
2019-07-02 5:46 [PATCH bpf-next] libbpf: fix GCC8 warning for strncpy Andrii Nakryiko
@ 2019-07-02 6:09 ` Y Song
2019-07-02 7:32 ` Magnus Karlsson
2019-07-02 15:14 ` Andrii Nakryiko
0 siblings, 2 replies; 4+ messages in thread
From: Y Song @ 2019-07-02 6:09 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: andrii.nakryiko, bpf, netdev, Alexei Starovoitov,
Daniel Borkmann, Magnus Karlsson
On Mon, Jul 1, 2019 at 10:47 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> GCC8 started emitting warning about using strncpy with number of bytes
> exactly equal destination size, which is generally unsafe, as can lead
> to non-zero terminated string being copied. Use IFNAMSIZ - 1 as number
> of bytes to ensure name is always zero-terminated.
>
> Cc: Magnus Karlsson <magnus.karlsson@intel.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/xsk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index bf15a80a37c2..9588e7f87d0b 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -327,7 +327,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
>
> channels.cmd = ETHTOOL_GCHANNELS;
> ifr.ifr_data = (void *)&channels;
> - strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ);
> + strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
To accommodate the xsk->ifname string length FNAMSIZ - 1, we need to have
ifr.ifr_name[FNAMSIZ - 1] = '\0';
right?
> err = ioctl(fd, SIOCETHTOOL, &ifr);
> if (err && errno != EOPNOTSUPP) {
> ret = -errno;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] libbpf: fix GCC8 warning for strncpy
2019-07-02 6:09 ` Y Song
@ 2019-07-02 7:32 ` Magnus Karlsson
2019-07-02 15:14 ` Andrii Nakryiko
1 sibling, 0 replies; 4+ messages in thread
From: Magnus Karlsson @ 2019-07-02 7:32 UTC (permalink / raw)
To: Y Song
Cc: Andrii Nakryiko, Andrii Nakryiko, bpf, netdev,
Alexei Starovoitov, Daniel Borkmann, Magnus Karlsson
On Tue, Jul 2, 2019 at 8:10 AM Y Song <ys114321@gmail.com> wrote:
>
> On Mon, Jul 1, 2019 at 10:47 PM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > GCC8 started emitting warning about using strncpy with number of bytes
> > exactly equal destination size, which is generally unsafe, as can lead
> > to non-zero terminated string being copied. Use IFNAMSIZ - 1 as number
> > of bytes to ensure name is always zero-terminated.
> >
> > Cc: Magnus Karlsson <magnus.karlsson@intel.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/xsk.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> > index bf15a80a37c2..9588e7f87d0b 100644
> > --- a/tools/lib/bpf/xsk.c
> > +++ b/tools/lib/bpf/xsk.c
> > @@ -327,7 +327,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
> >
> > channels.cmd = ETHTOOL_GCHANNELS;
> > ifr.ifr_data = (void *)&channels;
> > - strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ);
> > + strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
>
> To accommodate the xsk->ifname string length FNAMSIZ - 1, we need to have
> ifr.ifr_name[FNAMSIZ - 1] = '\0';
> right?
Yes. Since the ifr struct is allocated from the stack and not zeroed,
we should do this.
Thanks to both of you for catching this.
/Magnus
> > err = ioctl(fd, SIOCETHTOOL, &ifr);
> > if (err && errno != EOPNOTSUPP) {
> > ret = -errno;
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] libbpf: fix GCC8 warning for strncpy
2019-07-02 6:09 ` Y Song
2019-07-02 7:32 ` Magnus Karlsson
@ 2019-07-02 15:14 ` Andrii Nakryiko
1 sibling, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2019-07-02 15:14 UTC (permalink / raw)
To: Y Song
Cc: Andrii Nakryiko, bpf, netdev, Alexei Starovoitov,
Daniel Borkmann, Magnus Karlsson
On Mon, Jul 1, 2019 at 11:10 PM Y Song <ys114321@gmail.com> wrote:
>
> On Mon, Jul 1, 2019 at 10:47 PM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > GCC8 started emitting warning about using strncpy with number of bytes
> > exactly equal destination size, which is generally unsafe, as can lead
> > to non-zero terminated string being copied. Use IFNAMSIZ - 1 as number
> > of bytes to ensure name is always zero-terminated.
> >
> > Cc: Magnus Karlsson <magnus.karlsson@intel.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/xsk.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> > index bf15a80a37c2..9588e7f87d0b 100644
> > --- a/tools/lib/bpf/xsk.c
> > +++ b/tools/lib/bpf/xsk.c
> > @@ -327,7 +327,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
> >
> > channels.cmd = ETHTOOL_GCHANNELS;
> > ifr.ifr_data = (void *)&channels;
> > - strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ);
> > + strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
>
> To accommodate the xsk->ifname string length FNAMSIZ - 1, we need to have
> ifr.ifr_name[FNAMSIZ - 1] = '\0';
> right?
Yes. I somehow misread description of strncpy and assumed it does that
automatically (which would make sense), but it actually doesn't. Only
strlcpy does. v2 with fix is coming.
>
> > err = ioctl(fd, SIOCETHTOOL, &ifr);
> > if (err && errno != EOPNOTSUPP) {
> > ret = -errno;
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-02 15:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-02 5:46 [PATCH bpf-next] libbpf: fix GCC8 warning for strncpy Andrii Nakryiko
2019-07-02 6:09 ` Y Song
2019-07-02 7:32 ` Magnus Karlsson
2019-07-02 15:14 ` Andrii Nakryiko
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.