Re: [PATCH iproute2-next] bpf: replace snprintf with asprintf when dealing with long buffers

[David Ahern <dsahern@gmail.com>]

On 9/9/19 10:05 AM, Andrea Claudi wrote:
> This reduces stack usage, as asprintf allocates memory on the heap.
> 
> This indirectly fixes a snprintf truncation warning (from gcc v9.2.1):
> 
> bpf.c: In function ‘bpf_get_work_dir’:
> bpf.c:784:49: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
>   784 |  snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
>       |                                                 ^
> bpf.c:784:2: note: ‘snprintf’ output between 2 and 4097 bytes into a destination of size 4096
>   784 |  snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
>       |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fixes: e42256699cac ("bpf: make tc's bpf loader generic and move into lib")
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
> ---
>  lib/bpf.c | 95 +++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 57 insertions(+), 38 deletions(-)
> 
> diff --git a/lib/bpf.c b/lib/bpf.c
> index 7d2a322ffbaec..18e0334d3f11b 100644
> --- a/lib/bpf.c
> +++ b/lib/bpf.c
> @@ -406,13 +406,14 @@ static int bpf_derive_elf_map_from_fdinfo(int fd, struct bpf_elf_map *map,
>  					  struct bpf_map_ext *ext)
>  {
>  	unsigned int val, owner_type = 0, owner_jited = 0;
> -	char file[PATH_MAX], buff[4096];
> +	char *file, buff[4096];
>  	FILE *fp;
>  
> -	snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd);
> +	asprintf(&file, "/proc/%d/fdinfo/%d", getpid(), fd);

Thanks for taking this on.

From 'man asprintf':
  "If  memory  allocation  wasn't  possible, or some other error occurs,
these functions will return -1, and the contents of strp are undefined."

ie., you should check the return code of asprintf and make sure the
pointer passed is always initialized to NULL (static ones do not need
the init).