On Mon, 28 Jan 2019 10:17:03 +0000
Andre Przywara <andre.przywara@xxxxxxx> wrote:
Forgot to mention one thing ...
> On Thu, 17 Jan 2019 13:40:13 +0100
> Anisse Astier <aastier@xxxxxxxxxx> wrote:
>
> Hi,
>
> > GCC 8.2 gives this warning:
> >
> > virtio/net.c: In function ‘virtio_net__tap_init’:
> > virtio/net.c:336:47: error: argument to ‘sizeof’ in ‘strncpy’ call
> > is the same expression as the source; did you mean to use the size
> > of the destination? [-Werror=sizeof-pointer-memaccess]
> > strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name)); ^
> > virtio/net.c:348:47: error: argument to ‘sizeof’ in ‘strncpy’ call
> > is the same expression as the source; did you mean to use the size
> > of the destination? [-Werror=sizeof-pointer-memaccess]
> > strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name)); ^
> >
> > Fix it by using sizeof of destination instead, even if they're the
> > same size in this case.
>
> Yes, looks right the right thing to do.
>
> > Signed-off-by: Anisse Astier <aastier@xxxxxxxxxx>
>
> Reviewed-by: Andre Przywara <andre.przywara@xxxxxxx>
> > ---
> virtio/net.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/virtio/net.c b/virtio/net.c
> index f95258c..80c1d81 100644
> --- a/virtio/net.c
> +++ b/virtio/net.c
> @@ -333,7 +333,7 @@ static bool virtio_net__tap_init(struct net_dev *ndev)
> goto fail;
> } else if (!skipconf) {
> memset(&ifr, 0, sizeof(ifr));
> - strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name));
> + strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ifr.ifr_name));
Isn't that the old strncpy trap, where we could end up with an
unterminated string? So it should either be "sizeof() - 1" or we use
strlcpy(), which would just require to #include "kvm/strbuf.h".
It seems that this should not happen in this particular case, but still.
I see that we have more strncpy's and friends with the same problems
around, so this fix is still valid, especially if it appeases GCC 8. So
my R-B: still stands, but I think we should fix those as well. I
remember there was once a series to tackle this ...
Cheers,
Andre
> sin.sin_addr.s_addr = inet_addr(params->host_ip);
> memcpy(&(ifr.ifr_addr), &sin, sizeof(ifr.ifr_addr));
> ifr.ifr_addr.sa_family = AF_INET;
> @@ -345,7 +345,7 @@ static bool virtio_net__tap_init(struct net_dev
> *ndev) if (!skipconf) {
> memset(&ifr, 0, sizeof(ifr));
> - strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name));
> + strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ifr.ifr_name));
> ioctl(sock, SIOCGIFFLAGS, &ifr);
> ifr.ifr_flags |= IFF_UP | IFF_RUNNING;
> if (ioctl(sock, SIOCSIFFLAGS, &ifr) < 0)
>
