[bpf,0/3] AF_XDP small fixes

Message

Maciej Fijalkowski March 3, 2021, 3:51 p.m. UTC

Hi,

This time three no-brainers, one is a fix that I sent among with
bpf_link for AF_XDP set where John suggested that it should land in bpf
tree.

Other is the missing munmap on xdpsock and some ancient function
declaration removal.

Thanks!
Maciej

Maciej Fijalkowski (3):
  xsk: remove dangling function declaration from header file
  samples: bpf: add missing munmap in xdpsock
  libbpf: clear map_info before each bpf_obj_get_info_by_fd

 include/linux/netdevice.h  | 2 --
 samples/bpf/xdpsock_user.c | 2 ++
 tools/lib/bpf/xsk.c        | 5 +++--
 3 files changed, 5 insertions(+), 4 deletions(-)

Comments

Björn Töpel March 3, 2021, 4:33 p.m. UTC | #1

On 2021-03-03 16:51, Maciej Fijalkowski wrote:
> xsk_lookup_bpf_maps, based on prog_fd, looks whether current prog has a
> reference to XSKMAP. BPF prog can include insns that work on various BPF
> maps and this is covered by iterating through map_ids.
> 
> The bpf_map_info that is passed to bpf_obj_get_info_by_fd for filling
> needs to be cleared at each iteration, so that it doesn't contain any
> outdated fields and that is currently missing in the function of
> interest.
> 
> To fix that, zero-init map_info via memset before each
> bpf_obj_get_info_by_fd call.
> 
> Also, since the area of this code is touched, in general strcmp is
> considered harmful, so let's convert it to strncmp and provide the
> length of the array name from current map_info.
> 
> Last but not least, do s/continue/break/ once we have found the xsks_map
> to terminate the search.
> 
> Fixes: 5750902a6e9b ("libbpf: proper XSKMAP cleanup")
> Signed-off-by: Maciej Fijalkowski <[e-mail]>
> ---
>   tools/lib/bpf/xsk.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index ffbb588724d8..e56b2d76efc2 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -610,15 +610,16 @@ static int xsk_lookup_bpf_maps(struct xsk_socket *xsk)
>   		if (fd < 0)
>   			continue;
>   
> +		memset(&map_info, 0, map_len);
>   		err = bpf_obj_get_info_by_fd(fd, &map_info, &map_len);
>   		if (err) {
>   			close(fd);
>   			continue;
>   		}
>   
> -		if (!strcmp(map_info.name, "xsks_map")) {
> +		if (!strncmp(map_info.name, "xsks_map", strlen(map_info.name))) {

This should be sizeof(map_info.name) and not strlen, otherwise it's kind
of useless! :-P


Björn

>   			ctx->xsks_map_fd = fd;
> -			continue;
> +			break;
>   		}
>   
>   		close(fd);
>