[LTP] [PATCH] syscalls/keyctl06: Print TFAIL if keyring_read() returns wrong size
Xiao Yang
yangx.jy@cn.fujitsu.com
Wed Oct 18 08:25:25 CEST 2017
Hi Eric,
On 2017/10/18 0:12, Eric Biggers wrote:
> Hi Xiao,
>
> On Tue, Oct 17, 2017 at 08:53:12PM +0800, Xiao Yang wrote:
>> According to keyctl06's message, the mentioned bug is introduced
>> by the following patch which is merged into kernel since v3.13:
>> 'b2a4df200d57 ("KEYS: Expand the capacity of a keyring")'
>>
>> However, we still got the following output before v3.13:
>> tst_test.c:958: INFO: Timeout per run is 0h 05m 00s
>> keyctl06.c:60: BROK: KEYCTL_READ returned 8 but expected 4
>>
>> In old kernels, the output exposed that keyring_read() could not
>> return the size of data read into buffer, because it just returned
>> the size of a keyring. So i think this issue should be targeted
>> as TFAIL.
>>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>> testcases/kernel/syscalls/keyctl/keyctl06.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/testcases/kernel/syscalls/keyctl/keyctl06.c b/testcases/kernel/syscalls/keyctl/keyctl06.c
>> index 8873431..bf30fb6 100644
>> --- a/testcases/kernel/syscalls/keyctl/keyctl06.c
>> +++ b/testcases/kernel/syscalls/keyctl/keyctl06.c
>> @@ -56,7 +56,7 @@ static void do_test(void)
>> tst_brk(TBROK, "KEYCTL_READ didn't return correct key ID");
>>
>> if (TEST_RETURN != sizeof(key_serial_t)) {
>> - tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu",
>> + tst_brk(TFAIL, "KEYCTL_READ returned %ld but expected %zu",
>> TEST_RETURN, sizeof(key_serial_t));
>> }
>>
> It was actually pointed out yesterday that the short return value is a bug in
> the kernel patch. The documented behavior of keyctl_read() (as well as the
> actual behavior for the other key types that implement it) is to return the full
> count on a short read, rather than a short count. It's not really intuitive but
> I'm going to have to fix it with another kernel patch.
Thanks for your explanation.
Sorry, i misunderstood the expected return value before.
> For now we probably should just make the test accept both return values:
>
> if (TEST_RETURN != sizeof(key_serial_t)&&
> TEST_RETURN != sizeof(key_ids)) {
> tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu or %zu",
> TEST_RETURN, sizeof(key_serial_t), sizeof(key_ids));
> }
>
> Then once there is another kernel patch, I'll update the test to reference that
> commit too, and accept only TEST_RETURN == sizeof(key_ids).
Could we update the test to check both return values? as below:
if (TEST_RETURN != sizeof(key_ids)) {
/* keyctl_read() should return the size of buffer required, rather than the size
* of data read into buffer. This bug was introduced by the commit:
* e645016abc80 ("KEYS: fix writing past end of user-supplied buffer in keyring_read()")
*/
if (TEST_RETURN == sizeof(key_serial_t)) {
tst_brk(TFAIL, "KEYCTL_READ returned %ld but expected %zu",
TEST_RETURN, sizeof(key_ids));
}
tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu",
TEST_RETURN, sizeof(key_ids));
}
We probably should expose the short return value as a bug, rather than ignore it.
> There is also the question of whether anything should be read at all when the
> buffer is too small. Currently the test assumes that a short read is done.
> Unfortunately, there is no simple answer to that question as the documentation
> for keyctl_read() and implementations are all inconsistent.
I also find the documentation and implementations are inconsistent, and either of
them may need to update.
Thanks,
Xiao yang
> Eric
>
>
>
More information about the ltp
mailing list