[PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Yang Xu]

On my test machine, ext4/033 fails even use the non-overflow size.
It reports invalid new size when using strtoull because errno is 1.

As man-pages said "Since  strtoul()  can legitimately return 0 or ULONG_MAX
(ULLONG_MAX for strtoull()) on both success and failure, the calling program
should set errno to 0 before the call, and then determine if an error occurred
by checking whether errno has a nonzero value after the call".

So add a step to set errno to 0 before strtoull call.

Fixes: 92b9c0dedace ("ext4/033: test EXT4_IOC_RESIZE_FS by calling the ioctl directly")
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 src/ext4_resize.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/ext4_resize.c b/src/ext4_resize.c
index 1ac51e6f..39e16529 100644
--- a/src/ext4_resize.c
+++ b/src/ext4_resize.c
@@ -35,6 +35,7 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	errno = 0;
 	new_size = strtoull(argv[2], &tmp, 10);
 	if ((errno) || (*tmp != '\0')) {
 		fprintf(stderr, "%s: invalid new size\n", argv[0]);
-- 
2.23.0

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Darrick J. Wong]

On Mon, Jan 17, 2022 at 03:36:54PM +0800, Yang Xu wrote:
> On my test machine, ext4/033 fails even use the non-overflow size.
> It reports invalid new size when using strtoull because errno is 1.
> 
> As man-pages said "Since  strtoul()  can legitimately return 0 or ULONG_MAX
> (ULLONG_MAX for strtoull()) on both success and failure, the calling program
> should set errno to 0 before the call, and then determine if an error occurred
> by checking whether errno has a nonzero value after the call".
> 
> So add a step to set errno to 0 before strtoull call.
> 
> Fixes: 92b9c0dedace ("ext4/033: test EXT4_IOC_RESIZE_FS by calling the ioctl directly")
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>

