Re: [PATCH] dax: fix offset overflow in dax_io

[Dan Williams <dan.j.williams@intel.com>]

On Sat, Jun 25, 2016 at 11:49 AM, Eric Sandeen <sandeen@sandeen.net> wrote:
>
> On 6/25/16 12:39 PM, Dan Williams wrote:
>> On Thu, Jun 23, 2016 at 2:54 PM, Eric Sandeen <sandeen@redhat.com> wrote:
>>> This isn't functionally apparent for some reason, but
>>> when we test io at extreme offsets at the end of the loff_t
>>> rang, such as in fstests xfs/071, the calculation of
>>> "max" in dax_io() can be wrong due to pos + size overflowing.
>>>
>>> For example,
>>>
>>> # xfs_io -c "pwrite 9223372036854771712 512" /mnt/test/file
>>>
>>> enters dax_io with:
>>>
>>> start 0x7ffffffffffff000
>>> end   0x7ffffffffffff200
>>>
>>> and the rounded up "size" variable is 0x1000.  This yields:
>>>
>>> pos + size 0x8000000000000000 (overflows loff_t)
>>>        end 0x7ffffffffffff200
>>>
>>> Due to the overflow, the min() function picks the wrong
>>> value for the "max" variable, and when we send (max - pos)
>>> into i.e. copy_from_iter_pmem() it is also the wrong value.
>>>
>>> This somehow(tm) gets magically absorbed without incident,
>>> probably because iter->count is correct.  But it seems best
>>> to fix it up properly by comparing the two values as
>>> unsigned.
>>
>> So this is 4.8 material, or a user visible bug that we should take
>> into 4.7 and -stable?
>
> I have not seen any user-visible problems upstream, but the same
> behavior certainly exists in older upstream kernels.  To be honest
> I haven't quite sorted out why sending the "wrong" length into
> copy_from_iter_pmem() doesn't seem to matter.
>
> It only exists at the 8EB boundary, so other than fstests specific
> to that scenario, I don't think it's much of a real-world problem.
>
> It's probably fine and safe for -stable, but it's in no way
> critical or urgent IMHO.
>

Ok, I'll queue it with a few other libnvdimm fixes that are pending.