linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Olga Kornievskaia <aglo@umich.edu>
Cc: Trond Myklebust <trondmy@gmail.com>,
	"J. Bruce Fields" <bfields@redhat.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	linux-nfs <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfsd: Fix error return values for nfsd4_clone_file_range()
Date: Fri, 25 Jan 2019 15:15:51 -0500	[thread overview]
Message-ID: <20190125201551.GB5173@fieldses.org> (raw)
In-Reply-To: <20190125201037.GA5173@fieldses.org>

On Fri, Jan 25, 2019 at 03:10:37PM -0500, J. Bruce Fields wrote:
> Yeah.  I was assuming it could happen in the case you ask to clone
> beyond the end of the source file.  But looking at the code, there's a
> check for that case in generic_remap_checks() before doing the clone,
> and while holding a write lock on i_rwsem (I assume that's enough to
> hold the file size constant).  At least that's true in the cases (btrfs
> & xfs) that I checked.
> 
> So, I don't know, maybe that check is just dead code.

In the xfs case it looks like the main work of the clone is done in
xfs_reflink_remap_blocks(), where there's a loop like:

	while (len) {
		... mysterious code that clones range_len worth of
		    extents?
		if (fatal_signal_pending(current)) {
			error =-EINTR;
			break;
		}
		...
		len -= range_len;
		remapped_len += range_len;
	}

And then it ends up returning remapped_len if it's positive.

So it looks to me like if you do a big clone on xfs and kill the
process, it can clone part of the range, return the amount cloned, and
then the ioctl code will throw away that amount and just return EINVAL,
with the result that the application thinks the operation failed
completely actually it cloned a bunch of data.

--b.

  reply	other threads:[~2019-01-25 20:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-21 20:58 [PATCH] nfsd: Fix error return values for nfsd4_clone_file_range() Trond Myklebust
2019-01-25  0:46 ` J. Bruce Fields
2019-01-25  5:50   ` Trond Myklebust
2019-01-25 16:32     ` J. Bruce Fields
2019-01-25 16:42       ` Olga Kornievskaia
2019-01-25 20:10         ` J. Bruce Fields
2019-01-25 20:15           ` J. Bruce Fields [this message]
2019-01-25 20:57             ` Darrick J. Wong
2019-01-26 22:36               ` J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190125201551.GB5173@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=aglo@umich.edu \
    --cc=bfields@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trondmy@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).