[PATCH bpf-next 0/2] tools: bpftool: work with frozen maps

[PATCH bpf-next 0/2] tools: bpftool: work with frozen maps

[Quentin Monnet]

Hi,
This is a simple set to add support for BPF map freezing to bpftool. First
patch makes bpftool indicate if a map is frozen when listing BPF maps.
Second patch adds a command to freeze a map loaded on the system.

Quentin Monnet (2):
  tools: bpftool: show frozen status for maps
  tools: bpftool: add "bpftool map freeze" subcommand

 .../bpf/bpftool/Documentation/bpftool-map.rst |  9 +++
 tools/bpf/bpftool/bash-completion/bpftool     |  4 +-
 tools/bpf/bpftool/map.c                       | 64 +++++++++++++++++--
 3 files changed, 71 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH bpf-next 1/2] tools: bpftool: show frozen status for maps

[Quentin Monnet]

When listing maps, read their "frozen" status from procfs, and tell if
maps are frozen.

As commit log for map freezing command mentions that the feature might
be extended with flags (e.g. for write-only instead of read-only) in the
future, use an integer and not a boolean for JSON output.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/map.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index bfbbc6b4cb83..af2e9eb9747b 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -481,9 +481,11 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 
 static int show_map_close_json(int fd, struct bpf_map_info *info)
 {
-	char *memlock;
+	char *memlock, *frozen_str;
+	int frozen = 0;
 
 	memlock = get_fdinfo(fd, "memlock");
+	frozen_str = get_fdinfo(fd, "frozen");
 
 	jsonw_start_object(json_wtr);
 
@@ -533,6 +535,12 @@ static int show_map_close_json(int fd, struct bpf_map_info *info)
 	}
 	close(fd);
 
+	if (frozen_str) {
+		frozen = atoi(frozen_str);
+		free(frozen_str);
+	}
+	jsonw_int_field(json_wtr, "frozen", frozen);
+
 	if (info->btf_id)
 		jsonw_int_field(json_wtr, "btf_id", info->btf_id);
 
@@ -555,9 +563,11 @@ static int show_map_close_json(int fd, struct bpf_map_info *info)
 
 static int show_map_close_plain(int fd, struct bpf_map_info *info)
 {
-	char *memlock;
+	char *memlock, *frozen_str;
+	int frozen = 0;
 
 	memlock = get_fdinfo(fd, "memlock");
+	frozen_str = get_fdinfo(fd, "frozen");
 
 	printf("%u: ", info->id);
 	if (info->type < ARRAY_SIZE(map_type_name))
@@ -610,9 +620,23 @@ static int show_map_close_plain(int fd, struct bpf_map_info *info)
 				printf("\n\tpinned %s", obj->path);
 		}
 	}
+	printf("\n");
+
+	if (frozen_str) {
+		frozen = atoi(frozen_str);
+		free(frozen_str);
+	}
+
+	if (!info->btf_id && !frozen)
+		return 0;
+
+	printf("\t");
 
 	if (info->btf_id)
-		printf("\n\tbtf_id %d", info->btf_id);
+		printf("btf_id %d", info->btf_id);
+
+	if (frozen)
+		printf("%sfrozen", info->btf_id ? "  " : "");
 
 	printf("\n");
 	return 0;
-- 
2.17.1

[PATCH bpf-next 2/2] tools: bpftool: add "bpftool map freeze" subcommand

[Quentin Monnet]

Add a new subcommand to freeze maps from user space.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../bpf/bpftool/Documentation/bpftool-map.rst |  9 +++++
 tools/bpf/bpftool/bash-completion/bpftool     |  4 +--
 tools/bpf/bpftool/map.c                       | 34 ++++++++++++++++++-
 3 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 61d1d270eb5e..1c0f7146aab0 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -36,6 +36,7 @@ MAP COMMANDS
 |	**bpftool** **map pop**        *MAP*
 |	**bpftool** **map enqueue**    *MAP* **value** *VALUE*
 |	**bpftool** **map dequeue**    *MAP*
+|	**bpftool** **map freeze**     *MAP*
 |	**bpftool** **map help**
 |
 |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
@@ -127,6 +128,14 @@ DESCRIPTION
 	**bpftool map dequeue**  *MAP*
 		  Dequeue and print **value** from the queue.
 
+	**bpftool map freeze**  *MAP*
+		  Freeze the map as read-only from user space. Entries from a
+		  frozen map can not longer be updated or deleted with the
+		  **bpf\ ()** system call. This operation is not reversible,
+		  and the map remains immutable from user space until its
+		  destruction. However, read and write permissions for BPF
+		  programs to the map remain unchanged.
+
 	**bpftool map help**
 		  Print short help message.
 
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 2ffd351f9dbf..70493a6da206 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -449,7 +449,7 @@ _bpftool()
         map)
             local MAP_TYPE='id pinned'
             case $command in
-                show|list|dump|peek|pop|dequeue)
+                show|list|dump|peek|pop|dequeue|freeze)
                     case $prev in
                         $command)
                             COMPREPLY=( $( compgen -W "$MAP_TYPE" -- "$cur" ) )
