netdev - Re: [v4 2/4] samples: pktgen: fix proc

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, 4 Oct 2019 15:24:09 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     "Daniel T. Lee" <danieltimlee@...il.com>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        brouer@...hat.com
Subject: Re: [v4 2/4] samples: pktgen: fix proc_cmd command result check
 logic

On Fri,  4 Oct 2019 10:32:59 +0900
"Daniel T. Lee" <danieltimlee@...il.com> wrote:

> Currently, proc_cmd is used to dispatch command to 'pg_ctrl', 'pg_thread',
> 'pg_set'. proc_cmd is designed to check command result with grep the
> "Result:", but this might fail since this string is only shown in
> 'pg_thread' and 'pg_set'.
> 
> This commit fixes this logic by grep-ing the "Result:" string only when
> the command is not for 'pg_ctrl'.
> 
> For clarity of an execution flow, 'errexit' flag has been set.
> 
> To cleanup pktgen on exit, trap has been added for EXIT signal.
> 
> Signed-off-by: Daniel T. Lee <danieltimlee@...il.com>
> ---
>  samples/pktgen/functions.sh | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh
> index 4af4046d71be..e1865660b033 100644
> --- a/samples/pktgen/functions.sh
> +++ b/samples/pktgen/functions.sh
> @@ -5,6 +5,8 @@
>  # Author: Jesper Dangaaard Brouer
>  # License: GPL
>  
> +set -o errexit
> +
>  ## -- General shell logging cmds --
>  function err() {
>      local exitcode=$1
> @@ -58,6 +60,7 @@ function pg_set() {
>  function proc_cmd() {
>      local result
>      local proc_file=$1
> +    local status=0
>      # after shift, the remaining args are contained in $@
>      shift
>      local proc_ctrl=${PROC_DIR}/$proc_file
> @@ -73,13 +76,14 @@ function proc_cmd() {
>  	echo "cmd: $@ > $proc_ctrl"
>      fi
>      # Quoting of "$@" is important for space expansion
> -    echo "$@" > "$proc_ctrl"
> -    local status=$?
> -
> -    result=$(grep "Result: OK:" $proc_ctrl)
> -    # Due to pgctrl, cannot use exit code $? from grep
> -    if [[ "$result" == "" ]]; then
> -	grep "Result:" $proc_ctrl >&2
> +    echo "$@" > "$proc_ctrl" || status=$?
> +
> +    if [[ "$proc_file" != "pgctrl" ]]; then
> +        result=$(grep "Result: OK:" $proc_ctrl) || true
> +        # Due to pgctrl, cannot use exit code $? from grep

Is this comment still relevant?  You just excluded "pgctrl" from
getting into this section.

> +        if [[ "$result" == "" ]]; then
> +        grep "Result:" $proc_ctrl >&2

Missing tap/indention?

> +        fi
>      fi
>      if (( $status != 0 )); then
>  	err 5 "Write error($status) occurred cmd: \"$@ > $proc_ctrl\""
> @@ -105,6 +109,8 @@ function pgset() {
>      fi
>  }
>  
> +trap 'pg_ctrl "reset"' EXIT
> +

This line is activated when I ctrl-C the scripts, but something weird
happens, it reports:

 ERROR: proc file:/proc/net/pktgen/pgctrl not writable, not root?!


>  ## -- General shell tricks --
>  
>  function root_check_run_with_sudo() {



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer