linux-kernel - Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 8 Dec 2017 14:12:59 +0000
From:   Roman Gushchin <guro@...com>
To:     Quentin Monnet <quentin.monnet@...ronome.com>
CC:     <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <kernel-team@...com>, <ast@...nel.org>, <daniel@...earbox.net>,
        <jakub.kicinski@...ronome.com>, <kafai@...com>,
        David Ahern <dsahern@...il.com>
Subject: Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations

On Fri, Dec 08, 2017 at 10:34:16AM +0000, Quentin Monnet wrote:
> 2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <guro@...com>
> > This patch adds basic cgroup bpf operations to bpftool:
> > cgroup list, attach and detach commands.
> > 
> > Usage is described in the corresponding man pages,
> > and examples are provided.
[...]
> > +MAP COMMANDS
> > +=============
> > +
> > +|	**bpftool** **cgroup list** *CGROUP*
> > +|	**bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
> > +|	**bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
> > +|	**bpftool** **cgroup help**
> > +|
> > +|	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
> 
> Could you please give the different possible values for ATTACH_TYPE and
> ATTACH_FLAGS, and provide some documentation for the flags?

I intentionally didn't include the list of possible values, as it depends
on the exact kernel version, and other bpftool docs are carefully avoiding
specifying such things.

It would be nice to have a way to ask the kernel about provided bpf program types,
attach types, etc; but I'm not sure that hardcoding it in bpftool docs is
a good idea.

> 
> > +
> > +DESCRIPTION
> > +===========
> > +	**bpftool cgroup list** *CGROUP*
> > +		  List all programs attached to the cgroup *CGROUP*.
> > +
> > +		  Output will start with program ID followed by attach type,
> > +		  attach flags and program name.
> > +
> > +	**bpftool cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
> > +		  Attach program *PROG* to the cgroup *CGROUP* with attach type
> > +		  *ATTACH_TYPE* and optional *ATTACH_FLAGS*.
[...]
> > +
> > +	attach_type = parse_attach_type(argv[1]);
> > +	if (attach_type == __MAX_BPF_ATTACH_TYPE) {
> > +		p_err("invalid attach type");
> > +		goto exit_cgroup;
> > +	}
> > +
> > +	argc -= 2;
> > +	argv = &argv[2];
> > +	prog_fd = prog_parse_fd(&argc, &argv);
> > +	if (prog_fd < 0)
> > +		goto exit_cgroup;
> > +
> > +	if (bpf_prog_detach2(prog_fd, cgroup_fd, attach_type)) {
> > +		p_err("failed to attach program");
> 
> Failed to *detach* instead of “attach”.

Fixed.

> 
> > +		goto exit_prog;
> > +	}
> > +
> > +	if (json_output)
> > +		jsonw_null(json_wtr);
> > +
> > +	ret = 0;
> > +
> > +exit_prog:
> > +	close(prog_fd);
> > +exit_cgroup:
> > +	close(cgroup_fd);
> > +exit:
> > +	return ret;
> > +}
> 
> […]
> 
> Very nice work on this v2, thanks a lot!
> Quentin

Thank you for reviewing!