[LTP] [PATCH] syscalls/statvfs01: Correcly zero terminate the strings

[LTP] [PATCH] syscalls/statvfs01: Correcly zero terminate the strings

[Cyril Hrubis]

Fixes random failures caused by the fact that the stack is not
guaranteed to be zeroed.

Fixes: e305ac4a305f ("statvfs01: Convert to new LTP API")
Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
CC: Petr Vorel <pvorel@suse.cz>
CC: Richard Palethorpe <rpalethorpe@suse.com>
CC: Avinesh Kumar <akumar@suse.de>
---
 testcases/kernel/syscalls/statvfs/statvfs01.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/testcases/kernel/syscalls/statvfs/statvfs01.c b/testcases/kernel/syscalls/statvfs/statvfs01.c
index dd14d6a0e..f17dc4cfa 100644
--- a/testcases/kernel/syscalls/statvfs/statvfs01.c
+++ b/testcases/kernel/syscalls/statvfs/statvfs01.c
@@ -38,6 +38,9 @@ static void run(void)
 	memset(valid_fname, 'a', valid_len);
 	memset(toolong_fname, 'b', valid_len + 1);
 
+	valid_fname[valid_len] = 0;
+	toolong_fname[valid_len+1] = 0;
+
 	TST_EXP_FD(creat(valid_fname, 0444));
 	SAFE_CLOSE(TST_RET);
 
-- 
2.38.2


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

Re: [LTP] [PATCH] syscalls/statvfs01: Correcly zero terminate the strings

[Petr Vorel]

Hi Cyril,

I'm sorry not catching these, thanks for fixing.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

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

Re: [LTP] [PATCH] syscalls/statvfs01: Correcly zero terminate the strings

[Martin Doucha]

Hi,

On 18. 01. 23 14:36, Cyril Hrubis wrote:
> Fixes random failures caused by the fact that the stack is not
> guaranteed to be zeroed.
> 
> Fixes: e305ac4a305f ("statvfs01: Convert to new LTP API")
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> CC: Petr Vorel <pvorel@suse.cz>
> CC: Richard Palethorpe <rpalethorpe@suse.com>
> CC: Avinesh Kumar <akumar@suse.de>
> ---
>   testcases/kernel/syscalls/statvfs/statvfs01.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/testcases/kernel/syscalls/statvfs/statvfs01.c b/testcases/kernel/syscalls/statvfs/statvfs01.c
> index dd14d6a0e..f17dc4cfa 100644
> --- a/testcases/kernel/syscalls/statvfs/statvfs01.c
> +++ b/testcases/kernel/syscalls/statvfs/statvfs01.c
> @@ -38,6 +38,9 @@ static void run(void)
>   	memset(valid_fname, 'a', valid_len);
>   	memset(toolong_fname, 'b', valid_len + 1);
>   
> +	valid_fname[valid_len] = 0;
> +	toolong_fname[valid_len+1] = 0;

Is there a possibility that valid_len could be equal to PATH_MAX-1? I 
think toolong_fname[] size should be bumped to at least PATH_MAX+1.

> +
>   	TST_EXP_FD(creat(valid_fname, 0444));
>   	SAFE_CLOSE(TST_RET);
>   

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


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

Re: [LTP] [PATCH] syscalls/statvfs01: Correcly zero terminate the strings

[Petr Vorel]

> Hi,

> On 18. 01. 23 14:36, Cyril Hrubis wrote:
> > Fixes random failures caused by the fact that the stack is not
> > guaranteed to be zeroed.

> > Fixes: e305ac4a305f ("statvfs01: Convert to new LTP API")
> > Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> > CC: Petr Vorel <pvorel@suse.cz>
> > CC: Richard Palethorpe <rpalethorpe@suse.com>
> > CC: Avinesh Kumar <akumar@suse.de>
> > ---
> >   testcases/kernel/syscalls/statvfs/statvfs01.c | 3 +++
> >   1 file changed, 3 insertions(+)

> > diff --git a/testcases/kernel/syscalls/statvfs/statvfs01.c b/testcases/kernel/syscalls/statvfs/statvfs01.c
> > index dd14d6a0e..f17dc4cfa 100644
> > --- a/testcases/kernel/syscalls/statvfs/statvfs01.c
> > +++ b/testcases/kernel/syscalls/statvfs/statvfs01.c
> > @@ -38,6 +38,9 @@ static void run(void)
> >   	memset(valid_fname, 'a', valid_len);
> >   	memset(toolong_fname, 'b', valid_len + 1);
> > +	valid_fname[valid_len] = 0;
> > +	toolong_fname[valid_len+1] = 0;

> Is there a possibility that valid_len could be equal to PATH_MAX-1? I think

PATH_MAX is 4096. statvfs.f_namemax is mostly 255 (only for vfat and exfat is
1530, but that's for multibyte names, the "real" length, when counting
characters is also 255).

> toolong_fname[] size should be bumped to at least PATH_MAX+1.
statvfs01.c:43:22: warning: array subscript 4097 is above array bounds of ‘char[4096]’ [-Warray-bounds]
=> PATH_MAX-1 is the maximum (array size is PATH_MAX).

Kind regards,
Petr

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

Re: [LTP] [PATCH] syscalls/statvfs01: Correcly zero terminate the strings

[Cyril Hrubis]

Hi!
> >   	memset(valid_fname, 'a', valid_len);
> >   	memset(toolong_fname, 'b', valid_len + 1);
> >   
> > +	valid_fname[valid_len] = 0;
> > +	toolong_fname[valid_len+1] = 0;
> 
> Is there a possibility that valid_len could be equal to PATH_MAX-1?

I do not think so, POSIX explicitly says that PATH_MAX should include
space for terminating null character.

Btw there is NAME_MAX constant that is defined to 256 which is the
usuall maximal lenght + 1.

> I  think toolong_fname[] size should be bumped to at least PATH_MAX+1.

Well we can do that (in a separate patch) to be extra sure, but even
then the PATH_MAX seems to be at least order of magnitude larger than
NAME_MAX.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH] syscalls/statvfs01: Correcly zero terminate the strings

[Martin Doucha]

On 19. 01. 23 15:41, Cyril Hrubis wrote:
> Hi!
>>>    	memset(valid_fname, 'a', valid_len);
>>>    	memset(toolong_fname, 'b', valid_len + 1);
>>>    
>>> +	valid_fname[valid_len] = 0;
>>> +	toolong_fname[valid_len+1] = 0;
>>
>> Is there a possibility that valid_len could be equal to PATH_MAX-1?
> 
> I do not think so, POSIX explicitly says that PATH_MAX should include
> space for terminating null character.

That's still true if valid_len == PATH_MAX-1. But we write valid_len+2 
bytes into toolong_fname.

But if always f_namemax <<< PATH_MAX, then we don't need to do anything.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


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