@@ -638,7 +638,7 @@ _bpftool()
                     [[ $prev == $object ]] && \
                         COMPREPLY=( $( compgen -W 'delete dump getnext help \
                             lookup pin event_pipe show list update create \
-                            peek push enqueue pop dequeue' -- \
+                            peek push enqueue pop dequeue freeze' -- \
                             "$cur" ) )
                     ;;
             esac
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index af2e9eb9747b..de61d73b9030 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -1262,6 +1262,35 @@ static int do_pop_dequeue(int argc, char **argv)
 	return err;
 }
 
+static int do_freeze(int argc, char **argv)
+{
+	int err, fd;
+
+	if (!REQ_ARGS(2))
+		return -1;
+
+	fd = map_parse_fd(&argc, &argv);
+	if (fd < 0)
+		return -1;
+
+	if (argc) {
+		close(fd);
+		return BAD_ARG();
+	}
+
+	err = bpf_map_freeze(fd);
+	close(fd);
+	if (err) {
+		p_err("failed to freeze map: %s", strerror(errno));
+		return err;
+	}
+
+	if (json_output)
+		jsonw_null(json_wtr);
+
+	return 0;
+}
+
 static int do_help(int argc, char **argv)
 {
 	if (json_output) {
@@ -1286,6 +1315,7 @@ static int do_help(int argc, char **argv)
 		"       %s %s pop        MAP\n"
 		"       %s %s enqueue    MAP value VALUE\n"
 		"       %s %s dequeue    MAP\n"
+		"       %s %s freeze     MAP\n"
 		"       %s %s help\n"
 		"\n"
 		"       " HELP_SPEC_MAP "\n"
@@ -1304,7 +1334,8 @@ static int do_help(int argc, char **argv)
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
-		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
+		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
+		bin_name, argv[-2]);
 
 	return 0;
 }
@@ -1326,6 +1357,7 @@ static const struct cmd cmds[] = {
 	{ "enqueue",	do_update },
 	{ "pop",	do_pop_dequeue },
 	{ "dequeue",	do_pop_dequeue },
+	{ "freeze",	do_freeze },
 	{ 0 }
 };
 
-- 
2.17.1

Re: [PATCH bpf-next 2/2] tools: bpftool: add "bpftool map freeze" subcommand

[Daniel Borkmann]

On 8/21/19 10:52 AM, Quentin Monnet wrote:
> Add a new subcommand to freeze maps from user space.
> 
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>   .../bpf/bpftool/Documentation/bpftool-map.rst |  9 +++++
>   tools/bpf/bpftool/bash-completion/bpftool     |  4 +--
>   tools/bpf/bpftool/map.c                       | 34 ++++++++++++++++++-
>   3 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
> index 61d1d270eb5e..1c0f7146aab0 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
> @@ -36,6 +36,7 @@ MAP COMMANDS
>   |	**bpftool** **map pop**        *MAP*
>   |	**bpftool** **map enqueue**    *MAP* **value** *VALUE*
>   |	**bpftool** **map dequeue**    *MAP*
> +|	**bpftool** **map freeze**     *MAP*
>   |	**bpftool** **map help**
>   |
>   |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
> @@ -127,6 +128,14 @@ DESCRIPTION
>   	**bpftool map dequeue**  *MAP*
>   		  Dequeue and print **value** from the queue.
>   
> +	**bpftool map freeze**  *MAP*
> +		  Freeze the map as read-only from user space. Entries from a
> +		  frozen map can not longer be updated or deleted with the
> +		  **bpf\ ()** system call. This operation is not reversible,
> +		  and the map remains immutable from user space until its
> +		  destruction. However, read and write permissions for BPF
> +		  programs to the map remain unchanged.

That is not correct, programs that are loaded into the system /after/ the map
has been frozen cannot modify values either, thus read-only from both sides.

Thanks,
Daniel

Re: [PATCH bpf-next 2/2] tools: bpftool: add "bpftool map freeze" subcommand

[Quentin Monnet]

2019-08-21 13:40 UTC+0200 ~ Daniel Borkmann <daniel@iogearbox.net>
> On 8/21/19 10:52 AM, Quentin Monnet wrote:
>> Add a new subcommand to freeze maps from user space.
>>
>> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
>> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>> ---
>>   .../bpf/bpftool/Documentation/bpftool-map.rst |  9 +++++
>>   tools/bpf/bpftool/bash-completion/bpftool     |  4 +--
>>   tools/bpf/bpftool/map.c                       | 34 ++++++++++++++++++-
>>   3 files changed, 44 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst
>> b/tools/bpf/bpftool/Documentation/bpftool-map.rst
>> index 61d1d270eb5e..1c0f7146aab0 100644
>> --- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
>> +++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
>> @@ -36,6 +36,7 @@ MAP COMMANDS
>>   |    **bpftool** **map pop**        *MAP*
>>   |    **bpftool** **map enqueue**    *MAP* **value** *VALUE*
>>   |    **bpftool** **map dequeue**    *MAP*
>> +|    **bpftool** **map freeze**     *MAP*
>>   |    **bpftool** **map help**
>>   |
>>   |    *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
>> @@ -127,6 +128,14 @@ DESCRIPTION
>>       **bpftool map dequeue**  *MAP*
>>             Dequeue and print **value** from the queue.
>>   +    **bpftool map freeze**  *MAP*
>> +          Freeze the map as read-only from user space. Entries from a
>> +          frozen map can not longer be updated or deleted with the
>> +          **bpf\ ()** system call. This operation is not reversible,
>> +          and the map remains immutable from user space until its
>> +          destruction. However, read and write permissions for BPF
>> +          programs to the map remain unchanged.
> 
> That is not correct, programs that are loaded into the system /after/
> the map
> has been frozen cannot modify values either, thus read-only from both
> sides.
> 
> Thanks,
> Daniel

Hi Daniel,

Are you entirely sure about it? I could not find the relevant
restriction in the code, the checks seem to be on map flags
(BPF_F_RDONLY) which do not seem to be modified by the "frozen" status
in map_freeze()? And tests I ran on my side seem to indicate the map can
still be updated by new programs. Did I miss something?

Tested on 5.3.0-rc1:

1. Create hash map
2. Load BPF program foo, using map
3. Test-run program foo - map is updated
4. Freeze map - update effectively becomes impossible from user space
5. Load BPF program bar, using same map
6. Test-run program bar - map is still updated

Best regards,
Quentin

Re: [PATCH bpf-next 2/2] tools: bpftool: add "bpftool map freeze" subcommand

[Daniel Borkmann]

On 8/21/19 2:58 PM, Quentin Monnet wrote:
> 2019-08-21 13:40 UTC+0200 ~ Daniel Borkmann <daniel@iogearbox.net>
>> On 8/21/19 10:52 AM, Quentin Monnet wrote:
>>> Add a new subcommand to freeze maps from user space.
>>>
>>> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
>>> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>>> ---
>>>    .../bpf/bpftool/Documentation/bpftool-map.rst |  9 +++++
>>>    tools/bpf/bpftool/bash-completion/bpftool     |  4 +--
>>>    tools/bpf/bpftool/map.c                       | 34 ++++++++++++++++++-
>>>    3 files changed, 44 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst
>>> b/tools/bpf/bpftool/Documentation/bpftool-map.rst
>>> index 61d1d270eb5e..1c0f7146aab0 100644
>>> --- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
>>> +++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
>>> @@ -36,6 +36,7 @@ MAP COMMANDS
>>>    |    **bpftool** **map pop**        *MAP*
>>>    |    **bpftool** **map enqueue**    *MAP* **value** *VALUE*
>>>    |    **bpftool** **map dequeue**    *MAP*
>>> +|    **bpftool** **map freeze**     *MAP*
>>>    |    **bpftool** **map help**
>>>    |
>>>    |    *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
>>> @@ -127,6 +128,14 @@ DESCRIPTION
>>>        **bpftool map dequeue**  *MAP*
>>>              Dequeue and print **value** from the queue.
>>>    +    **bpftool map freeze**  *MAP*
>>> +          Freeze the map as read-only from user space. Entries from a
>>> +          frozen map can not longer be updated or deleted with the
>>> +          **bpf\ ()** system call. This operation is not reversible,
>>> +          and the map remains immutable from user space until its
>>> +          destruction. However, read and write permissions for BPF
>>> +          programs to the map remain unchanged.
>>
>> That is not correct, programs that are loaded into the system /after/
>> the map
>> has been frozen cannot modify values either, thus read-only from both
>> sides.
> 
> Are you entirely sure about it? I could not find the relevant
> restriction in the code, the checks seem to be on map flags
> (BPF_F_RDONLY) which do not seem to be modified by the "frozen" status
> in map_freeze()? And tests I ran on my side seem to indicate the map can
> still be updated by new programs. Did I miss something?
> 
> Tested on 5.3.0-rc1:
> 
> 1. Create hash map
> 2. Load BPF program foo, using map
> 3. Test-run program foo - map is updated
> 4. Freeze map - update effectively becomes impossible from user space
> 5. Load BPF program bar, using same map
> 6. Test-run program bar - map is still updated

Looks like I need some more coffee. ;-) Indeed, the program side was via
BPF_F_RDONLY_PROG flag.

Re: [PATCH bpf-next 0/2] tools: bpftool: work with frozen maps

[Daniel Borkmann]

On 8/21/19 10:52 AM, Quentin Monnet wrote:
> Hi,
> This is a simple set to add support for BPF map freezing to bpftool. First
> patch makes bpftool indicate if a map is frozen when listing BPF maps.
> Second patch adds a command to freeze a map loaded on the system.
> 
> Quentin Monnet (2):
>    tools: bpftool: show frozen status for maps
>    tools: bpftool: add "bpftool map freeze" subcommand
> 
>   .../bpf/bpftool/Documentation/bpftool-map.rst |  9 +++
>   tools/bpf/bpftool/bash-completion/bpftool     |  4 +-
>   tools/bpf/bpftool/map.c                       | 64 +++++++++++++++++--
>   3 files changed, 71 insertions(+), 6 deletions(-)
> 

Applied, thanks!