Re: [PATCH] fsdax: unshare: zero destination if srcmap is HOLE or UNWRITTEN

[Shiyang Ruan <ruansy.fnst@fujitsu.com>]

在 2023/3/24 12:17, Matthew Wilcox 写道:
> On Fri, Mar 24, 2023 at 11:42:53AM +0800, Shiyang Ruan wrote:
>>
>>
>> 在 2023/3/24 11:33, Matthew Wilcox 写道:
>>> On Fri, Mar 24, 2023 at 09:50:54AM +0800, Shiyang Ruan wrote:
>>>>
>>>>
>>>> 在 2023/3/24 6:11, Andrew Morton 写道:
>>>>> On Thu, 23 Mar 2023 14:50:38 +0800 Shiyang Ruan <ruansy.fnst@fujitsu.com> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> 在 2023/3/23 7:03, Andrew Morton 写道:
>>>>>>> On Wed, 22 Mar 2023 11:11:09 +0000 Shiyang Ruan <ruansy.fnst@fujitsu.com> wrote:
>>>>>>>
>>>>>>>> unshare copies data from source to destination. But if the source is
>>>>>>>> HOLE or UNWRITTEN extents, we should zero the destination, otherwise the
>>>>>>>> result will be unexpectable.
>>>>>>>
>>>>>>> Please provide much more detail on the user-visible effects of the bug.
>>>>>>> For example, are we leaking kernel memory contents to userspace?
>>>>>>
>>>>>> This fixes fail of generic/649.
>>>>>
>>>>> OK, but this doesn't really help.  I'm trying to determine whether this
>>>>> fix should be backported into -stable kernels and whether it should be
>>>>> fast-tracked into Linus's current -rc tree.
>>>>>
>>>>> But to determine this I (and others) need to know what effect the bug
>>>>> has upon our users.
>>>>
>>>> I didn't get any bug report form users.  I just found this by running
>>>> xfstests.  The phenomenon of this problem is: if we funshare a reflinked
>>>> file which contains HOLE extents, the result of the HOLE extents should be
>>>> zero but actually not (unexpectable data).
>>>
>>> You still aren't answering the question.  If this did happen to a user,
>>> what would they see in the file?  Random data?  Something somebody else
>>> wrote some time ago?  A copy of /etc/passwd, perhaps?  A copy of your
>>> credit card number?
>>
>> Ok.  If this happenned to a user, the HOLE or UNWRITTEN part will be old
>> data of the new allocated extent because it didn't be cleared.
> 
> ie it's the data that was in whatever file happened to use that space
> last, so this is a security bug because it's a data leak, and a backport
> is needed, and you should have indicated that by putting a cc: stable
> tag on the patch?

Yes, cc stable is needed.  Then should I send a new patch with the tag 
added?


--
Thanks,
Ruan.