All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] syscalls/copy_file_range02.c: Compatible with new and old kernels
Date: Thu, 4 Jul 2019 18:35:04 +0800	[thread overview]
Message-ID: <5D1DD658.8050303@cn.fujitsu.com> (raw)
In-Reply-To: <CAOQ4uxjxOWCOBV1OQVg9B5D2eT4jUdQfOAaxTSJgFkqQDUVapg@mail.gmail.com>

on 2019/06/27 16:39, Amir Goldstein wrote:
> On Thu, Jun 27, 2019 at 11:03 AM Yang Xu<xuyang2018.jy@cn.fujitsu.com>  wrote:
>> on 2019/05/31 21:02, Amir Goldstein wrote:
>>
>>> On Fri, May 31, 2019 at 3:03 PM Li Wang<liwang@redhat.com>   wrote:
>>>>
>>>> On Fri, May 31, 2019 at 6:15 PM Xiao Yang<yangx.jy@cn.fujitsu.com>   wrote:
>>>>> On 2019/05/31 16:44, Jinhui huang wrote:
>>>>>> On new kernels, copy_file_range() returned EISDIR when copyed contents
>>>>>> to directory, but on old kernels, it returned EBADF, we should accept
>>>>>> EBADF to be compatible with new and old kernels.
>>>>>>
>>>>>> The patch as follows:
>>>>>> commit 11cbfb10775a ("vfs: deny copy_file_range() for non regular files")
>>>>> Hi,
>>>>>
>>>>>   From description of commit, I wonder if we can add more tests for some
>>>>> non regular files(e.g. block, pipe)?
>>>> I have no objection on this. But, is there really make sense to test some more non regular files which not being mentioned by Linux Manual Page?
>>>>
>>> FYI, more changes to copy_file_range checks are in the works:
>>> https://lore.kernel.org/linux-fsdevel/20190526061100.21761-1-amir73il@gmail.com/T/#me34d4363449118bd3b2ec8421d282a77e9a7d2d1
>> Hi Amir
>>
>>       Meet again.  We can increase ltp copy_file_range02 coverage include( swapfile ->ETXTBUSY,  immutable file->EPERM) as same as xfstests generic/553[4].
>> Also the two other checks(overlaping and offset wrap).  I am glad to do it.
> That would be great!

Hi Amir

I have tried it.  swapfile and immutable file has no problem, but when I try to reproduce EINVAL(same file overlaping) without xfs_io, I got EFAULT error.
It look likes we must depend on the new xfs_io feature patch.  Right?

ps: If it must xfs_io command, I think we can not check this situation because we should only check by copy_file_range syscall.
what do you think about it?

another question:
I have seen copy_file_range manpage,  it says fd_out data can be overwriting, but I got EFAULT when I use the following code.

open(src_path, O_RDWR|O_CREAT, 0644) = fd_copy
open(copy_path, O_RDWR|O_CREAT, 0644) = fd_src
SAFE_WRITE(1, fd_src,  CONTENT,  CONTSIZE);
SAFE_WRITE(1, fd_copy,  CONTENT,  CONTSIZE);
copy_file_range(fd_src,0, fd_copy, CONTSIZE/4, CONTSIZE/2, 0) = -1 EFAULT

fs/read_write.c copy_file_range function copy_from_user reports this error. If off_out or off_in isn't equal to 0, the error occurs.
  
---------------------
ret= -EFAULT;
....
if (off_out) {
                copy_from_user(&pos_out, off_out, sizeof(loff_t));

                         goto out;
              }
----------------------

Is it a bug or I miss something?

Thanks
Yang Xu

>
>> PS: Why we don't have test for overlaping and offset wrap check on xfstests? Or, I miss it?
> The bounds check test was posted to xfstests:
> https://marc.info/?l=linux-xfs&m=155947929219690&w=2
>
> But because the test requires a new feature from xfs_io:
> https://marc.info/?l=linux-xfs&m=156152984109774&w=2
>
> I recommended that xfstests maintainer will hold off merging the test,
> before the
> required change is at least in xfsprogs for-next branch.
>
> Thanks,
> Amir.
>
>
>




  reply	other threads:[~2019-07-04 10:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-31  8:44 [LTP] [PATCH] syscalls/copy_file_range02.c: Compatible with new and old kernels Jinhui huang
2019-05-31 10:02 ` Li Wang
2019-05-31 10:15 ` Xiao Yang
2019-05-31 12:03   ` Li Wang
2019-05-31 12:26     ` Cyril Hrubis
2019-05-31 12:46       ` Li Wang
2019-05-31 13:02     ` Amir Goldstein
2019-06-27  8:03       ` Yang Xu
2019-06-27  8:39         ` Amir Goldstein
2019-07-04 10:35           ` Yang Xu [this message]
2019-07-04 11:15             ` Amir Goldstein
2019-05-31 11:01 ` Cyril Hrubis
2019-05-31 11:47   ` Li Wang
2019-05-31 12:00     ` Cyril Hrubis

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=5D1DD658.8050303@cn.fujitsu.com \
    --to=xuyang2018.jy@cn.fujitsu.com \
    --cc=ltp@lists.linux.it \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.