On Tue, Jul 23, 2019 at 5:08 AM Ilya Maximets <i.maximets@xxxxxxxxxxx> wrote:
>
> 'channels.max_combined' initialized only on ioctl success and
> errno is only valid on ioctl failure.
>
> The code doesn't produce any runtime issues, but makes memory
> sanitizers angry:
>
> Conditional jump or move depends on uninitialised value(s)
> at 0x55C056F: xsk_get_max_queues (xsk.c:336)
> by 0x55C05B2: xsk_create_bpf_maps (xsk.c:354)
> by 0x55C089F: xsk_setup_xdp_prog (xsk.c:447)
> by 0x55C0E57: xsk_socket__create (xsk.c:601)
> Uninitialised value was created by a stack allocation
> at 0x55C04CD: xsk_get_max_queues (xsk.c:318)
>
> Additionally fixed warning on uninitialized bytes in ioctl arguments:
>
> Syscall param ioctl(SIOCETHTOOL) points to uninitialised byte(s)
> at 0x648D45B: ioctl (in /usr/lib64/libc-2.28.so)
> by 0x55C0546: xsk_get_max_queues (xsk.c:330)
> by 0x55C05B2: xsk_create_bpf_maps (xsk.c:354)
> by 0x55C089F: xsk_setup_xdp_prog (xsk.c:447)
> by 0x55C0E57: xsk_socket__create (xsk.c:601)
> Address 0x1ffefff378 is on thread 1's stack
> in frame #1, created by xsk_get_max_queues (xsk.c:318)
> Uninitialised value was created by a stack allocation
> at 0x55C04CD: xsk_get_max_queues (xsk.c:318)
>
> CC: Magnus Karlsson <magnus.karlsson@xxxxxxxxx>
> Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
> Signed-off-by: Ilya Maximets <i.maximets@xxxxxxxxxxx>
> ---
> tools/lib/bpf/xsk.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index 5007b5d4fd2c..c4f912dc30f9 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -317,7 +317,7 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk)
>
> static int xsk_get_max_queues(struct xsk_socket *xsk)
> {
> - struct ethtool_channels channels;
> + struct ethtool_channels channels = { .cmd = ETHTOOL_GCHANNELS };
> struct ifreq ifr;
> int fd, err, ret;
>
> @@ -325,7 +325,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
> if (fd < 0)
> return -errno;
>
> - channels.cmd = ETHTOOL_GCHANNELS;
> + memset(&ifr, 0, sizeof(ifr));
I refactored this bit into
struct ifreq ifr = {};
based on Andrii suggestion and pushed to bpf tree.
Thanks
> ifr.ifr_data = (void *)&channels;
> strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
> ifr.ifr_name[IFNAMSIZ - 1] = '\0';
> @@ -335,7 +335,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
> goto out;
> }
>
> - if (channels.max_combined == 0 || errno == EOPNOTSUPP)
> + if (err || channels.max_combined == 0)
> /* If the device says it has no channels, then all traffic
> * is sent to a single stream, so max queues = 1.
> */
> --
> 2.17.1
>
