* [RFC PATCH 1/1] device-dax: check for vma range while dax_mmap.
@ 2018-07-31 11:46 Zhang Yi
2018-08-01 19:40 ` Dave Jiang
0 siblings, 1 reply; 5+ messages in thread
From: Zhang Yi @ 2018-07-31 11:46 UTC (permalink / raw)
To: linux-kernel, linux-nvdimm, dan.j.williams, jack, zwisler,
dave.jiang, yu.c.zhang
Cc: yi.z.zhang
It should be prevent user map an illegal vma range which larger than
dax device phiscal resourse, as we don't have swap logic while page
faulting in dax device.
Applications, especailly qemu, map the /dev/dax for virtual nvdimm's
backend device, we defined the v-nvdimm label area at the end of mapped
rang. By using an illegal size that exceeds the physical resource of
/dev/dax, then it will triger qemu a signal fault while accessing these
label area.
Signed-off-by: Zhang Yi <yi.z.zhang@linux.intel.com>
---
drivers/dax/device.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index aff2c15..c9a50cd 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -177,6 +177,32 @@ static const struct attribute_group *dax_attribute_groups[] = {
NULL,
};
+static int check_vma_range(struct dev_dax *dev_dax, struct vm_area_struct *vma,
+ const char *func)
+{
+ struct device *dev = &dev_dax->dev;
+ struct resource *res;
+ unsigned long size;
+ int ret, i;
+
+ if (!dax_alive(dev_dax->dax_dev))
+ return -ENXIO;
+
+ size = vma->vm_end - vma->vm_start + (vma->vm_pgoff << PAGE_SHIFT);
+ ret = -EINVAL;
+ for (i = 0; i < dev_dax->num_resources; i++) {
+ res = &dev_dax->res[i];
+ if (size > resource_size(res)) {
+ dev_info(dev, "%s: %s: fail, vma range is overflow\n",
+ current->comm, func);
+ ret = -EINVAL;
+ continue;
+ } else
+ return 0;
+ }
+ return ret;
+}
+
static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma,
const char *func)
{
@@ -465,6 +491,8 @@ static int dax_mmap(struct file *filp, struct vm_area_struct *vma)
*/
id = dax_read_lock();
rc = check_vma(dev_dax, vma, __func__);
+ if (!rc)
+ rc |= check_vma_range(dev_dax, vma, __func__);
dax_read_unlock(id);
if (rc)
return rc;
--
2.7.4
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [RFC PATCH 1/1] device-dax: check for vma range while dax_mmap.
2018-07-31 11:46 [RFC PATCH 1/1] device-dax: check for vma range while dax_mmap Zhang Yi
@ 2018-08-01 19:40 ` Dave Jiang
2018-08-02 9:32 ` Zhang,Yi
0 siblings, 1 reply; 5+ messages in thread
From: Dave Jiang @ 2018-08-01 19:40 UTC (permalink / raw)
To: Zhang Yi, linux-kernel, linux-nvdimm, dan.j.williams, jack,
zwisler, yu.c.zhang
Cc: yi.z.zhang
On 07/31/2018 04:46 AM, Zhang Yi wrote:
> It should be prevent user map an illegal vma range which larger than
> dax device phiscal resourse, as we don't have swap logic while page
> faulting in dax device.
This patch prevents a user mapping an illegal vma range that is larger
than a dax device physical resource.
>
> Applications, especailly qemu, map the /dev/dax for virtual nvdimm's
> backend device, we defined the v-nvdimm label area at the end of mapped
> rang. By using an illegal size that exceeds the physical resource of
> /dev/dax, then it will triger qemu a signal fault while accessing these
> label area.
When qemu maps the dax device for virtual nvdimm's backend device, the
v-nvdimm label area is defined at the end of mapped range. By using an
illegal size that exceeds the range of the device dax, it will trigger a
fault with qemu.
>
> Signed-off-by: Zhang Yi <yi.z.zhang@linux.intel.com>
> ---
> drivers/dax/device.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index aff2c15..c9a50cd 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -177,6 +177,32 @@ static const struct attribute_group *dax_attribute_groups[] = {
> NULL,
> };
>
> +static int check_vma_range(struct dev_dax *dev_dax, struct vm_area_struct *vma,
> + const char *func)
> +{
> + struct device *dev = &dev_dax->dev;
> + struct resource *res;
> + unsigned long size;
> + int ret, i;
> +
> + if (!dax_alive(dev_dax->dax_dev))
> + return -ENXIO;
> +
> + size = vma->vm_end - vma->vm_start + (vma->vm_pgoff << PAGE_SHIFT);
> + ret = -EINVAL;
> + for (i = 0; i < dev_dax->num_resources; i++) {
> + res = &dev_dax->res[i];
> + if (size > resource_size(res)) {
> + dev_info(dev, "%s: %s: fail, vma range is overflow\n",
> + current->comm, func);
> + ret = -EINVAL;
> + continue;
> + } else
> + return 0;
> + }
> + return ret;
> +}
> +
> static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma,
> const char *func)
> {
> @@ -465,6 +491,8 @@ static int dax_mmap(struct file *filp, struct vm_area_struct *vma)
> */
> id = dax_read_lock();
> rc = check_vma(dev_dax, vma, __func__);
> + if (!rc)
> + rc |= check_vma_range(dev_dax, vma, __func__);
I think you want to augment check_vma() rather than adding another
function? If this is added inside check_vma() then you can also skip the
!dax_alive() check. Do you expect this function to be called anywhere else?
> dax_read_unlock(id);
> if (rc)
> return rc;
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC PATCH 1/1] device-dax: check for vma range while dax_mmap.
2018-08-01 19:40 ` Dave Jiang
@ 2018-08-02 9:32 ` Zhang,Yi
2018-08-08 0:05 ` Dave Jiang
0 siblings, 1 reply; 5+ messages in thread
From: Zhang,Yi @ 2018-08-02 9:32 UTC (permalink / raw)
To: Dave Jiang, linux-kernel, linux-nvdimm, dan.j.williams, jack,
zwisler, yu.c.zhang
Cc: yi.z.zhang
On 2018年08月02日 03:40, Dave Jiang wrote:
>
> On 07/31/2018 04:46 AM, Zhang Yi wrote:
>> It should be prevent user map an illegal vma range which larger than
>> dax device phiscal resourse, as we don't have swap logic while page
>> faulting in dax device.
> This patch prevents a user mapping an illegal vma range that is larger
> than a dax device physical resource.
>
>> Applications, especailly qemu, map the /dev/dax for virtual nvdimm's
>> backend device, we defined the v-nvdimm label area at the end of mapped
>> rang. By using an illegal size that exceeds the physical resource of
>> /dev/dax, then it will triger qemu a signal fault while accessing these
>> label area.
> When qemu maps the dax device for virtual nvdimm's backend device, the
> v-nvdimm label area is defined at the end of mapped range. By using an
> illegal size that exceeds the range of the device dax, it will trigger a
> fault with qemu.
Thanks Dava, that's could be much better.
>
>> Signed-off-by: Zhang Yi <yi.z.zhang@linux.intel.com>
>> ---
>> drivers/dax/device.c | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
>> index aff2c15..c9a50cd 100644
>> --- a/drivers/dax/device.c
>> +++ b/drivers/dax/device.c
>> @@ -177,6 +177,32 @@ static const struct attribute_group *dax_attribute_groups[] = {
>> NULL,
>> };
>>
>> +static int check_vma_range(struct dev_dax *dev_dax, struct vm_area_struct *vma,
>> + const char *func)
>> +{
>> + struct device *dev = &dev_dax->dev;
>> + struct resource *res;
>> + unsigned long size;
>> + int ret, i;
>> +
>> + if (!dax_alive(dev_dax->dax_dev))
>> + return -ENXIO;
>> +
>> + size = vma->vm_end - vma->vm_start + (vma->vm_pgoff << PAGE_SHIFT);
>> + ret = -EINVAL;
>> + for (i = 0; i < dev_dax->num_resources; i++) {
>> + res = &dev_dax->res[i];
>> + if (size > resource_size(res)) {
>> + dev_info(dev, "%s: %s: fail, vma range is overflow\n",
>> + current->comm, func);
>> + ret = -EINVAL;
>> + continue;
>> + } else
>> + return 0;
>> + }
>> + return ret;
>> +}
>> +
>> static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma,
>> const char *func)
>> {
>> @@ -465,6 +491,8 @@ static int dax_mmap(struct file *filp, struct vm_area_struct *vma)
>> */
>> id = dax_read_lock();
>> rc = check_vma(dev_dax, vma, __func__);
>> + if (!rc)
>> + rc |= check_vma_range(dev_dax, vma, __func__);
> I think you want to augment check_vma() rather than adding another
> function? If this is added inside check_vma() then you can also skip the
> !dax_alive() check. Do you expect this function to be called anywhere else?
since check_vma range also will be called while dax page faulting. I
don't wanna this check_vma_range introduce some additional workload
while page fault. just let it checked in the mmap scenario
>
>> dax_read_unlock(id);
>> if (rc)
>> return rc;
>>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC PATCH 1/1] device-dax: check for vma range while dax_mmap.
2018-08-02 9:32 ` Zhang,Yi
@ 2018-08-08 0:05 ` Dave Jiang
2018-08-09 11:01 ` Zhang,Yi
0 siblings, 1 reply; 5+ messages in thread
From: Dave Jiang @ 2018-08-08 0:05 UTC (permalink / raw)
To: Zhang,Yi, linux-kernel, linux-nvdimm, dan.j.williams, jack,
zwisler, yu.c.zhang
Cc: yi.z.zhang
On 08/02/2018 02:32 AM, Zhang,Yi wrote:
>
>
> On 2018年08月02日 03:40, Dave Jiang wrote:
>>
>> On 07/31/2018 04:46 AM, Zhang Yi wrote:
>>> It should be prevent user map an illegal vma range which larger than
>>> dax device phiscal resourse, as we don't have swap logic while page
>>> faulting in dax device.
>> This patch prevents a user mapping an illegal vma range that is larger
>> than a dax device physical resource.
>>
>>> Applications, especailly qemu, map the /dev/dax for virtual nvdimm's
>>> backend device, we defined the v-nvdimm label area at the end of mapped
>>> rang. By using an illegal size that exceeds the physical resource of
>>> /dev/dax, then it will triger qemu a signal fault while accessing these
>>> label area.
>> When qemu maps the dax device for virtual nvdimm's backend device, the
>> v-nvdimm label area is defined at the end of mapped range. By using an
>> illegal size that exceeds the range of the device dax, it will trigger a
>> fault with qemu.
> Thanks Dava, that's could be much better.
>>
>>> Signed-off-by: Zhang Yi <yi.z.zhang@linux.intel.com>
>>> ---
>>> drivers/dax/device.c | 28 ++++++++++++++++++++++++++++
>>> 1 file changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
>>> index aff2c15..c9a50cd 100644
>>> --- a/drivers/dax/device.c
>>> +++ b/drivers/dax/device.c
>>> @@ -177,6 +177,32 @@ static const struct attribute_group *dax_attribute_groups[] = {
>>> NULL,
>>> };
>>>
>>> +static int check_vma_range(struct dev_dax *dev_dax, struct vm_area_struct *vma,
>>> + const char *func)
>>> +{
>>> + struct device *dev = &dev_dax->dev;
>>> + struct resource *res;
>>> + unsigned long size;
>>> + int ret, i;
>>> +
>>> + if (!dax_alive(dev_dax->dax_dev))
>>> + return -ENXIO;
>>> +
>>> + size = vma->vm_end - vma->vm_start + (vma->vm_pgoff << PAGE_SHIFT);
>>> + ret = -EINVAL;
>>> + for (i = 0; i < dev_dax->num_resources; i++) {
>>> + res = &dev_dax->res[i];
>>> + if (size > resource_size(res)) {
>>> + dev_info(dev, "%s: %s: fail, vma range is overflow\n",
>>> + current->comm, func);
>>> + ret = -EINVAL;
>>> + continue;
>>> + } else
>>> + return 0;
>>> + }
>>> + return ret;
>>> +}
>>> +
>>> static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma,
>>> const char *func)
>>> {
>>> @@ -465,6 +491,8 @@ static int dax_mmap(struct file *filp, struct vm_area_struct *vma)
>>> */
>>> id = dax_read_lock();
>>> rc = check_vma(dev_dax, vma, __func__);
>>> + if (!rc)
>>> + rc |= check_vma_range(dev_dax, vma, __func__);
I don't see any reason to logical or the return code. It should be 0, so
you can just assign it to check_vma_range().
>> I think you want to augment check_vma() rather than adding another
>> function? If this is added inside check_vma() then you can also skip the
>> !dax_alive() check. Do you expect this function to be called anywhere else?
> since check_vma range also will be called while dax page faulting. I
> don't wanna this check_vma_range introduce some additional workload
> while page fault. just let it checked in the mmap scenario
Good point. Ok.
>>
>>> dax_read_unlock(id);
>>> if (rc)
>>> return rc;
>>>
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC PATCH 1/1] device-dax: check for vma range while dax_mmap.
2018-08-08 0:05 ` Dave Jiang
@ 2018-08-09 11:01 ` Zhang,Yi
0 siblings, 0 replies; 5+ messages in thread
From: Zhang,Yi @ 2018-08-09 11:01 UTC (permalink / raw)
To: Dave Jiang, linux-kernel, linux-nvdimm, dan.j.williams, jack,
zwisler, yu.c.zhang
Cc: yi.z.zhang
On 2018年08月08日 08:05, Dave Jiang wrote:
>
> On 08/02/2018 02:32 AM, Zhang,Yi wrote:
>>
>> On 2018年08月02日 03:40, Dave Jiang wrote:
>>> On 07/31/2018 04:46 AM, Zhang Yi wrote:
>>>> It should be prevent user map an illegal vma range which larger than
>>>> dax device phiscal resourse, as we don't have swap logic while page
>>>> faulting in dax device.
>>> This patch prevents a user mapping an illegal vma range that is larger
>>> than a dax device physical resource.
>>>
>>>> Applications, especailly qemu, map the /dev/dax for virtual nvdimm's
>>>> backend device, we defined the v-nvdimm label area at the end of mapped
>>>> rang. By using an illegal size that exceeds the physical resource of
>>>> /dev/dax, then it will triger qemu a signal fault while accessing these
>>>> label area.
>>> When qemu maps the dax device for virtual nvdimm's backend device, the
>>> v-nvdimm label area is defined at the end of mapped range. By using an
>>> illegal size that exceeds the range of the device dax, it will trigger a
>>> fault with qemu.
>> Thanks Dava, that's could be much better.
>>>> Signed-off-by: Zhang Yi <yi.z.zhang@linux.intel.com>
>>>> ---
>>>> drivers/dax/device.c | 28 ++++++++++++++++++++++++++++
>>>> 1 file changed, 28 insertions(+)
>>>>
>>>> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
>>>> index aff2c15..c9a50cd 100644
>>>> --- a/drivers/dax/device.c
>>>> +++ b/drivers/dax/device.c
>>>> @@ -177,6 +177,32 @@ static const struct attribute_group *dax_attribute_groups[] = {
>>>> NULL,
>>>> };
>>>>
>>>> +static int check_vma_range(struct dev_dax *dev_dax, struct vm_area_struct *vma,
>>>> + const char *func)
>>>> +{
>>>> + struct device *dev = &dev_dax->dev;
>>>> + struct resource *res;
>>>> + unsigned long size;
>>>> + int ret, i;
>>>> +
>>>> + if (!dax_alive(dev_dax->dax_dev))
>>>> + return -ENXIO;
>>>> +
>>>> + size = vma->vm_end - vma->vm_start + (vma->vm_pgoff << PAGE_SHIFT);
>>>> + ret = -EINVAL;
>>>> + for (i = 0; i < dev_dax->num_resources; i++) {
>>>> + res = &dev_dax->res[i];
>>>> + if (size > resource_size(res)) {
>>>> + dev_info(dev, "%s: %s: fail, vma range is overflow\n",
>>>> + current->comm, func);
>>>> + ret = -EINVAL;
>>>> + continue;
>>>> + } else
>>>> + return 0;
>>>> + }
>>>> + return ret;
>>>> +}
>>>> +
>>>> static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma,
>>>> const char *func)
>>>> {
>>>> @@ -465,6 +491,8 @@ static int dax_mmap(struct file *filp, struct vm_area_struct *vma)
>>>> */
>>>> id = dax_read_lock();
>>>> rc = check_vma(dev_dax, vma, __func__);
>>>> + if (!rc)
>>>> + rc |= check_vma_range(dev_dax, vma, __func__);
> I don't see any reason to logical or the return code. It should be 0, so
> you can just assign it to check_vma_range().
Ah.. Yes, Thanks for your kindly review, Dace, I will send the next version.
>
>>> I think you want to augment check_vma() rather than adding another
>>> function? If this is added inside check_vma() then you can also skip the
>>> !dax_alive() check. Do you expect this function to be called anywhere else?
>> since check_vma range also will be called while dax page faulting. I
>> don't wanna this check_vma_range introduce some additional workload
>> while page fault. just let it checked in the mmap scenario
> Good point. Ok.
>
>>>> dax_read_unlock(id);
>>>> if (rc)
>>>> return rc;
>>>>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-08-09 3:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-31 11:46 [RFC PATCH 1/1] device-dax: check for vma range while dax_mmap Zhang Yi
2018-08-01 19:40 ` Dave Jiang
2018-08-02 9:32 ` Zhang,Yi
2018-08-08 0:05 ` Dave Jiang
2018-08-09 11:01 ` Zhang,Yi
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).