GNU bug report logs - #60489
Programs should exit after EIO from FICLONE or copy_file_range()

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 60489 in the body.
You can then email your comments to 60489 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Noah Misch <noah <at> leadboat.com>
To: bug-coreutils <at> gnu.org
Subject: Programs should exit after EIO from FICLONE or copy_file_range()
Date: Sun, 1 Jan 2023 22:36:20 -0800

Because Debian coreutils 9.1-1 "cp" silently falls back to copy_file_range()
after FICLONE reports EIO, "cp" can transfer incorrect bytes.  Linux syscalls
having a file descriptor parameter report EIO after a fault in the underlying
device.  The affected file is not recoverable in the general case, but syscall
outcomes after the EIO don't reflect that.  For example, consider FICLONE
returning EIO for a fault during source file writeback.  The kernel will mark
"clean" the affected page cache entries and clear the EIO state.  If the page
cache evicts those pages, their file offsets revert to the last written-back
values if any, else zeros.  If userspace issues a syscall that bypasses the
page cache, like copy_file_range() or another FICLONE, that syscall clones the
last written-back state or zeroes.  See
https://lore.kernel.org/linux-xfs/20221108172436.GA3613139 <at> rfd.leadboat.com
for a "cp" and "cat" test script, background, and discussion.

I recommend instead reporting the EIO and terminating when FICLONE or
copy_file_range() fails with EIO.  One could argue that ENOSPC also warrants
termination, since no fallback reduces space usage.  For other errno values,
fallback to the next transfer strategy, like today.  An alternative would be
to fallback from FICLONE to copy_file_range() only after known-appropriate
errors EBADF, EINVAL, EOPNOTSUPP, ETXTBSY, and EXDEV.  That alternative wins
if future FICLONE reports an additional termination-deserving errno value.
Since just EIO needs termination today, I bet new errno values are more likely
than not to deserve fallback.  What do you think?

Thanks,
nm

Message #8 received at 60489 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Noah Misch <noah <at> leadboat.com>, 60489 <at> debbugs.gnu.org
Subject: Re: bug#60489: Programs should exit after EIO from FICLONE or
 copy_file_range()
Date: Mon, 2 Jan 2023 13:19:39 +0000

[Message part 1 (text/plain, inline)]

On 02/01/2023 06:36, Noah Misch wrote:
> Because Debian coreutils 9.1-1 "cp" silently falls back to copy_file_range()
> after FICLONE reports EIO, "cp" can transfer incorrect bytes.  Linux syscalls
> having a file descriptor parameter report EIO after a fault in the underlying
> device.  The affected file is not recoverable in the general case, but syscall
> outcomes after the EIO don't reflect that.  For example, consider FICLONE
> returning EIO for a fault during source file writeback.  The kernel will mark
> "clean" the affected page cache entries and clear the EIO state.  If the page
> cache evicts those pages, their file offsets revert to the last written-back
> values if any, else zeros.  If userspace issues a syscall that bypasses the
> page cache, like copy_file_range() or another FICLONE, that syscall clones the
> last written-back state or zeroes.  See
> https://lore.kernel.org/linux-xfs/20221108172436.GA3613139 <at> rfd.leadboat.com
> for a "cp" and "cat" test script, background, and discussion.
> 
> I recommend instead reporting the EIO and terminating when FICLONE or
> copy_file_range() fails with EIO.  One could argue that ENOSPC also warrants
> termination, since no fallback reduces space usage.  For other errno values,
> fallback to the next transfer strategy, like today.  An alternative would be
> to fallback from FICLONE to copy_file_range() only after known-appropriate
> errors EBADF, EINVAL, EOPNOTSUPP, ETXTBSY, and EXDEV.  That alternative wins
> if future FICLONE reports an additional termination-deserving errno value.
> Since just EIO needs termination today, I bet new errno values are more likely
> than not to deserve fallback.  What do you think?

Yes we should have an allow list that we retry for,
and otherwise fail for EIO, ENOSPC, ENOMEM, ...

We already do this for copy_file_range(),
but should treat FICLONE similarly,
as done in the attached.

thanks!
Pádraig

[copy-ficlone-fail.patch (text/x-patch, attachment)]

Message #11 received at 60489 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 60489 <at> debbugs.gnu.org, Noah Misch <noah <at> leadboat.com>
Subject: Re: bug#60489: Programs should exit after EIO from FICLONE or
 copy_file_range()
Date: Mon, 02 Jan 2023 14:28:49 +0100

On Jan 02 2023, Pádraig Brady wrote:

> +          /* Note error set is consistent with copy_file_range().  */
> +          bool no_clone_attempted = errno == ENOSYS || is_ENOTSUP (errno)
> +                                    || errno == EINVAL || errno == EBADF
> +                                    || errno == EXDEV || errno == ETXTBSY;

Should this be refactored to avoid duplication?

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

Message #14 received at 60489 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: 60489 <at> debbugs.gnu.org, Noah Misch <noah <at> leadboat.com>
Subject: Re: bug#60489: Programs should exit after EIO from FICLONE or
 copy_file_range()
Date: Mon, 2 Jan 2023 16:22:03 +0000

[Message part 1 (text/plain, inline)]

On 02/01/2023 13:28, Andreas Schwab wrote:
> On Jan 02 2023, Pádraig Brady wrote:
> 
>> +          /* Note error set is consistent with copy_file_range().  */
>> +          bool no_clone_attempted = errno == ENOSYS || is_ENOTSUP (errno)
>> +                                    || errno == EINVAL || errno == EBADF
>> +                                    || errno == EXDEV || errno == ETXTBSY;
> 
> Should this be refactored to avoid duplication?

Yes good call.
We should also refactor the handling of clone failure
to also apply to the usage of fclonefileat() on macOS.

Updated patch attached.

cheers,
Pádraig

[copy-clone-fail.patch (text/x-patch, attachment)]

Message #19 received at 60489-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: 60489-done <at> debbugs.gnu.org, Noah Misch <noah <at> leadboat.com>
Subject: Re: bug#60489: Programs should exit after EIO from FICLONE or
 copy_file_range()
Date: Fri, 6 Jan 2023 14:47:43 +0000

On 02/01/2023 16:22, Pádraig Brady wrote:
> On 02/01/2023 13:28, Andreas Schwab wrote:
>> On Jan 02 2023, Pádraig Brady wrote:
>>
>>> +          /* Note error set is consistent with copy_file_range().  */
>>> +          bool no_clone_attempted = errno == ENOSYS || is_ENOTSUP (errno)
>>> +                                    || errno == EINVAL || errno == EBADF
>>> +                                    || errno == EXDEV || errno == ETXTBSY;
>>
>> Should this be refactored to avoid duplication?
> 
> Yes good call.
> We should also refactor the handling of clone failure
> to also apply to the usage of fclonefileat() on macOS.
> 
> Updated patch attached.

Pushed.

Marking this as done.

thanks,
Pádraig

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.