Looks good,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  src/ext4_resize.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/ext4_resize.c b/src/ext4_resize.c
> index 1ac51e6f..39e16529 100644
> --- a/src/ext4_resize.c
> +++ b/src/ext4_resize.c
> @@ -35,6 +35,7 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
>  
> +	errno = 0;
>  	new_size = strtoull(argv[2], &tmp, 10);
>  	if ((errno) || (*tmp != '\0')) {
>  		fprintf(stderr, "%s: invalid new size\n", argv[0]);
> -- 
> 2.23.0
> 

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Theodore Ts'o]

On Mon, Jan 17, 2022 at 03:36:54PM +0800, Yang Xu wrote:
> On my test machine, ext4/033 fails even use the non-overflow size.
> It reports invalid new size when using strtoull because errno is 1.
> 
> As man-pages said "Since  strtoul()  can legitimately return 0 or ULONG_MAX
> (ULLONG_MAX for strtoull()) on both success and failure, the calling program
> should set errno to 0 before the call, and then determine if an error occurred
> by checking whether errno has a nonzero value after the call".
> 
> So add a step to set errno to 0 before strtoull call.
> 
> Fixes: 92b9c0dedace ("ext4/033: test EXT4_IOC_RESIZE_FS by calling the ioctl directly")
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>

Reviewed-by: Theodore Ts'o <tytso@mit.edu>

You're right of course, but out of curiosity, which C library are you
using?

						- Ted

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[xuyang2018.jy]

on 2022/1/18 10:32, Theodore Ts'o wrote:
> On Mon, Jan 17, 2022 at 03:36:54PM +0800, Yang Xu wrote:
>> On my test machine, ext4/033 fails even use the non-overflow size.
>> It reports invalid new size when using strtoull because errno is 1.
>>
>> As man-pages said "Since  strtoul()  can legitimately return 0 or ULONG_MAX
>> (ULLONG_MAX for strtoull()) on both success and failure, the calling program
>> should set errno to 0 before the call, and then determine if an error occurred
>> by checking whether errno has a nonzero value after the call".
>>
>> So add a step to set errno to 0 before strtoull call.
>>
>> Fixes: 92b9c0dedace ("ext4/033: test EXT4_IOC_RESIZE_FS by calling the ioctl directly")
>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>
> Reviewed-by: Theodore Ts'o<tytso@mit.edu>
>
> You're right of course, but out of curiosity, which C library are you
> using?
I use glibc-2.34.

Best Regards
Yang Xu
>
> 						- Ted

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Theodore Ts'o]

On Tue, Jan 18, 2022 at 02:43:26AM +0000, xuyang2018.jy@fujitsu.com wrote:
> > You're right of course, but out of curiosity, which C library are you
> > using?
> I use glibc-2.34.

Hmm, ok.  I'm using glibc 2.31, and in this particular program, errno
shouldn't have been set by any prior system call.  I'm guessing maybe
it was something in crt0 which ended up setting errno?

						- Ted

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[xuyang2018.jy]

on 2022/1/18 11:56, Theodore Ts'o wrote:
> On Tue, Jan 18, 2022 at 02:43:26AM +0000, xuyang2018.jy@fujitsu.com wrote:
>>> You're right of course, but out of curiosity, which C library are you
>>> using?
>> I use glibc-2.34.
>
> Hmm, ok.  I'm using glibc 2.31, and in this particular program, errno
> shouldn't have been set by any prior system call.  I'm guessing maybe
> it was something in crt0 which ended up setting errno?
It maybe a glibc bug.
I cc glibc mailing list and see whether they have met this problem.

@Florian

Now, I use glibc-2.34 and run the following program[1] but the errno is 
not 0 in the beginning. So is this a known bug on glibc-2.34(Theodore 
doesn't meet this problem on glicb-2.31)?

[1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/ext4_resize.c

Best Regards
Yang Xu
>
> 						- Ted

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Adhemerval Zanella]

  

On 18/01/2022 02:27, xuyang2018.jy--- via Libc-alpha wrote:
> on 2022/1/18 11:56, Theodore Ts'o wrote:
>> On Tue, Jan 18, 2022 at 02:43:26AM +0000, xuyang2018.jy@fujitsu.com wrote:
>>>> You're right of course, but out of curiosity, which C library are you
>>>> using?
>>> I use glibc-2.34.
>>
>> Hmm, ok.  I'm using glibc 2.31, and in this particular program, errno
>> shouldn't have been set by any prior system call.  I'm guessing maybe
>> it was something in crt0 which ended up setting errno?
> It maybe a glibc bug.
> I cc glibc mailing list and see whether they have met this problem.
> 
> @Florian
> 
> Now, I use glibc-2.34 and run the following program[1] but the errno is 
> not 0 in the beginning. So is this a known bug on glibc-2.34(Theodore 
> doesn't meet this problem on glicb-2.31)?
> 
> [1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/ext4_resize.c

The errno should be only set on a failure, no function shall set errno
to 0 (it is a POSIX definition which glibc adheres).  The application
need to explicitly set errno to 0 before the function call to check if 
an error occurs.

So you need to do:

  errno = 0
  new_size = strtoull(argv[2], &tmp, 10);
  if ((errno) || (*tmp != '\0')) {
    ...
  }

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Florian Weimer]

* Adhemerval Zanella:

>   
>
> On 18/01/2022 02:27, xuyang2018.jy--- via Libc-alpha wrote:
>> on 2022/1/18 11:56, Theodore Ts'o wrote:
>>> On Tue, Jan 18, 2022 at 02:43:26AM +0000, xuyang2018.jy@fujitsu.com wrote:
>>>>> You're right of course, but out of curiosity, which C library are you
>>>>> using?
>>>> I use glibc-2.34.
>>>
>>> Hmm, ok.  I'm using glibc 2.31, and in this particular program, errno
>>> shouldn't have been set by any prior system call.  I'm guessing maybe
>>> it was something in crt0 which ended up setting errno?
>> It maybe a glibc bug.
>> I cc glibc mailing list and see whether they have met this problem.
>> 
>> @Florian
>> 
>> Now, I use glibc-2.34 and run the following program[1] but the errno is 
>> not 0 in the beginning. So is this a known bug on glibc-2.34(Theodore 
>> doesn't meet this problem on glicb-2.31)?
>> 
>> [1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/ext4_resize.c
>
> The errno should be only set on a failure, no function shall set errno
> to 0 (it is a POSIX definition which glibc adheres).  The application
> need to explicitly set errno to 0 before the function call to check if 
> an error occurs.

While this is true, I think errno should still be 0 at the start of the
program.

Thanks,
Florian

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Adhemerval Zanella]

On 18/01/2022 08:26, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>>   
>>
>> On 18/01/2022 02:27, xuyang2018.jy--- via Libc-alpha wrote:
>>> on 2022/1/18 11:56, Theodore Ts'o wrote:
>>>> On Tue, Jan 18, 2022 at 02:43:26AM +0000, xuyang2018.jy@fujitsu.com wrote:
>>>>>> You're right of course, but out of curiosity, which C library are you
>>>>>> using?
>>>>> I use glibc-2.34.
>>>>
>>>> Hmm, ok.  I'm using glibc 2.31, and in this particular program, errno
>>>> shouldn't have been set by any prior system call.  I'm guessing maybe
>>>> it was something in crt0 which ended up setting errno?
>>> It maybe a glibc bug.
>>> I cc glibc mailing list and see whether they have met this problem.
>>>
>>> @Florian
>>>
>>> Now, I use glibc-2.34 and run the following program[1] but the errno is 
>>> not 0 in the beginning. So is this a known bug on glibc-2.34(Theodore 
>>> doesn't meet this problem on glicb-2.31)?
>>>
>>> [1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/ext4_resize.c
>>
>> The errno should be only set on a failure, no function shall set errno
>> to 0 (it is a POSIX definition which glibc adheres).  The application
>> need to explicitly set errno to 0 before the function call to check if 
>> an error occurs.
> 
> While this is true, I think errno should still be 0 at the start of the
> program.

I think this is a implementation detail, I am not aware that either C or
POSIX now states it should initialized to any specific value (in fact, 
POSIX at Issue 5 [1] has removed the 'The value of errno is 0 at program 
start-up' on its description).

In any case, we set errno to be an uninitialized TLS variable.  Unless we
have a bug on .tbss initialization I think the issue is somewhere else.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/errno.html

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Florian Weimer]

* Adhemerval Zanella:

> On 18/01/2022 08:26, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>>   
>>>
>>> On 18/01/2022 02:27, xuyang2018.jy--- via Libc-alpha wrote:
>>>> on 2022/1/18 11:56, Theodore Ts'o wrote:
>>>>> On Tue, Jan 18, 2022 at 02:43:26AM +0000, xuyang2018.jy@fujitsu.com wrote:
>>>>>>> You're right of course, but out of curiosity, which C library are you
>>>>>>> using?
>>>>>> I use glibc-2.34.
>>>>>
>>>>> Hmm, ok.  I'm using glibc 2.31, and in this particular program, errno
>>>>> shouldn't have been set by any prior system call.  I'm guessing maybe
>>>>> it was something in crt0 which ended up setting errno?
>>>> It maybe a glibc bug.
>>>> I cc glibc mailing list and see whether they have met this problem.
>>>>
>>>> @Florian
>>>>
>>>> Now, I use glibc-2.34 and run the following program[1] but the errno is 
>>>> not 0 in the beginning. So is this a known bug on glibc-2.34(Theodore 
>>>> doesn't meet this problem on glicb-2.31)?
>>>>
>>>> [1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/ext4_resize.c
>>>
>>> The errno should be only set on a failure, no function shall set errno
>>> to 0 (it is a POSIX definition which glibc adheres).  The application
>>> need to explicitly set errno to 0 before the function call to check if 
>>> an error occurs.
>> 
>> While this is true, I think errno should still be 0 at the start of the
>> program.
>
> I think this is a implementation detail, I am not aware that either C or
> POSIX now states it should initialized to any specific value (in fact, 
> POSIX at Issue 5 [1] has removed the 'The value of errno is 0 at program 
> start-up' on its description).

It would be nice to stay compatible with that.

> In any case, we set errno to be an uninitialized TLS variable.  Unless we
> have a bug on .tbss initialization I think the issue is somewhere else.

I suspect it's some additional system call, which is why I requested
strace output.

Thanks,
Florian

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Andreas Schwab]

On Jan 18 2022, Adhemerval Zanella wrote:

> I think this is a implementation detail, I am not aware that either C or
> POSIX now states it should initialized to any specific value (in fact, 
> POSIX at Issue 5 [1] has removed the 'The value of errno is 0 at program 
> start-up' on its description).

It's still part of the C standard, though.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Adhemerval Zanella]

On 18/01/2022 09:04, Andreas Schwab wrote:
> On Jan 18 2022, Adhemerval Zanella wrote:
> 
>> I think this is a implementation detail, I am not aware that either C or
>> POSIX now states it should initialized to any specific value (in fact, 
>> POSIX at Issue 5 [1] has removed the 'The value of errno is 0 at program 
>> start-up' on its description).
> 
> It's still part of the C standard, though.
> 

And I think it is error-prone, since it requires caller to handle two 
different assumptions (whether the function is called after program 
initialization or after errno might be clobbered).

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Florian Weimer]

* xuyang2018.jy:

> on 2022/1/18 11:56, Theodore Ts'o wrote:
>> On Tue, Jan 18, 2022 at 02:43:26AM +0000, xuyang2018.jy@fujitsu.com wrote:
>>>> You're right of course, but out of curiosity, which C library are you
>>>> using?
>>> I use glibc-2.34.
>>
>> Hmm, ok.  I'm using glibc 2.31, and in this particular program, errno
>> shouldn't have been set by any prior system call.  I'm guessing maybe
>> it was something in crt0 which ended up setting errno?
> It maybe a glibc bug.
> I cc glibc mailing list and see whether they have met this problem.
>
> @Florian
>
> Now, I use glibc-2.34 and run the following program[1] but the errno is 
> not 0 in the beginning. So is this a known bug on glibc-2.34(Theodore 
> doesn't meet this problem on glicb-2.31)?
>
> [1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/ext4_resize.c

I'm not aware of this issue.  Could you run strace or strace -k to see
where the failing system call is coming from?  Thanks.

Florian

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Andreas Schwab]

On Jan 18 2022, xuyang2018.jy@fujitsu.com wrote:

> Now, I use glibc-2.34 and run the following program[1] but the errno is 
> not 0 in the beginning. So is this a known bug on glibc-2.34(Theodore 
> doesn't meet this problem on glicb-2.31)?
>
> [1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/ext4_resize.c

Note that there is a call to open preceding the strtoull call, so no
guarantees can be made about the value of errno before the latter.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[H.J. Lu]

On Tue, Jan 18, 2022 at 6:22 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Jan 18 2022, xuyang2018.jy@fujitsu.com wrote:
>
> > Now, I use glibc-2.34 and run the following program[1] but the errno is
> > not 0 in the beginning. So is this a known bug on glibc-2.34(Theodore
> > doesn't meet this problem on glicb-2.31)?
> >
> > [1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/ext4_resize.c
>
> Note that there is a call to open preceding the strtoull call, so no
> guarantees can be made about the value of errno before the latter.
>

strtoull man page:

RETURN VALUE
       The strtoul() function returns either the result of the conversion  or,
       if  there  was  a leading minus sign, the negation of the result of the
       conversion represented as an unsigned value, unless the original  (non‐
       negated)  value  would  overflow; in the latter case, strtoul() returns
       ULONG_MAX and sets errno to ERANGE.  Precisely the same holds for  str‐
       toull() (with ULLONG_MAX instead of ULONG_MAX).

new_size = strtoull(argv[2], &tmp, 10);
if ((errno) || (*tmp != '\0')) {
^^^^^^^^^^^^ Shouldn't errno be checked only after the prior function
return value is checked first?

fprintf(stderr, "%s: invalid new size\n", argv[0]);
return 1;
}

-- 
H.J.

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[xuyang2018.jy]

on 2022/1/18 22:22, Andreas Schwab wrote:
> On Jan 18 2022, xuyang2018.jy@fujitsu.com wrote:
> 
>> Now, I use glibc-2.34 and run the following program[1] but the errno is
>> not 0 in the beginning. So is this a known bug on glibc-2.34(Theodore
>> doesn't meet this problem on glicb-2.31)?
>>
>> [1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/ext4_resize.c
> 
> Note that there is a call to open preceding the strtoull call, so no
> guarantees can be made about the value of errno before the latter.
I just tried, the errno is 1 before open syscall.

Best Regards
Yang Xu
> 

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[xuyang2018.jy]

Hi Andrew

errno doesn't be initialized to 0 when c program link with lcap since
libcap-2.30 (commit f1f62a748d7c Refactor the way we do the psx linkage
in libcap introduced this bug.)

The c example code as below:
-------------------------------------------
#include <stdio.h>
#include <errno.h>

int main(int argc, char **argv)
{
        printf("errno %d\n", errno);
        return 0;
}
---------------------------------------

#gcc test.c -lcap -o test
#./test
errno 1

Best Regards
Yang Xu
> on 2022/1/18 22:02, Florian Weimer wrote:
>> * xuyang2018.jy:
>>
>>> on 2022/1/18 11:56, Theodore Ts'o wrote:
>>>> On Tue, Jan 18, 2022 at 02:43:26AM +0000, xuyang2018.jy@fujitsu.com wrote:
>>>>>> You're right of course, but out of curiosity, which C library are you
>>>>>> using?
>>>>> I use glibc-2.34.
>>>>
>>>> Hmm, ok.  I'm using glibc 2.31, and in this particular program, errno
>>>> shouldn't have been set by any prior system call.  I'm guessing maybe
>>>> it was something in crt0 which ended up setting errno?
>>> It maybe a glibc bug.
>>> I cc glibc mailing list and see whether they have met this problem.
>>>
>>> @Florian
>>>
>>> Now, I use glibc-2.34 and run the following program[1] but the errno is
>>> not 0 in the beginning. So is this a known bug on glibc-2.34(Theodore
>>> doesn't meet this problem on glicb-2.31)?
>>>
>>> [1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/ext4_resize.c
>>
>> I'm not aware of this issue.  Could you run strace or strace -k to see
>> where the failing system call is coming from?  Thanks.
> Even before open call, errno is also 1.
> 
> The strace output see attachment.
> 
> I found the following difference(compare with glibc-2.28, search 'err'
> keywords)
> 
> bad(glibc-2.34):
> 
> map(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0)
> = 0x7fdf815a3000
>   >  /usr/lib64/ld-linux-x86-64.so.2(__mmap+0x26) [0x21c56]
>   >  /usr/lib64/ld-linux-x86-64.so.2(rtld_malloc+0xa0) [0x1dc50]
>   >  /usr/lib64/ld-linux-x86-64.so.2(_dl_check_map_versions+0x4ec) [0x121fc]
>   >  /usr/lib64/ld-linux-x86-64.so.2(_dl_check_all_versions+0x3e) [0x124de]
>   >  /usr/lib64/ld-linux-x86-64.so.2(version_check_doit+0x1b) [0x1cab]
>   >  /usr/lib64/ld-linux-x86-64.so.2(_dl_receive_error+0x31) [0x1e811]
>   >  /usr/lib64/ld-linux-x86-64.so.2(dl_main+0x1e46) [0x4416]
>   >  /usr/lib64/ld-linux-x86-64.so.2(_dl_sysdep_start+0x3f6) [0x1d696]
>   >  /usr/lib64/ld-linux-x86-64.so.2(_dl_start+0x217) [0x2097]
>   >  /usr/lib64/ld-linux-x86-64.so.2(_start+0x7) [0x1097]
>   >  no matching address range
> arch_prctl(ARCH_SET_FS, 0x7fdf815a4480) = 0
> 
> good(glibc-2.28):
> 
> map(NULL, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0)
> = 0x7febd628b000
>   >  /usr/lib64/ld-2.28.so(__mmap+0x47) [0x1c377]
>   >  /usr/lib64/ld-2.28.so(malloc+0x8a) [0x1a94a]
>   >  /usr/lib64/ld-2.28.so(_dl_allocate_tls_storage+0x23) [0x12ac3]
>   >  /usr/lib64/ld-2.28.so(init_tls+0xa5) [0x1da5]
>   >  /usr/lib64/ld-2.28.so(dl_main+0x2c6d) [0x51ed]
>   >  /usr/lib64/ld-2.28.so(_dl_sysdep_start+0x48e) [0x1a14e]
>   >  /usr/lib64/ld-2.28.so(_dl_start+0x275) [0x2135]
>   >  /usr/lib64/ld-2.28.so(_start+0x7) [0x1087]
>   >  No DWARF information found
> arch_prctl(ARCH_SET_FS, 0x7febd628b740) = 0
> 
> Also if use gcc to compile this c program directly instead of using make
> in xfstests testsuite,  the errno value is normal(0).
> 
> After tried, I found if using the following command, errno is still 0
> gcc ext4_resize.c -g -O2  -D_GNU_SOURCE   -D_FILE_OFFSET_BITS=64
> -funsigned-char -fno-strict-aliasing  -Wall -o ext4_resize
> 
> But if I used link with lcap, then errno becomes 1 in the beginning.
> 
> Even I use lastest upstream libcap, this problem still occurs.
> 
> I am still looking into it .
> 
> Best Regards
> Yang Xu
> 
>>
>> Florian
>>
> 

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Andreas Schwab]

On Jan 19 2022, xuyang2018.jy@fujitsu.com wrote:

> I just tried, the errno is 1 before open syscall.

Worksforme.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Cristian Rodríguez]

On Wed, Jan 19, 2022 at 4:20 AM xuyang2018.jy--- via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Hi Andrew
>
> errno doesn't be initialized to 0 when c program link with lcap since
> libcap-2.30 (commit f1f62a748d7c Refactor the way we do the psx linkage
> in libcap introduced this bug.)
>
> The c example code as below:
> -------------------------------------------
> #include <stdio.h>
> #include <errno.h>
>
> int main(int argc, char **argv)
> {
>         printf("errno %d\n", errno);
>         return 0;
> }
> ---------------------------------------
>
> #gcc test.c -lcap -o test
> #./test
> errno 1
>

Yes, that reproduces the problem, quite well. (link with
-Wl,--no-as-needed if it does not trigger the bug for you)

This is not a glibc problem though, looks like lcap is clobbering
errno. I'd bet good CLP on the code called in
__attribute__((constructor (300))) static void _initialize_libcap(void) .
I strongly suggest not to use constructors on shared libraries unless
all the components using the library are in your control and you are
sure constructors will not ruin some other application's day.

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Cristian Rodríguez]

On Wed, Jan 19, 2022 at 10:57 AM Cristian Rodríguez
<crrodriguez@opensuse.org> wrote:

> This is not a glibc problem though, looks like lcap is clobbering
> errno. I'd bet good CLP on the code called in
> __attribute__((constructor (300))) static void _initialize_libcap(void) .
> I strongly suggest not to use constructors on shared libraries unless
> all the components using the library are in your control and you are
> sure constructors will not ruin some other application's day.

__attribute__((constructor (300))) static void _initialize_libcap(void)
{
    if (_cap_max_bits) {
        return;
    }
    cap_set_syscall(NULL, NULL);  --> nope
    _binary_search(_cap_max_bits, cap_get_bound, 0, __CAP_MAXBITS,
__CAP_BITS); --> 🤔
    cap_proc_root("/proc");
}

do, what cap_get_bound does ?

int cap_get_bound(cap_value_t cap)
{
    int result;

    result = prctl(PR_CAPBSET_READ, pr_arg(cap), pr_arg(0));
    if (result < 0) {
        errno = -result; --> If all my bets paid , I would be rich..
here is your 1
        return -1;
    }
    return result;
}

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Andrew G. Morgan]

Thanks for finding this.

Fixed in:

https://git.kernel.org/pub/scm/libs/libcap/libcap.git/commit/?id=f25a1b7e69f7b33e6afb58b3e38f3450b7d2d9a0

Cheers

Andrew

On Wed, Jan 19, 2022 at 6:08 AM Cristian Rodríguez
<crrodriguez@opensuse.org> wrote:
>
> On Wed, Jan 19, 2022 at 10:57 AM Cristian Rodríguez
> <crrodriguez@opensuse.org> wrote:
>
> > This is not a glibc problem though, looks like lcap is clobbering
> > errno. I'd bet good CLP on the code called in
> > __attribute__((constructor (300))) static void _initialize_libcap(void) .
> > I strongly suggest not to use constructors on shared libraries unless
> > all the components using the library are in your control and you are
> > sure constructors will not ruin some other application's day.
>
> __attribute__((constructor (300))) static void _initialize_libcap(void)
> {
>     if (_cap_max_bits) {
>         return;
>     }
>     cap_set_syscall(NULL, NULL);  --> nope
>     _binary_search(_cap_max_bits, cap_get_bound, 0, __CAP_MAXBITS,
> __CAP_BITS); --> 🤔
>     cap_proc_root("/proc");
> }
>
> do, what cap_get_bound does ?
>
> int cap_get_bound(cap_value_t cap)
> {
>     int result;
>
>     result = prctl(PR_CAPBSET_READ, pr_arg(cap), pr_arg(0));
>     if (result < 0) {
>         errno = -result; --> If all my bets paid , I would be rich..
> here is your 1
>         return -1;
>     }
>     return result;
> }

Re: [PATCH] src/ext4_resize.c: set errno to 0 before the strtoull call

[Theodore Ts'o]

On Wed, Jan 19, 2022 at 06:50:41AM -0800, Andrew G. Morgan wrote:
> Thanks for finding this.
> 
> Fixed in:
> 
> https://git.kernel.org/pub/scm/libs/libcap/libcap.git/commit/?id=f25a1b7e69f7b33e6afb58b3e38f3450b7d2d9a0

Thanks!

FWIW, I agree that we should not have depended on it being initialized
to zero at the beginning program.  It was just wierd since it wasn't,
and I was wondering if it was some exotic (musl, dietlibc) C library
that was triggering it --- and then was surprised when it turned out
to be glibc.

The Jon Postel principle of "Be conservaive in what you send, and
liberal in what you accept" is good one to follow, so while it's nice
to some userspace programs that might not be as well-behaved as to not
depend on the status of errno when main() is called, we should fix the
calling program in xfstests as well.

Cheers,

						- Ted