public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH] fix readv01 testcase for parisc architecture
@ 2013-11-19 21:46 Helge Deller
  2013-11-20  8:10 ` Jan Stancek
  0 siblings, 1 reply; 4+ messages in thread
From: Helge Deller @ 2013-11-19 21:46 UTC (permalink / raw)
  To: ltp-list

In the second test of readv01 the rd_iovec[] array was not defined or
zero-initialized for vectors which will be accessed by the readv() syscall.
This leads to the fact that on the parisc platform the readv syscall accessed
random memory which is located behind the array.  Fix this problem by adding
two more vectors with address NULL and length 0.

Furthermore, in the test2 itself we tell readv() to read 4 vecors, while
the error message behind it says that it has read CHUNK bytes "followed
by *two* NULL vectors", which then sums up to a total of 3 vectors
instead of 4.
Fix this by calling readv() with only 3 vectors instead of 4.

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/testcases/kernel/syscalls/readv/readv01.c b/testcases/kernel/syscalls/readv/readv01.c
index d92c1be..ca83310 100644
--- a/testcases/kernel/syscalls/readv/readv01.c
+++ b/testcases/kernel/syscalls/readv/readv01.c
@@ -66,7 +66,9 @@ struct iovec rd_iovec[MAX_IOVEC] = {
 	{(buf2 + CHUNK * 10), CHUNK},
 
 	/* Test case #2 */
-	{(buf2 + CHUNK * 11), CHUNK}
+	{(buf2 + CHUNK * 11), CHUNK},
+	{NULL, 0},
+	{NULL, 0}
 };
 
 char f_name[K_1];
@@ -112,7 +114,7 @@ int main(int ac, char **av)
 
 //test2:
 		l_seek(fd, CHUNK * 12, 0);
-		if (readv(fd, (rd_iovec + 1), 4) != CHUNK) {
+		if (readv(fd, (rd_iovec + 1), 3) != CHUNK) {
 			tst_resm(TFAIL, "readv failed reading %d bytes, "
 				 "followed by two NULL vectors", CHUNK);
 		} else {

------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] fix readv01 testcase for parisc architecture
  2013-11-19 21:46 [LTP] [PATCH] fix readv01 testcase for parisc architecture Helge Deller
@ 2013-11-20  8:10 ` Jan Stancek
  2013-11-20 19:54   ` Helge Deller
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Stancek @ 2013-11-20  8:10 UTC (permalink / raw)
  To: Helge Deller; +Cc: ltp-list





----- Original Message -----
> From: "Helge Deller" <deller@gmx.de>
> To: ltp-list@lists.sourceforge.net
> Sent: Tuesday, 19 November, 2013 10:46:12 PM
> Subject: [LTP] [PATCH] fix readv01 testcase for parisc architecture
> 
> In the second test of readv01 the rd_iovec[] array was not defined or
> zero-initialized for vectors which will be accessed by the readv() syscall.
> This leads to the fact that on the parisc platform the readv syscall accessed
> random memory which is located behind the array.  Fix this problem by adding
> two more vectors with address NULL and length 0.

Hi,

It shouldn't be behind, since rd_iovec array has size of MAX_IOVEC == 16.

It's partially initialized, and looking at C99 6.7.8.21, I'd expect the
rest to be zero-initialized as well:
"If there are fewer initializers in a brace-enclosed list than there are elements or members
of an aggregate, or fewer characters in a string literal used to initialize an array of known
size than there are elements in the array, the remainder of the aggregate shall be
initialized implicitly the same as objects that have static storage duration."

If you add 'static' to rd_iovec array, does the problem go away?
What compiler are you using?

> 
> Furthermore, in the test2 itself we tell readv() to read 4 vecors, while
> the error message behind it says that it has read CHUNK bytes "followed
> by *two* NULL vectors", which then sums up to a total of 3 vectors
> instead of 4.
> Fix this by calling readv() with only 3 vectors instead of 4.

Agreed.

Regards,
Jan

> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> diff --git a/testcases/kernel/syscalls/readv/readv01.c
> b/testcases/kernel/syscalls/readv/readv01.c
> index d92c1be..ca83310 100644
> --- a/testcases/kernel/syscalls/readv/readv01.c
> +++ b/testcases/kernel/syscalls/readv/readv01.c
> @@ -66,7 +66,9 @@ struct iovec rd_iovec[MAX_IOVEC] = {
>  	{(buf2 + CHUNK * 10), CHUNK},
>  
>  	/* Test case #2 */
> -	{(buf2 + CHUNK * 11), CHUNK}
> +	{(buf2 + CHUNK * 11), CHUNK},
> +	{NULL, 0},
> +	{NULL, 0}
>  };
>  
>  char f_name[K_1];
> @@ -112,7 +114,7 @@ int main(int ac, char **av)
>  
>  //test2:
>  		l_seek(fd, CHUNK * 12, 0);
> -		if (readv(fd, (rd_iovec + 1), 4) != CHUNK) {
> +		if (readv(fd, (rd_iovec + 1), 3) != CHUNK) {
>  			tst_resm(TFAIL, "readv failed reading %d bytes, "
>  				 "followed by two NULL vectors", CHUNK);
>  		} else {
> 
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
> 

------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] fix readv01 testcase for parisc architecture
  2013-11-20  8:10 ` Jan Stancek
