| Message ID | 20190821085219.30387-3-quentin.monnet@netronome.com |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | tools: bpftool: work with frozen maps | expand |
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
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
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.
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 } };