[LTP] [PATCH 1/3] commands/keyctl01: Fix potential infinite loop

[LTP] [PATCH 1/3] commands/keyctl01: Fix potential infinite loop

[Guangwen Feng]

End the loop when max key quota is less than or equal to current
key usage, in case some errors happen and result in endless loop.

We expect the test to trigger key quota excess which breaks the
loop but it may not happen, so add a TWARN message for this.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/commands/keyctl/keyctl01.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
index 076a130..5a97499 100644
--- a/testcases/commands/keyctl/keyctl01.sh
+++ b/testcases/commands/keyctl/keyctl01.sh
@@ -62,15 +62,17 @@ cleanup()
 
 do_test()
 {
+	local quota_excd=0
 	local maxkeysz=$((ORIG_KEYSZ + 100))
 
-	while true
+	while [ $maxkeysz -gt $ORIG_KEYSZ ]
 	do
 		echo $maxkeysz >$PATH_KEYQUOTA
 
 		keyctl request2 user debug:fred negate @t >temp 2>&1
 		grep -q -E "quota exceeded" temp
 		if [ $? -eq 0 ]; then
+			quota_excd=1
 			break
 		fi
 
@@ -83,6 +85,10 @@ do_test()
 		((maxkeysz -= 4))
 	done
 
+	if [ $quota_excd -eq 0 ]; then
+		tst_res TWARN "Failed to trigger the quota excess"
+	fi
+
 	tst_res TPASS "Bug not reproduced"
 }
 
-- 
1.8.4.2

[LTP] [PATCH 2/3] commands/keyctl01: Fix getting key serial number

[Guangwen Feng]

We expect to get the key serial number by matching "debug:fred":

Session Keyring
1072563454 --alswrv      0     0  keyring: _ses
 340758544 --alswrv      0 65534   \_ keyring: _uid.0
 783746283 --alswrv      0     0   \_ user: debug:fred

But in some kernels the key will be inaccessible immediately
after being requested with "debug:fred negate":

Session Keyring
        -3 --alswrv      0     0  keyring: _ses
 842289032 --alswrv      0    -1   \_ keyring: _uid.0
877125164: key inaccessible (Permission denied)

So fix this by adding a match with "inaccessible".

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/commands/keyctl/keyctl01.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
index 5a97499..8ea2b25 100644
--- a/testcases/commands/keyctl/keyctl01.sh
+++ b/testcases/commands/keyctl/keyctl01.sh
@@ -77,6 +77,11 @@ do_test()
 		fi
 
 		local key=`keyctl show | awk '/debug:fred/ {print $1}'`
+		if [ -z "$key" ]; then
+			key=`keyctl show | \
+				awk -F ':' '/inaccessible/ {print $1}'`
+		fi
+
 		if [ -n "$key" ]; then
 			keyctl unlink $key @s >/dev/null
 			tst_sleep 50ms
-- 
1.8.4.2

[LTP] [PATCH 2/3] commands/keyctl01: Fix getting key serial number

[Cyril Hrubis]

Hi!
> So fix this by adding a match with "inaccessible".
> 
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> ---
>  testcases/commands/keyctl/keyctl01.sh | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
> index 5a97499..8ea2b25 100644
> --- a/testcases/commands/keyctl/keyctl01.sh
> +++ b/testcases/commands/keyctl/keyctl01.sh
> @@ -77,6 +77,11 @@ do_test()
>  		fi
>  
>  		local key=`keyctl show | awk '/debug:fred/ {print $1}'`
> +		if [ -z "$key" ]; then
> +			key=`keyctl show | \
> +				awk -F ':' '/inaccessible/ {print $1}'`
> +		fi

Can't we rather split the keyctl request and keyctl negate operations
into two and get the key after the key has been requested but before it
was negated?

Or is it required to do the request and negate operation in a signle
keyctl command in order to reproduce the kernel crash?

