netdev - Re: [PATCH bpf-next 2/3] libbpf: cleanup after partial failure in bpf_object_

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 7 Nov 2018 14:56:48 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Stanislav Fomichev <sdf@...gle.com>
Cc:     netdev@...r.kernel.org, linux-kselftest@...r.kernel.org,
        ast@...nel.org, daniel@...earbox.net, shuah@...nel.org,
        quentin.monnet@...ronome.com, guro@...com,
        jiong.wang@...ronome.com, bhole_prashant_q7@....ntt.co.jp,
        john.fastabend@...il.com, jbenc@...hat.com,
        treeze.taeung@...il.com, yhs@...com, osk@...com,
        sandipan@...ux.vnet.ibm.com
Subject: Re: [PATCH bpf-next 2/3] libbpf: cleanup after partial failure in
 bpf_object__pin

On Wed,  7 Nov 2018 14:43:55 -0800, Stanislav Fomichev wrote:
> bpftool will use bpf_object__pin in the next commit to pin all programs
> and maps from the file; in case of a partial failure, we need to get
> back to the clean state (undo previous program/map pins).
> 
> Signed-off-by: Stanislav Fomichev <sdf@...gle.com>
> ---
>  tools/lib/bpf/libbpf.c | 58 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index d6e62e90e8d4..309abe7196f3 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1803,14 +1803,17 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
>  
>  		len = snprintf(buf, PATH_MAX, "%s/%s", path,
>  			       bpf_map__name(map));
> -		if (len < 0)
> -			return -EINVAL;
> -		else if (len >= PATH_MAX)
> -			return -ENAMETOOLONG;
> +		if (len < 0) {
> +			err = -EINVAL;
> +			goto err_unpin_maps;
> +		} else if (len >= PATH_MAX) {
> +			err = -ENAMETOOLONG;
> +			goto err_unpin_maps;
> +		}
>  
>  		err = bpf_map__pin(map, buf);
>  		if (err)
> -			return err;
> +			goto err_unpin_maps;
>  	}
>  
>  	bpf_object__for_each_program(prog, obj) {
> @@ -1819,17 +1822,52 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
>  
>  		len = snprintf(buf, PATH_MAX, "%s/%s", path,
>  			       prog->section_name);
> -		if (len < 0)
> -			return -EINVAL;
> -		else if (len >= PATH_MAX)
> -			return -ENAMETOOLONG;
> +		if (len < 0) {
> +			err = -EINVAL;
> +			goto err_unpin_programs;
> +		} else if (len >= PATH_MAX) {
> +			err = -ENAMETOOLONG;
> +			goto err_unpin_programs;
> +		}
>  
>  		err = bpf_program__pin(prog, buf);
>  		if (err)
> -			return err;
> +			goto err_unpin_programs;
>  	}
>  
>  	return 0;
> +
> +err_unpin_programs:
> +	bpf_object__for_each_program(prog, obj) {
> +		char buf[PATH_MAX];
> +		int len;
> +
> +		len = snprintf(buf, PATH_MAX, "%s/%s", path,
> +			       prog->section_name);
> +		if (len < 0)
> +			continue;
> +		else if (len >= PATH_MAX)
> +			continue;
> +
> +		unlink(buf);

I think that's no bueno, if pin failed because the file already exists
you'll now remove that already existing file.

> +	}
> +
> +err_unpin_maps:
> +	bpf_map__for_each(map, obj) {
> +		char buf[PATH_MAX];
> +		int len;
> +
> +		len = snprintf(buf, PATH_MAX, "%s/%s", path,
> +			       bpf_map__name(map));
> +		if (len < 0)
> +			continue;
> +		else if (len >= PATH_MAX)
> +			continue;
> +
> +		unlink(buf);
> +	}
> +
> +	return err;
>  }
>  
>  void bpf_object__close(struct bpf_object *obj)