EINTR for fsync(2)

EINTR for fsync(2)

[Mathnerd314]

Hi,

The POSIX standard says fsync(2) can return EINTR:
https://pubs.opengroup.org/onlinepubs/9699919799/

The man page does not:
https://man7.org/linux/man-pages/man2/fsync.2.html

I think fsync can be interrupted by a signal on Linux, so this should
just be an oversight in the man page.

At least, fsync on fuse seems be able to return EINTR:
https://github.com/torvalds/linux/blob/5367cf1c3ad02f7f14d79733814302a96cc97b96/fs/fuse/dev.c#L114

Actually there seem to be numerous error codes that can be returned
from all filesystem calls on fuse: ENOTCONN, ENOMEM, etc. But EINTR is
at least documented in the POSIX standard, whereas these others seem
really rare. But for full correctness I suppose these should be
documented as well. It would be quite an undertaking.

-- Mathnerd314 (pseudonym)

Re: EINTR for fsync(2)

[Alejandro Colomar (man-pages)]

Hello Alexander,

On 1/31/22 19:32, Mathnerd314 wrote:
> Hi,
> 
> The POSIX standard says fsync(2) can return EINTR:
> https://pubs.opengroup.org/onlinepubs/9699919799/
> 
> The man page does not:
> https://man7.org/linux/man-pages/man2/fsync.2.html
> 
> I think fsync can be interrupted by a signal on Linux, so this should
> just be an oversight in the man page.
> 
> At least, fsync on fuse seems be able to return EINTR:
> https://github.com/torvalds/linux/blob/5367cf1c3ad02f7f14d79733814302a96cc97b96/fs/fuse/dev.c#L114
> 
> Actually there seem to be numerous error codes that can be returned
> from all filesystem calls on fuse: ENOTCONN, ENOMEM, etc. But EINTR is
> at least documented in the POSIX standard, whereas these others seem
> really rare. But for full correctness I suppose these should be
> documented as well. It would be quite an undertaking.
> 
> -- Mathnerd314 (pseudonym)

I got this report on linux-man@.  Could you please confirm if there are
any ERRORS that should be added to the fsync(2) manual page?

Thanks,

Alex

Mathnerd314:  Thanks for the report!  It's useful to CC the relevant
kernel developers when reporting non-trivial bugs such as this one.
They know better than we do.  :)

Cheers,

Alex

-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

Re: EINTR for fsync(2)

[Mathnerd314]

On Mon, Jan 31, 2022 at 1:44 PM Alejandro Colomar (man-pages)
<alx.manpages@gmail.com> wrote:
>
> Mathnerd314:  Thanks for the report!  It's useful to CC the relevant
> kernel developers when reporting non-trivial bugs such as this one.
> They know better than we do.  :)

Ah, but then I would have had to know that this is a non-trivial
question, as opposed to a 2-line patch. And CC'ing kernel developers
is not mentioned on
https://www.kernel.org/doc/man-pages/reporting_bugs.html.

Oh well, there's always next time.

-- Mathnerd314

Re: EINTR for fsync(2)

[Alejandro Colomar (man-pages)]

Hi Mathnerd314,

On 2/1/22 00:17, Mathnerd314 wrote:
> On Mon, Jan 31, 2022 at 1:44 PM Alejandro Colomar (man-pages)
> <alx.manpages@gmail.com> wrote:
>>
>> Mathnerd314:  Thanks for the report!  It's useful to CC the relevant
>> kernel developers when reporting non-trivial bugs such as this one.
>> They know better than we do.  :)
> 
> Ah, but then I would have had to know that this is a non-trivial
> question, as opposed to a 2-line patch.

It's very subjective.  But when it's a technical bug about very specific
knowledge, they know better than us.  When it's a typo or a formatting
issue in the manual page, it's trivial. ;)

> And CC'ing kernel developers
> is not mentioned on
> https://www.kernel.org/doc/man-pages/reporting_bugs.html.

Hmm, yes, I should fix that.  Thanks!

> 
> Oh well, there's always next time.

Yup, don't worry, I meant it for next time.  Anyway, it's easy for me to
add them to the CC if you don't or forget.

> 
> -- Mathnerd314

Cheers,

Alex

-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

Re: EINTR for fsync(2)

[Matthew Wilcox]

On Mon, Jan 31, 2022 at 09:44:38PM +0100, Alejandro Colomar (man-pages) wrote:
> Hello Alexander,
> 
> On 1/31/22 19:32, Mathnerd314 wrote:
> > Hi,
> > 
> > The POSIX standard says fsync(2) can return EINTR:
> > https://pubs.opengroup.org/onlinepubs/9699919799/
> > 
> > The man page does not:
> > https://man7.org/linux/man-pages/man2/fsync.2.html
> > 
> > I think fsync can be interrupted by a signal on Linux, so this should
> > just be an oversight in the man page.
> > 
> > At least, fsync on fuse seems be able to return EINTR:
> > https://github.com/torvalds/linux/blob/5367cf1c3ad02f7f14d79733814302a96cc97b96/fs/fuse/dev.c#L114
> > 
> > Actually there seem to be numerous error codes that can be returned
> > from all filesystem calls on fuse: ENOTCONN, ENOMEM, etc. But EINTR is
> > at least documented in the POSIX standard, whereas these others seem
> > really rare. But for full correctness I suppose these should be
> > documented as well. It would be quite an undertaking.

It's probably worth reading this part of POSIX:

: 2.3 Error Numbers
:
: Most functions can provide an error number. The means by which each
: function provides its error numbers is specified in its description.
: 
: Some functions provide the error number in a variable accessed through
: the symbol errno, defined by including the <errno.h> header. The value
: of errno should only be examined when it is indicated to be valid by
: a function's return value. No function in this volume of POSIX.1-2017
: shall set errno to zero. For each thread of a process, the value of
: errno shall not be affected by function calls or assignments to errno
: by other threads.
: 
: Some functions return an error number directly as the function
: value. These functions return a value of zero to indicate success.
: 
: If more than one error occurs in processing a function call, any one
: of the possible errors may be returned, as the order of detection is
: undefined.
: 
: Implementations may support additional errors not included in this list,
: may generate errors included in this list under circumstances other
: than those described here, or may contain extensions or limitations that
: prevent some errors from occurring.
: 
: The ERRORS section on each reference page specifies which error conditions
: shall be detected by all implementations (``shall fail") and which may
: be optionally detected by an implementation (``may fail"). If no error
: condition is detected, the action requested shall be successful. If an
: error condition is detected, the action requested may have been partially
: performed, unless otherwise stated.

So while it's worth adding EINTR to the man page, I don't think it's
worth going through an exercise of trying to add every possible
errno to every syscall.

Re: EINTR for fsync(2)

[Alejandro Colomar (man-pages)]

Hi Mathnerd314, Matthew,

On 2/1/22 01:30, Matthew Wilcox wrote:
[...]
> So while it's worth adding EINTR to the man page, I don't think it's
> worth going through an exercise of trying to add every possible
> errno to every syscall.
> 

Okay.  I documented this error.

Thanks,

Alex

---
    fsync.2: ERRORS: Document EINTR

    Reported-by: Mathnerd314 <mathnerd314.gph@gmail.com>
    Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
    Cc: <linux-fsdevel@vger.kernel.org>
    Cc: Matthew Wilcox <willy@infradead.org>
    Cc: Alexander Viro <viro@zeniv.linux.org.uk>


diff --git a/man2/fsync.2 b/man2/fsync.2
index 0f070ed2c..c79723ed8 100644
--- a/man2/fsync.2
+++ b/man2/fsync.2
@@ -126,6 +126,10 @@ is set to indicate the error.
 .I fd
 is not a valid open file descriptor.
 .TP
+.B EINTR
+The function was interrupted by a signal; see
+.BR signal (7).
+.TP
 .B EIO
 An error occurred during synchronization.
 This error may relate to data written to some other file descriptor


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

Re: EINTR for fsync(2)

[Mathnerd314]

On Mon, Jan 31, 2022 at 5:30 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> It's probably worth reading this part of POSIX:
>
> : Implementations may support additional errors not included in this list,
> : may generate errors included in this list under circumstances other
> : than those described here, or may contain extensions or limitations that
> : prevent some errors from occurring.
> :
> So while it's worth adding EINTR to the man page, I don't think it's
> worth going through an exercise of trying to add every possible
> errno to every syscall.
>
It's true that POSIX's error list is purely a guideline. But that
doesn't mean Linux can't specify the precise set of possible error
codes. There are currently 133 error codes defined, across an equally
large number of syscalls - coding defensively and handling every
combination is impossible. Not to mention that the meaning of the
error codes differs from syscall to syscall. But with precise
information there are likely only 5-10 codes per syscall, so handling
every error appropriately is feasible. So the information can be quite
useful.

I agree the lists would be too much work to create / maintain by hand.
What about a static analysis script that walks through the call tree
and collects the set of possible error codes for each syscall? Dan
Carpenter, is this something Smatch could do?

-- Mathnerd314

Re: EINTR for fsync(2)

[Matthew Wilcox]

On Tue, Feb 01, 2022 at 12:56:51PM -0700, Mathnerd314 wrote:
> On Mon, Jan 31, 2022 at 5:30 PM Matthew Wilcox <willy@infradead.org> wrote:
> > So while it's worth adding EINTR to the man page, I don't think it's
> > worth going through an exercise of trying to add every possible
> > errno to every syscall.
>
> It's true that POSIX's error list is purely a guideline. But that
> doesn't mean Linux can't specify the precise set of possible error
> codes. There are currently 133 error codes defined, across an equally
> large number of syscalls - coding defensively and handling every
> combination is impossible. Not to mention that the meaning of the
> error codes differs from syscall to syscall. But with precise
> information there are likely only 5-10 codes per syscall, so handling
> every error appropriately is feasible. So the information can be quite
> useful.

Ahahahaha.  For one, every filesystem gets to return its own errors.
So basically any syscall that takes an fd gets to return all the errors
that any of our 50+ filesystems decides to use.

Then there are the interposers.  Things like seccomp, Linux Security
Modules, and other such hooks which might return their own errors.

On top of that there's error injection, which might allow eBPF to
return any error that userspace has deemed appropriate from basically
any function.

Further, you generally don't execute syscalls.  You call glibc, which
might decide to return its own errors, not even bothering to call
the kernel.

But the real problem is that the kernel might decide to return new
errors at any time.  Just because you exhaustively handled every error
that could have occurred in 5.15 doesn't mean that there might not be
an error returned by 5.20.

This isn't even an Herculean task.  This is Sisyphean, and you'll
probably feel like Prometheus by the end of it.  I mean, you'd learn a
lot by attempting it, but you'd be better off picking a task that would
actually help people.

Fwd: EINTR for fsync(2)

[Mathnerd314]

Forgot to send this to the mailing list.

---------- Forwarded message ---------
From: Mathnerd314 <mathnerd314.gph@gmail.com>
Date: Thu, Feb 3, 2022 at 10:27 AM
Subject: Re: EINTR for fsync(2)
To: Matthew Wilcox <willy@infradead.org>


On Tue, Feb 1, 2022 at 1:32 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Tue, Feb 01, 2022 at 12:56:51PM -0700, Mathnerd314 wrote:
> > On Mon, Jan 31, 2022 at 5:30 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > So while it's worth adding EINTR to the man page, I don't think it's
> > > worth going through an exercise of trying to add every possible
> > > errno to every syscall.
> >
> > It's true that POSIX's error list is purely a guideline. But that
> > doesn't mean Linux can't specify the precise set of possible error
> > codes. There are currently 133 error codes defined, across an equally
> > large number of syscalls - coding defensively and handling every
> > combination is impossible. Not to mention that the meaning of the
> > error codes differs from syscall to syscall. But with precise
> > information there are likely only 5-10 codes per syscall, so handling
> > every error appropriately is feasible. So the information can be quite
> > useful.
>
> Ahahahaha.  For one, every filesystem gets to return its own errors.
> So basically any syscall that takes an fd gets to return all the errors
> that any of our 50+ filesystems decides to use.

Right. This is where static analysis would help, allowing an audit of
all the error codes returned by filesystems.
The simple approximation `git grep -ho '\-E[A-Z]*' fs | sort -u | wc
-l` = 142 suggests all error codes are in use by filesystem APIs.

>
> Then there are the interposers.  Things like seccomp, Linux Security
> Modules, and other such hooks which might return their own errors.

Here the situation is much better, replacing fs with security in the
command above gives only 56 error codes.

I guess the static analysis might need some help tracing through the
syscall -> function pointer call -> security call indirection, and
similarly for the vfs indirection.

>
> On top of that there's error injection, which might allow eBPF to
> return any error that userspace has deemed appropriate from basically
> any function.

You're right, it is possible: https://github.com/trailofbits/ebpfault.
But the point of error injection is to increase reliability against
real errors, so injecting an error that a syscall couldn't possibly
return would be really dumb.
I think a note in the syscall(2) man pages that "eBPF overrides allow
any syscall to return any error code" is sufficient.

>
> Further, you generally don't execute syscalls.  You call glibc, which
> might decide to return its own errors, not even bothering to call
> the kernel.

I don't think glibc modifies the error codes it gets from the kernel.
And Go and Zig don't use glibc for syscalls, so they definitely use
unmodified error codes.

If glibc doesn't call the kernel, then it's a glibc thing, not
relevant at all to this discussion.

>
> But the real problem is that the kernel might decide to return new
> errors at any time.  Just because you exhaustively handled every error
> that could have occurred in 5.15 doesn't mean that there might not be
> an error returned by 5.20.

The ABI stability guarantee for syscalls
(https://www.kernel.org/doc/Documentation/ABI/stable/syscalls) says
that interface will be added to and not have things removed from it.
This is kind of vague but the way I interpret it is that if a syscall
returns a certain error code in a specific situation, then it's always
going to return that error code. So for example open() could only
return a new error code if it was a new filesystem or a new flag - the
ext4 open() codes are frozen at this point.
So assuming that, new errors have very few places to show up
unexpectedly in the ABI.

>
> This isn't even an Herculean task.  This is Sisyphean, and you'll
> probably feel like Prometheus by the end of it.  I mean, you'd learn a
> lot by attempting it, but you'd be better off picking a task that would
> actually help people.

Zig inspired me because it converts all unrecognized error codes to
"unexpected error":
https://github.com/ziglang/zig/blob/5210b9074ca68c23da474b0afcaa4ce11f7cc57c/lib/std/os.zig#L4979
I like the approach, but it does seem Zig is missing a few 100 possible errors.

I've opened a Zig bug with your line of reasoning:
https://github.com/ziglang/zig/issues/10776

-- Mathnerd314