>  		if [ -n "$key" ]; then
>  			keyctl unlink $key @s >/dev/null
>  			tst_sleep 50ms
> -- 
> 1.8.4.2
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/3] commands/keyctl01: Fix getting key serial number

[Guangwen Feng]

Hi!

Thanks for your review.

在 07/07/2017 09:08 PM, Cyril Hrubis 写道:
> Hi!
>> So fix this by adding a match with "inaccessible".
>>
>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>> ---
>>  testcases/commands/keyctl/keyctl01.sh | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
>> index 5a97499..8ea2b25 100644
>> --- a/testcases/commands/keyctl/keyctl01.sh
>> +++ b/testcases/commands/keyctl/keyctl01.sh
>> @@ -77,6 +77,11 @@ do_test()
>>  		fi
>>  
>>  		local key=`keyctl show | awk '/debug:fred/ {print $1}'`
>> +		if [ -z "$key" ]; then
>> +			key=`keyctl show | \
>> +				awk -F ':' '/inaccessible/ {print $1}'`
>> +		fi
> 
> Can't we rather split the keyctl request and keyctl negate operations
> into two and get the key after the key has been requested but before it
> was negated?
> 
> Or is it required to do the request and negate operation in a signle
> keyctl command in order to reproduce the kernel crash?

I think it is required to do the operations in one command...
I will try to split them and reproduce the kernel crash, thanks.

Best Regards,
Guangwen Feng

> 
>>  		if [ -n "$key" ]; then
>>  			keyctl unlink $key @s >/dev/null
>>  			tst_sleep 50ms
>> -- 
>> 1.8.4.2
>>
>>
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

[LTP] [PATCH 2/3] commands/keyctl01: Fix getting key serial number

[Guangwen Feng]

Hi!

在 07/11/2017 08:27 PM, Guangwen Feng 写道:
> Hi!
> 
> Thanks for your review.
> 
> 在 07/07/2017 09:08 PM, Cyril Hrubis 写道:
>> Hi!
>>> So fix this by adding a match with "inaccessible".
>>>
>>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>>> ---
>>>  testcases/commands/keyctl/keyctl01.sh | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
>>> index 5a97499..8ea2b25 100644
>>> --- a/testcases/commands/keyctl/keyctl01.sh
>>> +++ b/testcases/commands/keyctl/keyctl01.sh
>>> @@ -77,6 +77,11 @@ do_test()
>>>  		fi
>>>  
>>>  		local key=`keyctl show | awk '/debug:fred/ {print $1}'`
>>> +		if [ -z "$key" ]; then
>>> +			key=`keyctl show | \
>>> +				awk -F ':' '/inaccessible/ {print $1}'`
>>> +		fi
>>
>> Can't we rather split the keyctl request and keyctl negate operations
>> into two and get the key after the key has been requested but before it
>> was negated?
>>
>> Or is it required to do the request and negate operation in a signle
>> keyctl command in order to reproduce the kernel crash?
> 
> I think it is required to do the operations in one command...
> I will try to split them and reproduce the kernel crash, thanks.

Without using a single keyctl command, this bug cannot be triggered,
so I want to keep it this way.

I think the problem is that we cannot get the keyid by matching
"debug:fred" via "keyctl show" when the key is expired, I find that
in /proc/keys even the key is expired, we can still see the key's
description, so we can just look it up this way.

I will send a V2 soon.

Best Regards,
Guangwen Feng

> 
> Best Regards,
> Guangwen Feng
> 
>>
>>>  		if [ -n "$key" ]; then
>>>  			keyctl unlink $key @s >/dev/null
>>>  			tst_sleep 50ms
>>> -- 
>>> 1.8.4.2
>>>
>>>
>>>
>>>
>>> -- 
>>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>
> 
> 
> 

[LTP] [PATCH 2/3] commands/keyctl01: Fix getting key serial number

[Guangwen Feng]

Hi!