@ 2013-11-20 19:54   ` Helge Deller
  2013-11-28 13:23     ` chrubis
  0 siblings, 1 reply; 4+ messages in thread
From: Helge Deller @ 2013-11-20 19:54 UTC (permalink / raw)
  To: Jan Stancek; +Cc: ltp-list

Hi Jan,

On 11/20/2013 09:10 AM, Jan Stancek wrote:
>> From: "Helge Deller" <deller@gmx.de>
>> Sent: Tuesday, 19 November, 2013 10:46:12 PM
>> Subject: [LTP] [PATCH] fix readv01 testcase for parisc architecture
>>
>> In the second test of readv01 the rd_iovec[] array was not defined or
>> zero-initialized for vectors which will be accessed by the readv() syscall.
>> This leads to the fact that on the parisc platform the readv syscall accessed
>> random memory which is located behind the array.  Fix this problem by adding
>> two more vectors with address NULL and length 0.
> 
> Hi,
> 
> It shouldn't be behind, since rd_iovec array has size of MAX_IOVEC == 16.

True!
I didn't saw that.
 
> It's partially initialized, and looking at C99 6.7.8.21, I'd expect the
> rest to be zero-initialized as well:
> "If there are fewer initializers in a brace-enclosed list than there are elements or members
> of an aggregate, or fewer characters in a string literal used to initialize an array of known
> size than there are elements in the array, the remainder of the aggregate shall be
> initialized implicitly the same as objects that have static storage duration."
> 
> If you add 'static' to rd_iovec array, does the problem go away?
> What compiler are you using?

Ok, it turned out that it wasn't the fault of the compiler.
It was me who was trying to fix the parisc kernel with all the errors I found
with this testcase and by accident had a buggy kernel running.

So, please ignore this part of my patch.
This specific code in readv01 is correct.

>> Furthermore, in the test2 itself we tell readv() to read 4 vecors, while
>> the error message behind it says that it has read CHUNK bytes "followed
>> by *two* NULL vectors", which then sums up to a total of 3 vectors
>> instead of 4.
>> Fix this by calling readv() with only 3 vectors instead of 4.
> 
> Agreed.

Ok, so if you could just apply this second hunk it would be great!

Thanks for maintaining LTP!
It helped me a lot to find bugs in the parisc arch code of the kernel.

Helge 

>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> diff --git a/testcases/kernel/syscalls/readv/readv01.c
>> b/testcases/kernel/syscalls/readv/readv01.c
>> index d92c1be..ca83310 100644
>> --- a/testcases/kernel/syscalls/readv/readv01.c
>> +++ b/testcases/kernel/syscalls/readv/readv01.c
>> @@ -66,7 +66,9 @@ struct iovec rd_iovec[MAX_IOVEC] = {
>>  	{(buf2 + CHUNK * 10), CHUNK},
>>  
>>  	/* Test case #2 */
>> -	{(buf2 + CHUNK * 11), CHUNK}
>> +	{(buf2 + CHUNK * 11), CHUNK},
>> +	{NULL, 0},
>> +	{NULL, 0}
>>  };
>>  
>>  char f_name[K_1];
>> @@ -112,7 +114,7 @@ int main(int ac, char **av)
>>  
>>  //test2:
>>  		l_seek(fd, CHUNK * 12, 0);
>> -		if (readv(fd, (rd_iovec + 1), 4) != CHUNK) {
>> +		if (readv(fd, (rd_iovec + 1), 3) != CHUNK) {
>>  			tst_resm(TFAIL, "readv failed reading %d bytes, "
>>  				 "followed by two NULL vectors", CHUNK);
>>  		} else {
>>


------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] fix readv01 testcase for parisc architecture
  2013-11-20 19:54   ` Helge Deller
@ 2013-11-28 13:23     ` chrubis
  0 siblings, 0 replies; 4+ messages in thread
From: chrubis @ 2013-11-28 13:23 UTC (permalink / raw)
  To: Helge Deller; +Cc: ltp-list

Hi!
> Ok, so if you could just apply this second hunk it would be great!

I've cleaned up the testcase and fixed the number of vectors as well.

> Thanks for maintaining LTP!
> It helped me a lot to find bugs in the parisc arch code of the kernel.

:)

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-11-28 13:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-19 21:46 [LTP] [PATCH] fix readv01 testcase for parisc architecture Helge Deller
2013-11-20  8:10 ` Jan Stancek
2013-11-20 19:54   ` Helge Deller
2013-11-28 13:23     ` chrubis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox