[bpf-next] bpftool: bash-completion: Add nopasswd sudo prefix for bpftool

Commit Message

Rong Tao Feb. 12, 2025, 10:14 a.m. UTC

From: Rong Tao <[e-mail]>

In the bpftool script of bash-completion, many bpftool commands require
superuser privileges to execute. Otherwise, Operation not permission will
be displayed. Here, we check whether ordinary users are exempt from
entering the sudo password. If so, we need to add the sudo prefix to the
bpftool command to be executed. In this way, we can obtain the correct
command completion content instead of the wrong one.

For example, when updating array_of_maps, the wrong 'hex' is completed:

    $ sudo bpftool map update name arr_maps key 0 0 0 0 value [tab]
    $ sudo bpftool map update name arr_maps key 0 0 0 0 value hex

However, what we need is "id name pinned". Similarly, there is the same
problem in getting the map 'name' and 'id':

    $ sudo bpftool map show name [tab] < get nothing
    $ sudo bpftool map show id [tab]   < get nothing

This commit fixes the issue.

    $ sudo bpftool map update name arr_maps key 0 0 0 0 value [tab]
    id      name    pinned

    $ sudo bpftool map show name
    arr_maps         cgroup_hash      inner_arr1       inner_arr2

    $ sudo bpftool map show id
    11    1383  4091  4096

Signed-off-by: Rong Tao <[e-mail]>
---
 tools/bpf/bpftool/bash-completion/bpftool | 29 +++++++++++++++--------
 1 file changed, 19 insertions(+), 10 deletions(-)

Comments

Quentin Monnet Feb. 12, 2025, 11 a.m. UTC | #1

2025-02-12 18:14 UTC+0800 ~ Rong Tao <[e-mail]>
> From: Rong Tao <[e-mail]>
> 
> In the bpftool script of bash-completion, many bpftool commands require
> superuser privileges to execute. Otherwise, Operation not permission will
> be displayed. Here, we check whether ordinary users are exempt from
> entering the sudo password. If so, we need to add the sudo prefix to the
> bpftool command to be executed. In this way, we can obtain the correct
> command completion content instead of the wrong one.
> 
> For example, when updating array_of_maps, the wrong 'hex' is completed:
> 
>     $ sudo bpftool map update name arr_maps key 0 0 0 0 value [tab]
>     $ sudo bpftool map update name arr_maps key 0 0 0 0 value hex
> 
> However, what we need is "id name pinned". Similarly, there is the same
> problem in getting the map 'name' and 'id':
> 
>     $ sudo bpftool map show name [tab] < get nothing
>     $ sudo bpftool map show id [tab]   < get nothing
> 
> This commit fixes the issue.
> 
>     $ sudo bpftool map update name arr_maps key 0 0 0 0 value [tab]
>     id      name    pinned
> 
>     $ sudo bpftool map show name
>     arr_maps         cgroup_hash      inner_arr1       inner_arr2
> 
>     $ sudo bpftool map show id
>     11    1383  4091  4096
> 
> Signed-off-by: Rong Tao <[e-mail]>

Hi, thanks for the patch.

I agree it's annoying to have a partially-working completion for
non-root users, however, I don't feel very comfortable introducing calls
to "sudo" in bash completion, without the user noticing. For what it's
worth, I searched other bash completion files (from
https://github.com/scop/bash-completion/) and I can't find any of them
running sudo to help complete commands, so it doesn't seem to be
something usual in completion. I think I'd rather keep the current state
(or fix the first example to have the right keywords displayed but
without running sudo).

Quentin

Rong Tao Feb. 12, 2025, 11:21 a.m. UTC | #2

On 2/12/25 19:00, Quentin Monnet wrote:
> 2025-02-12 18:14 UTC+0800 ~ Rong Tao <[e-mail]>
>> From: Rong Tao <[e-mail]>
>>
>> In the bpftool script of bash-completion, many bpftool commands require
>> superuser privileges to execute. Otherwise, Operation not permission will
>> be displayed. Here, we check whether ordinary users are exempt from
>> entering the sudo password. If so, we need to add the sudo prefix to the
>> bpftool command to be executed. In this way, we can obtain the correct
>> command completion content instead of the wrong one.
>>
>> For example, when updating array_of_maps, the wrong 'hex' is completed:
>>
>>      $ sudo bpftool map update name arr_maps key 0 0 0 0 value [tab]
>>      $ sudo bpftool map update name arr_maps key 0 0 0 0 value hex
>>
>> However, what we need is "id name pinned". Similarly, there is the same
>> problem in getting the map 'name' and 'id':
>>
>>      $ sudo bpftool map show name [tab] < get nothing
>>      $ sudo bpftool map show id [tab]   < get nothing
>>
>> This commit fixes the issue.
>>
>>      $ sudo bpftool map update name arr_maps key 0 0 0 0 value [tab]
>>      id      name    pinned
>>
>>      $ sudo bpftool map show name
>>      arr_maps         cgroup_hash      inner_arr1       inner_arr2
>>
>>      $ sudo bpftool map show id
>>      11    1383  4091  4096
>>
>> Signed-off-by: Rong Tao <[e-mail]>
> Hi, thanks for the patch.
>
> I agree it's annoying to have a partially-working completion for
> non-root users, however, I don't feel very comfortable introducing calls
> to "sudo" in bash completion, without the user noticing. For what it's
> worth, I searched other bash completion files (from
> https://github.com/scop/bash-completion/) and I can't find any of them
> running sudo to help complete commands, so it doesn't seem to be
> something usual in completion. I think I'd rather keep the current state
> (or fix the first example to have the right keywords displayed but
> without running sudo).

Thanks for the reply.

Using sudo to perform bash-completion is indeed not a perfect solution. 
However, using "bpftool map show" to obtain map information may be the 
only way, and this operation requires CAP_ADMIN, which may be a 
compromise. There is no other way.

Rong Tao

Patch

diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 1ce409a6cbd9..25fb859cdfa4 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -5,6 +5,15 @@ 
 #
 # Author: Quentin Monnet <[e-mail]>
 
+# In the bpftool script of bash-completion, many bpftool commands require
+# superuser privileges to be executed. Otherwise, EPERM will occur. Here,
+# it is detected whether ordinary users are exempt from sudo passwords. If
+# so, it is necessary to add the "sudo" prefix to the required bpftool
+# command execution.
+if sudo --non-interactive true 2>/dev/null; then
+    _sudo=sudo
+fi
+
 # Takes a list of words in argument; each one of them is added to COMPREPLY if
 # it is not already present on the command line. Returns no value.
 _bpftool_once_attr()
@@ -46,7 +55,7 @@  _bpftool_one_of_list()
 
 _bpftool_get_map_ids()
 {
-    COMPREPLY+=( $( compgen -W "$( bpftool -jp map  2>&1 | \
+    COMPREPLY+=( $( compgen -W "$( ${_sudo} bpftool -jp map  2>&1 | \
         command sed -n 's/.*"id": \(.*\),$/\1/p' )" -- "$cur" ) )
 }
 
@@ -54,14 +63,14 @@  _bpftool_get_map_ids()
 _bpftool_get_map_ids_for_type()
 {
     local type="$1"
-    COMPREPLY+=( $( compgen -W "$( bpftool -jp map  2>&1 | \
+    COMPREPLY+=( $( compgen -W "$( ${_sudo} bpftool -jp map  2>&1 | \
         command grep -C2 "$type" | \
         command sed -n 's/.*"id": \(.*\),$/\1/p' )" -- "$cur" ) )
 }
 
 _bpftool_get_map_names()
 {
-    COMPREPLY+=( $( compgen -W "$( bpftool -jp map  2>&1 | \
+    COMPREPLY+=( $( compgen -W "$( ${_sudo} bpftool -jp map  2>&1 | \
         command sed -n 's/.*"name": \(.*\),$/\1/p' )" -- "$cur" ) )
 }
 
@@ -69,38 +78,38 @@  _bpftool_get_map_names()
 _bpftool_get_map_names_for_type()
 {
     local type="$1"
-    COMPREPLY+=( $( compgen -W "$( bpftool -jp map  2>&1 | \
+    COMPREPLY+=( $( compgen -W "$( ${_sudo} bpftool -jp map  2>&1 | \
         command grep -C2 "$type" | \
         command sed -n 's/.*"name": \(.*\),$/\1/p' )" -- "$cur" ) )
 }
 
 _bpftool_get_prog_ids()
 {
-    COMPREPLY+=( $( compgen -W "$( bpftool -jp prog 2>&1 | \
+    COMPREPLY+=( $( compgen -W "$( ${_sudo} bpftool -jp prog 2>&1 | \
         command sed -n 's/.*"id": \(.*\),$/\1/p' )" -- "$cur" ) )
 }
 
 _bpftool_get_prog_tags()
 {
-    COMPREPLY+=( $( compgen -W "$( bpftool -jp prog 2>&1 | \
+    COMPREPLY+=( $( compgen -W "$( ${_sudo} bpftool -jp prog 2>&1 | \
         command sed -n 's/.*"tag": "\(.*\)",$/\1/p' )" -- "$cur" ) )
 }
 
 _bpftool_get_prog_names()
 {
-    COMPREPLY+=( $( compgen -W "$( bpftool -jp prog 2>&1 | \
+    COMPREPLY+=( $( compgen -W "$( ${_sudo} bpftool -jp prog 2>&1 | \
         command sed -n 's/.*"name": "\(.*\)",$/\1/p' )" -- "$cur" ) )
 }
 
 _bpftool_get_btf_ids()
 {
-    COMPREPLY+=( $( compgen -W "$( bpftool -jp btf 2>&1 | \
+    COMPREPLY+=( $( compgen -W "$( ${_sudo} bpftool -jp btf 2>&1 | \
         command sed -n 's/.*"id": \(.*\),$/\1/p' )" -- "$cur" ) )
 }
 
 _bpftool_get_link_ids()
 {
-    COMPREPLY+=( $( compgen -W "$( bpftool -jp link 2>&1 | \
+    COMPREPLY+=( $( compgen -W "$( ${_sudo} bpftool -jp link 2>&1 | \
         command sed -n 's/.*"id": \(.*\),$/\1/p' )" -- "$cur" ) )
 }
 
@@ -156,7 +165,7 @@  _bpftool_map_guess_map_type()
     [[ -z $ref ]] && return 0
 
     local type
-    type=$(bpftool -jp map show $keyword $ref | \
+    type=$(${_sudo} bpftool -jp map show $keyword $ref | \
         command sed -n 's/.*"type": "\(.*\)",$/\1/p')
     [[ -n $type ]] && printf $type
 }