在 07/13/2017 06:55 PM, Guangwen Feng 写道:
> Hi!
> 
> 在 07/11/2017 08:27 PM, Guangwen Feng 写道:
>> Hi!
>>
>> Thanks for your review.
>>
>> 在 07/07/2017 09:08 PM, Cyril Hrubis 写道:
>>> Hi!
>>>> So fix this by adding a match with "inaccessible".
>>>>
>>>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>>>> ---
>>>>  testcases/commands/keyctl/keyctl01.sh | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
>>>> index 5a97499..8ea2b25 100644
>>>> --- a/testcases/commands/keyctl/keyctl01.sh
>>>> +++ b/testcases/commands/keyctl/keyctl01.sh
>>>> @@ -77,6 +77,11 @@ do_test()
>>>>  		fi
>>>>  
>>>>  		local key=`keyctl show | awk '/debug:fred/ {print $1}'`
>>>> +		if [ -z "$key" ]; then
>>>> +			key=`keyctl show | \
>>>> +				awk -F ':' '/inaccessible/ {print $1}'`
>>>> +		fi
>>>
>>> Can't we rather split the keyctl request and keyctl negate operations
>>> into two and get the key after the key has been requested but before it
>>> was negated?
>>>
>>> Or is it required to do the request and negate operation in a signle
>>> keyctl command in order to reproduce the kernel crash?
>>
>> I think it is required to do the operations in one command...
>> I will try to split them and reproduce the kernel crash, thanks.
> 
> Without using a single keyctl command, this bug cannot be triggered,
> so I want to keep it this way.
> 
> I think the problem is that we cannot get the keyid by matching
> "debug:fred" via "keyctl show" when the key is expired, I find that
> in /proc/keys even the key is expired, we can still see the key's
> description, so we can just look it up this way.

Sorry, but there are also some old kernels like 2.6.18-398.el5 do not
show user key in /proc/keys: 

[root@rhel5 ~]# cat /proc/keys
00000001 I-----     1 perm 1f3f0000     0     0 keyring   _uid_ses.0: 1/4
00000002 I-----     3 perm 1f3f0000     0     0 keyring   _uid.0: empty
039d3f23 I--Q--     3 perm 1f3f0000     0     0 keyring   _ses.2742: 6/8
                                                                     ^
                                                        6 keys in this keyring

It only shows how many keys in a keyring, so this is unreliable...

Can we just use "keyctl show" to match "inaccessible" when this
happens?


Best Regards,
Guangwen Feng

> 
> I will send a V2 soon.
> 
> Best Regards,
> Guangwen Feng
> 
>>
>> Best Regards,
>> Guangwen Feng
>>
>>>
>>>>  		if [ -n "$key" ]; then
>>>>  			keyctl unlink $key @s >/dev/null
>>>>  			tst_sleep 50ms
>>>> -- 
>>>> 1.8.4.2
>>>>
>>>>
>>>>
>>>>
>>>> -- 
>>>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>>
>>
>>
>>
> 
> 
> 

[LTP] [PATCH 2/3] commands/keyctl01: Fix getting key serial number

[Cyril Hrubis]

Hi!
> Can we just use "keyctl show" to match "inaccessible" when this
> happens?

Sure, if there is no better way let's go with the original version.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 1/3] commands/keyctl01: Fix potential infinite loop

[Guangwen Feng]

End the loop once the max key quota is less than zero, in case
some errors happen and result in endless loop.

We expect the test to trigger key quota excess which breaks the
loop but it may not happen, so add a TWARN message for this.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/commands/keyctl/keyctl01.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
index 690fbbf..238b117 100644
--- a/testcases/commands/keyctl/keyctl01.sh
+++ b/testcases/commands/keyctl/keyctl01.sh
@@ -61,15 +61,17 @@ cleanup()
 
 do_test()
 {
+	local quota_excd=0
 	local maxkeysz=$((ORIG_KEYSZ + 100))
 
-	while true
+	while [ $maxkeysz -ge 0 ]
 	do
 		echo $maxkeysz >$PATH_KEYQUOTA
 
 		keyctl request2 user debug:fred negate @t >temp 2>&1
 		grep -q -E "quota exceeded" temp
 		if [ $? -eq 0 ]; then
+			quota_excd=1
 			break
 		fi
 
@@ -82,6 +84,10 @@ do_test()
 		((maxkeysz -= 4))
 	done
 
+	if [ $quota_excd -eq 0 ]; then
+		tst_res TWARN "Failed to trigger the quota excess"
+	fi
+
 	tst_res TPASS "Bug not reproduced"
 }
 
