Re: [LTP] [PATCH] ima_violations.sh: Another fix of condition evaluation

[Petr Vorel <pvorel@suse.cz>]

> Hi!

> On Thu Dec 11, 2025 at 12:10 PM CET, Petr Vorel wrote:
> > c0c35509f9 was not enough to fix evaluation against empty
> > $expected_violations:

> > ima_violations 1 TINFO: verify open writers violation
> > /opt/ltp/testcases/bin/ima_violations.sh: line 96: [: 0: unary operator expected

> > Therefore split checks into two if.

> > Also improvements (readability)
> > * shorten line length with saving subtraction into variable
> > * evaluate empty variable with ${:-}

> > Fixes: 726ed71905 ("ima_violations.sh: Update validate() to support multiple violations")
> > Reported-by: Martin Doucha <mdoucha@suse.cz>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > NOTE: this was found on old SLES 4.4 based kernel which does not log
> > validations. But missing validations might be just a Secure Boot related
> > setup problem:

> > $ mokutil --sb-state
> > Secure Boot: EFI variables not supported on SUT

> > Events are logged when Secure Boot is off:
> > $ mokutil --sb-state
> > SecureBoot disabled

> > Or maybe violations worked differently on the old kernel (I remember
> > only 6.15 change).

> > Kind regards,
> > Petr

> >  .../integrity/ima/tests/ima_violations.sh     | 21 ++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)

> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> > index 1d2f1d9447..a8476e6b59 100755
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> > @@ -87,23 +87,30 @@ validate()
> >  	local search="$3"
> >  	local expected_violations="$4"
> >  	local max_attempt=3
> > -	local count2 i num_violations_new
> > +	local count2 diff i num_violations_new pass

> >  	for i in $(seq 1 $max_attempt); do
> >  		read num_violations_new < $IMA_VIOLATIONS
> >  		count2="$(get_count $search)"
> > -		if [ -z "$expected_violations" -a $(($num_violations_new - $num_violations)) -gt 0 ] || \
> > -		   [ $(($num_violations_new - $num_violations)) -eq $expected_violations ]; then
> > -			[ -z "$expected_violations" ] && expected_violations=1
> > +		diff=$(($num_violations_new - $num_violations))
> > +
> > +		if [ "$expected_violations" ] && [ $diff -eq $expected_violations ]; then
> > +			pass=1
> > +		fi
> > +		if [ -z "$expected_violations" ] && [ $diff -gt 0 ]; then
> > +			pass=1
> > +		fi

> Maybe readability can be improved (well..shell scripts are pretty ugly
> by nature anyway):

> 	diff=$((num_violations_new - num_violations))

> 	if [ "$expected_violations" ]; then
> 		[ $diff -eq $expected_violations ] && pass=1
> 	else
> 		[ $diff -gt 0 ] && pass=1
> 	fi

Thanks, makes sense, I'll modify it before merge.
Feel free to add your RBT/TBT tags if you wish (as you spent time looking into
this).

Kind regards,
Petr

> > +
> > +		if [ "$pass" = 1 ]; then
> >  			if [ $count2 -gt $count ]; then
> > -				tst_res TPASS "$expected_violations $search violation(s) added"
> > +				tst_res TPASS "${expected_violations:-1} $search violation(s) added"
> >  				return
> >  			else
> >  				tst_res TINFO "$search not found in $LOG ($i/$max_attempt attempt)..."
> >  				tst_sleep 1s
> >  			fi
> > -		elif [ $(($num_violations_new - $num_violations)) -gt 0 ]; then
> > -			tst_res $IMA_FAIL "$search too many violations added: $num_violations_new - $num_violations"
> > +		elif [ $diff -gt 0 ]; then
> > +			tst_res $IMA_FAIL "$search too many violations added: $diff ($num_violations_new - $num_violations)"
> >  			return
> >  		else
> >  			tst_res $IMA_FAIL "$search violation not added"

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp