[ofa-general] [PATCH] infiniband-diags: terminate perl scripts with error if not root
Timothy A. Meier
meier3 at llnl.gov
Thu May 29 14:43:50 PDT 2008
Sasha Khapyorsky wrote:
> Hi Tim, Ira,
>
> On 08:58 Fri 23 May , Timothy A. Meier wrote:
>> Following Hals advice, authorization is based on the umad permissions.
>
> I will send some more comments about this method later today. But
> basically still think that some things could be broken and that it is
> not really trivial to separate in this way wrong usage from desired
> behavior reliably (with some approximation it is possible of course).
>
>> The intent is simply to provide a consistent and
>> non-silent fail mechanism.
>
> OTOH I fully agree with yours and Ira's arguments about this - 'Silent'
> fails are bad. I thought about how to solve this and and started to run
> diag perl scripts from unprivileged account in various conditions (cache
> file exists or not, cache dir is readable or not, etc.).
>
> First thing I saw was that even on bad usage most scripts return 0. Then
> I found that on many failures return status is not checked or ignored
> and program return 0. I did those two patches (below) and up to now it
> works fine for me (but likely I didn't cover everything). What do you
> say?
>
I think this patch is fine, and helps solve the improper "usage" issue.
(btw - should we prefer the "adapter" spelling over "adaptor"?)
My patch was addressing non-authorized use. Our philosophy was to not allow
"any" sort of functionality (even help) if not authorized. Fail, and provide
a reason/code.
So rather than go through each perl script to see if the proper thing is done
(return code is checked, error msg provided, terminate, etc.) each time a
privileged function is invoked, we just do it at the beginning of the script,
using a common (consistent) function call ( auth_check() ).
I don't know if this is the desired behavior, but it would have caught a few
problems we have encountered with "silent" failures that produce misleading
results. It would also catch any future (unauthorized) scripting issues.
On 5-23, I submitted a patch which adds an auth_check() function to the common
perl module. I agree, the implementation is non-ideal, but it is probably
sufficient for the vast majority of installations.
If you think the concept of an auth_check() function is desirable/acceptable,
then I will pursue fixing the implementation in a more universal way.
> Sasha
>
>
>>From cbbc155996c9f6efe91b78f055a643809b997468 Mon Sep 17 00:00:00 2001
> From: root <root at castor.voltaire.com>
> Date: Sat, 24 May 2008 11:04:08 +0300
> Subject: [PATCH] infiniband-diags/scripts/*.pl: exit 2 on usage errors
>
> Add non-zero exit status (2) on usage errors for perl scripts.
>
> Signed-off-by: root <root at castor.voltaire.com>
> ---
> infiniband-diags/scripts/check_lft_balance.pl | 2 +-
> infiniband-diags/scripts/ibfindnodesusing.pl | 2 +-
> infiniband-diags/scripts/ibidsverify.pl | 2 +-
> infiniband-diags/scripts/iblinkinfo.pl | 2 +-
> infiniband-diags/scripts/ibprintca.pl | 2 +-
> infiniband-diags/scripts/ibprintrt.pl | 2 +-
> infiniband-diags/scripts/ibprintswitch.pl | 2 +-
> infiniband-diags/scripts/ibqueryerrors.pl | 2 +-
> infiniband-diags/scripts/ibswportwatch.pl | 2 +-
> 9 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/infiniband-diags/scripts/check_lft_balance.pl b/infiniband-diags/scripts/check_lft_balance.pl
> index 66f5f0f..b0f0fef 100755
> --- a/infiniband-diags/scripts/check_lft_balance.pl
> +++ b/infiniband-diags/scripts/check_lft_balance.pl
> @@ -70,7 +70,7 @@ sub usage
> print "Usage: $prog [-R -v]\n";
> print " -R recalculate all cached information\n";
> print " -v verbose output\n";
> - exit 0;
> + exit 2;
> }
>
> sub is_port_up
> diff --git a/infiniband-diags/scripts/ibfindnodesusing.pl b/infiniband-diags/scripts/ibfindnodesusing.pl
> index 1bf0987..71656b3 100755
> --- a/infiniband-diags/scripts/ibfindnodesusing.pl
> +++ b/infiniband-diags/scripts/ibfindnodesusing.pl
> @@ -80,7 +80,7 @@ sub usage_and_exit
> print " -R Recalculate ibnetdiscover information\n";
> print " -C <ca_name> use selected Channel Adaptor name for queries\n";
> print " -P <ca_port> use selected channel adaptor port for queries\n";
> - exit 0;
> + exit 2;
> }
>
> my $argv0 = `basename $0`;
> diff --git a/infiniband-diags/scripts/ibidsverify.pl b/infiniband-diags/scripts/ibidsverify.pl
> index de78e6b..1a236c8 100755
> --- a/infiniband-diags/scripts/ibidsverify.pl
> +++ b/infiniband-diags/scripts/ibidsverify.pl
> @@ -46,7 +46,7 @@ sub usage_and_exit
> print " -h This help message\n";
> print
> " -R Recalculate ibnetdiscover information (Default is to reuse ibnetdiscover output)\n";
> - exit 0;
> + exit 2;
> }
>
> my $argv0 = `basename $0`;
> diff --git a/infiniband-diags/scripts/iblinkinfo.pl b/infiniband-diags/scripts/iblinkinfo.pl
> index a195474..a7a3df5 100755
> --- a/infiniband-diags/scripts/iblinkinfo.pl
> +++ b/infiniband-diags/scripts/iblinkinfo.pl
> @@ -62,7 +62,7 @@ sub usage_and_exit
> print " -C <ca_name> use selected Channel Adaptor name for queries\n";
> print " -P <ca_port> use selected channel adaptor port for queries\n";
> print " -g print port guids instead of node guids\n";
> - exit 0;
> + exit 2;
> }
>
> my $argv0 = `basename $0`;
> diff --git a/infiniband-diags/scripts/ibprintca.pl b/infiniband-diags/scripts/ibprintca.pl
> index 38b4330..0baea0b 100755
> --- a/infiniband-diags/scripts/ibprintca.pl
> +++ b/infiniband-diags/scripts/ibprintca.pl
> @@ -51,7 +51,7 @@ sub usage_and_exit
> print " -l list cas\n";
> print " -C <ca_name> use selected channel adaptor name for queries\n";
> print " -P <ca_port> use selected channel adaptor port for queries\n";
> - exit 0;
> + exit 2;
> }
>
> my $argv0 = `basename $0`;
> diff --git a/infiniband-diags/scripts/ibprintrt.pl b/infiniband-diags/scripts/ibprintrt.pl
> index 86dcb64..0b3db19 100755
> --- a/infiniband-diags/scripts/ibprintrt.pl
> +++ b/infiniband-diags/scripts/ibprintrt.pl
> @@ -51,7 +51,7 @@ sub usage_and_exit
> print " -l list rts\n";
> print " -C <ca_name> use selected channel adaptor name for queries\n";
> print " -P <ca_port> use selected channel adaptor port for queries\n";
> - exit 0;
> + exit 2;
> }
>
> my $argv0 = `basename $0`;
> diff --git a/infiniband-diags/scripts/ibprintswitch.pl b/infiniband-diags/scripts/ibprintswitch.pl
> index 6712201..c7377a9 100755
> --- a/infiniband-diags/scripts/ibprintswitch.pl
> +++ b/infiniband-diags/scripts/ibprintswitch.pl
> @@ -50,7 +50,7 @@ sub usage_and_exit
> print " -l list switches\n";
> print " -C <ca_name> use selected channel adaptor name for queries\n";
> print " -P <ca_port> use selected channel adaptor port for queries\n";
> - exit 0;
> + exit 2;
> }
>
> my $argv0 = `basename $0`;
> diff --git a/infiniband-diags/scripts/ibqueryerrors.pl b/infiniband-diags/scripts/ibqueryerrors.pl
> index c807c02..5f2e167 100755
> --- a/infiniband-diags/scripts/ibqueryerrors.pl
> +++ b/infiniband-diags/scripts/ibqueryerrors.pl
> @@ -149,7 +149,7 @@ sub usage_and_exit
> print " -d include the data counters in the output\n";
> print " -C <ca_name> use selected Channel Adaptor name for queries\n";
> print " -P <ca_port> use selected channel adaptor port for queries\n";
> - exit 0;
> + exit 2;
> }
>
> my $argv0 = `basename $0`;
> diff --git a/infiniband-diags/scripts/ibswportwatch.pl b/infiniband-diags/scripts/ibswportwatch.pl
> index 6d6ba1c..d888f51 100755
> --- a/infiniband-diags/scripts/ibswportwatch.pl
> +++ b/infiniband-diags/scripts/ibswportwatch.pl
> @@ -81,7 +81,7 @@ sub usage_and_exit
> print " -n <cycles> run n cycles then exit (default -1 == forever)\n";
> print " -G Address provided is a GUID\n";
> print " -b report bytes/second packets/second\n";
> - exit 0;
> + exit 2;
> }
>
> # =========================================================================
--
Timothy A. Meier
Computer Scientist
ICCD/High Performance Computing
925.422.3341
meier3 at llnl.gov
More information about the general
mailing list