-- 
2.9.4

[LTP] [PATCH v2 2/3] commands/keyctl01: Fix getting key serial number

[Guangwen Feng]

We expect to get the key serial number by matching "debug:fred":

Session Keyring
1072563454 --alswrv      0     0  keyring: _ses
 340758544 --alswrv      0 65534   \_ keyring: _uid.0
 783746283 --alswrv      0     0   \_ user: debug:fred

But in some kernels the key will be inaccessible immediately
after being requested with "debug:fred negate":

Session Keyring
        -3 --alswrv      0     0  keyring: _ses
 842289032 --alswrv      0    -1   \_ keyring: _uid.0
877125164: key inaccessible (Permission denied)

So fix this by adding a match with "inaccessible".

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/commands/keyctl/keyctl01.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
index 238b117..76226ae 100644
--- a/testcases/commands/keyctl/keyctl01.sh
+++ b/testcases/commands/keyctl/keyctl01.sh
@@ -76,6 +76,11 @@ do_test()
 		fi
 
 		local key=`keyctl show | awk '/debug:fred/ {print $1}'`
+		if [ -z "$key" ]; then
+			key=`keyctl show | \
+				awk -F ':' '/inaccessible/ {print $1}'`
+		fi
+
 		if [ -n "$key" ]; then
 			keyctl unlink $key @s >/dev/null
 			tst_sleep 50ms
-- 
2.9.4

[LTP] [PATCH v2 3/3] commands/keyctl01: Check keyctl support instead of kernel version

[Guangwen Feng]

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/commands/keyctl/keyctl01.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
index 76226ae..8e96622 100644
--- a/testcases/commands/keyctl/keyctl01.sh
+++ b/testcases/commands/keyctl/keyctl01.sh
@@ -36,8 +36,10 @@ TST_NEEDS_CMDS="keyctl"
 
 setup()
 {
-	if tst_kvcmp -le 2.6.33; then
-		tst_brk TCONF "Kernel newer than 2.6.33 is needed"
+	keyctl 2>&1 | tr '\n' ' ' | grep "request2" | grep "negate" | \
+		grep "show" | grep "unlink" >/dev/null
+	if [ $? -ne 0 ]; then
+		tst_brk TCONF "keyctl operations not supported"
 	fi
 
 	PATH_KEYSTAT="/proc/key-users"
-- 
2.9.4

[LTP] [PATCH v2 3/3] commands/keyctl01: Check keyctl support instead of kernel version

[Cyril Hrubis]

Hi!
I've pushed the first two patches, thanks.

>  setup()
>  {
> -	if tst_kvcmp -le 2.6.33; then
> -		tst_brk TCONF "Kernel newer than 2.6.33 is needed"
> +	keyctl 2>&1 | tr '\n' ' ' | grep "request2" | grep "negate" | \
> +		grep "show" | grep "unlink" >/dev/null

Uh, what are we grepping here for exactly? Can't we do that with a
single regular expression?

> +	if [ $? -ne 0 ]; then
> +		tst_brk TCONF "keyctl operations not supported"
>  	fi
>  
>  	PATH_KEYSTAT="/proc/key-users"
> -- 
> 2.9.4
> 
> 
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 3/3] commands/keyctl01: Check keyctl support instead of kernel version

[Guangwen Feng]

Hi!

在 07/18/2017 09:27 PM, Cyril Hrubis 写道:
> Hi!
> I've pushed the first two patches, thanks.

Thanks!

> 
>>  setup()
>>  {
>> -	if tst_kvcmp -le 2.6.33; then
>> -		tst_brk TCONF "Kernel newer than 2.6.33 is needed"
>> +	keyctl 2>&1 | tr '\n' ' ' | grep "request2" | grep "negate" | \
>> +		grep "show" | grep "unlink" >/dev/null
> 
> Uh, what are we grepping here for exactly?

We need to check the keyctl operations "request2", "negate",
"show", "unlink" are all available...

> Can't we do that with a single regular expression?

Yes, we can do:

keyctl 2>&1 | sort | tr '\n' ' ' | grep -P "\bnegate\b.*\brequest2\b.*\bshow\b.*\bunlink\b" >/dev/null


Best Regards,
Guangwen Feng

> 
>> +	if [ $? -ne 0 ]; then
>> +		tst_brk TCONF "keyctl operations not supported"
>>  	fi
>>  
>>  	PATH_KEYSTAT="/proc/key-users"
>> -- 
>> 2.9.4
>>
>>
>>
> 

[LTP] [PATCH v3] commands/keyctl01: Check keyctl support instead of kernel version

[Guangwen Feng]

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/commands/keyctl/keyctl01.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
index 76226ae..0b8e86c 100644
--- a/testcases/commands/keyctl/keyctl01.sh
+++ b/testcases/commands/keyctl/keyctl01.sh
@@ -36,8 +36,10 @@ TST_NEEDS_CMDS="keyctl"
 
 setup()
 {
-	if tst_kvcmp -le 2.6.33; then
-		tst_brk TCONF "Kernel newer than 2.6.33 is needed"
+	keyctl 2>&1 | sort | tr '\n' ' ' | grep -P \
+		"\bnegate\b.*\brequest2\b.*\bshow\b.*\bunlink\b" >/dev/null
+	if [ $? -ne 0 ]; then
+		tst_brk TCONF "keyctl operations not supported"
 	fi
 
 	PATH_KEYSTAT="/proc/key-users"
-- 
2.9.4

[LTP] [PATCH v3] commands/keyctl01: Check keyctl support instead of kernel version

[Cyril Hrubis]

Hi!
Looking at this closer we only check that the keyctl binary understands
these requests, shouldn't we as well try to request2 and unlink a key to
seek if kernel is up to the task?

And btw, it would be a bit cleaner to use a for loop to check for each
flag separately and tell user what flag is not supported:

diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
old mode 100644
new mode 100755
index 76226ae4d..f831acbfc
--- a/testcases/commands/keyctl/keyctl01.sh
+++ b/testcases/commands/keyctl/keyctl01.sh
@@ -34,11 +34,18 @@ TST_NEEDS_TMPDIR=1
 TST_NEEDS_CMDS="keyctl"
 . tst_test.sh
 
+check_keyctl()
+{
+       for op in $@; do
+               if ! keyctl 2>&1 | grep -q "keyctl $op"; then
+                       tst_brk TCONF "keyctl operation $op not supported"
+               fi
+       done
+}
+
 setup()
 {
-       if tst_kvcmp -le 2.6.33; then
-               tst_brk TCONF "Kernel newer than 2.6.33 is needed"
-       fi
+       check_keyctl negate request2 show unlink
 
        PATH_KEYSTAT="/proc/key-users"
        PATH_KEYQUOTA="/proc/sys/kernel/keys/root_maxbytes"


-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4] commands/keyctl01: Check keyctl support instead of kernel version

[Guangwen Feng]

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/commands/keyctl/keyctl01.sh | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
index 76226ae..650e7f4 100644
--- a/testcases/commands/keyctl/keyctl01.sh
+++ b/testcases/commands/keyctl/keyctl01.sh
@@ -34,11 +34,38 @@ TST_NEEDS_TMPDIR=1
 TST_NEEDS_CMDS="keyctl"
 . tst_test.sh
 
+check_keyctl()
+{
+	local nosup
+	for op in $@; do
+		nosup=0
+
+		if ! keyctl 2>&1 | grep -q "keyctl $op"; then
+			nosup=1
+		fi
+
+		if [ "$op" = "request2" ]; then
+			local key=`keyctl request2 user debug:foo bar`
+			if [ $? -ne 0 ]; then
+				nosup=1
+			fi
+		fi
+
+		if [ "$op" = "unlink" ]; then
+			if ! keyctl unlink $key @s; then
+				nosup=1
+			fi
+		fi
+
+		if [ $nosup -ne 0 ]; then
+			tst_brk TCONF "keyctl operation $op not supported"
+		fi
+	done
+}
+
 setup()
 {
-	if tst_kvcmp -le 2.6.33; then
-		tst_brk TCONF "Kernel newer than 2.6.33 is needed"
-	fi
+	check_keyctl negate request2 show unlink
 
 	PATH_KEYSTAT="/proc/key-users"
 	PATH_KEYQUOTA="/proc/sys/kernel/keys/root_maxbytes"
-- 
2.9.4

[LTP] [PATCH v4] commands/keyctl01: Check keyctl support instead of kernel version

[Cyril Hrubis]

Hi!
Applied, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/3] commands/keyctl01: Enable this test for RHEL6

[Guangwen Feng]

This bug also happened on RHEL6 and was introduced in 2.6.32-254.el6,
so add RHEL6's kernel as target as well.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/commands/keyctl/keyctl01.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
index 8ea2b25..40183b0 100644
--- a/testcases/commands/keyctl/keyctl01.sh
+++ b/testcases/commands/keyctl/keyctl01.sh
@@ -37,8 +37,9 @@ TST_NEEDS_CMDS="keyctl"
 
 setup()
 {
-	if tst_kvcmp -le 2.6.33; then
-		tst_brk TCONF "Kernel newer than 2.6.33 is needed"
+	if tst_kvcmp -le '2.6.33 RHEL6:2.6.32-254'; then
+		tst_brk TCONF \
+			"Kernel newer than 2.6.33 or 2.6.32-254.el6 is needed"
 	fi
 
 	PATH_KEYSTAT="/proc/key-users"
-- 
1.8.4.2

[LTP] [PATCH 3/3] commands/keyctl01: Enable this test for RHEL6

[Cyril Hrubis]

Hi!
> diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
> index 8ea2b25..40183b0 100644
> --- a/testcases/commands/keyctl/keyctl01.sh
> +++ b/testcases/commands/keyctl/keyctl01.sh
> @@ -37,8 +37,9 @@ TST_NEEDS_CMDS="keyctl"
>  
>  setup()
>  {
> -	if tst_kvcmp -le 2.6.33; then
> -		tst_brk TCONF "Kernel newer than 2.6.33 is needed"
> +	if tst_kvcmp -le '2.6.33 RHEL6:2.6.32-254'; then
> +		tst_brk TCONF \
> +			"Kernel newer than 2.6.33 or 2.6.32-254.el6 is needed"

I would be a bit happier if we could check if keyctl operations we need
for the test are supported (if that is reasonably easy to do) insetad of
listing minimal kernel versions per distribution.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/3] commands/keyctl01: Enable this test for RHEL6

[Guangwen Feng]

Hi!

Thanks for your review.

在 07/07/2017 09:13 PM, Cyril Hrubis 写道:
> Hi!
>> diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
>> index 8ea2b25..40183b0 100644
>> --- a/testcases/commands/keyctl/keyctl01.sh
>> +++ b/testcases/commands/keyctl/keyctl01.sh
>> @@ -37,8 +37,9 @@ TST_NEEDS_CMDS="keyctl"
>>  
>>  setup()
>>  {
>> -	if tst_kvcmp -le 2.6.33; then
>> -		tst_brk TCONF "Kernel newer than 2.6.33 is needed"
>> +	if tst_kvcmp -le '2.6.33 RHEL6:2.6.32-254'; then
>> +		tst_brk TCONF \
>> +			"Kernel newer than 2.6.33 or 2.6.32-254.el6 is needed"
> 
> I would be a bit happier if we could check if keyctl operations we need
> for the test are supported (if that is reasonably easy to do) insetad of
> listing minimal kernel versions per distribution.

OK, I see.
I will check if they are supported and drop the tst_kvcmp, thanks.

Best Regards,
Guangwen Feng

[LTP] [PATCH 1/3] commands/keyctl01: Fix potential infinite loop

[Guangwen Feng]

Hi!

Ping, thanks!


Best Regards,
Guangwen Feng

On 05/11/2017 02:20 PM, Guangwen Feng wrote:
> End the loop when max key quota is less than or equal to current
> key usage, in case some errors happen and result in endless loop.
> 
> We expect the test to trigger key quota excess which breaks the
> loop but it may not happen, so add a TWARN message for this.
> 
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> ---
>  testcases/commands/keyctl/keyctl01.sh | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
> index 076a130..5a97499 100644
> --- a/testcases/commands/keyctl/keyctl01.sh
> +++ b/testcases/commands/keyctl/keyctl01.sh
> @@ -62,15 +62,17 @@ cleanup()
>  
>  do_test()
>  {
> +	local quota_excd=0
>  	local maxkeysz=$((ORIG_KEYSZ + 100))
>  
> -	while true
> +	while [ $maxkeysz -gt $ORIG_KEYSZ ]
>  	do
>  		echo $maxkeysz >$PATH_KEYQUOTA
>  
>  		keyctl request2 user debug:fred negate @t >temp 2>&1
>  		grep -q -E "quota exceeded" temp
>  		if [ $? -eq 0 ]; then
> +			quota_excd=1
>  			break
>  		fi
>  
> @@ -83,6 +85,10 @@ do_test()
>  		((maxkeysz -= 4))
>  	done
>  
> +	if [ $quota_excd -eq 0 ]; then
> +		tst_res TWARN "Failed to trigger the quota excess"
> +	fi
> +
>  	tst_res TPASS "Bug not reproduced"
>  }
>  
> 

[LTP] [PATCH 1/3] commands/keyctl01: Fix potential infinite loop

[Cyril Hrubis]

Hi!
> diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
> index 076a130..5a97499 100644
> --- a/testcases/commands/keyctl/keyctl01.sh
> +++ b/testcases/commands/keyctl/keyctl01.sh
> @@ -62,15 +62,17 @@ cleanup()
>  
>  do_test()
>  {
> +	local quota_excd=0
>  	local maxkeysz=$((ORIG_KEYSZ + 100))
>  
> -	while true
> +	while [ $maxkeysz -gt $ORIG_KEYSZ ]

This may still be incorrect if some keys has been deleted after we
executed the setup() function though. This is quite unlikely but still
may happen. What about we ended the loop once $maxkeysz < 0?

Otherwise it looks fine.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/3] commands/keyctl01: Fix potential infinite loop

[Guangwen Feng]

Hi!

Thanks for your review.

在 07/07/2017 08:55 PM, Cyril Hrubis 写道:
> Hi!
>> diff --git a/testcases/commands/keyctl/keyctl01.sh b/testcases/commands/keyctl/keyctl01.sh
>> index 076a130..5a97499 100644
>> --- a/testcases/commands/keyctl/keyctl01.sh
>> +++ b/testcases/commands/keyctl/keyctl01.sh
>> @@ -62,15 +62,17 @@ cleanup()
>>  
>>  do_test()
>>  {
>> +	local quota_excd=0
>>  	local maxkeysz=$((ORIG_KEYSZ + 100))
>>  
>> -	while true
>> +	while [ $maxkeysz -gt $ORIG_KEYSZ ]
> 
> This may still be incorrect if some keys has been deleted after we
> executed the setup() function though. This is quite unlikely but still
> may happen. What about we ended the loop once $maxkeysz < 0?

Yes, it sounds more reasonable,
I got it, thanks.

Best Regards,
Guangwen Feng

> 
> Otherwise it looks fine.
>