Commit d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") introduced an error in gntdev_mmap(): in case the call of mmu_interval_notifier_insert_locked() fails the exit path should not call mmu_interval_notifier_remove(), as this might result in NULL dereferences.
One reason for failure is e.g. a signal pending for the running process.
Fixes: d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") Cc: stable@vger.kernel.org Reported-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Tested-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Signed-off-by: Juergen Gross jgross@suse.com --- drivers/xen/gntdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index f01d58c7a042..a3e7be96527d 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -1017,8 +1017,10 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) err = mmu_interval_notifier_insert_locked( &map->notifier, vma->vm_mm, vma->vm_start, vma->vm_end - vma->vm_start, &gntdev_mmu_ops); - if (err) + if (err) { + map->vma = NULL; goto out_unlock_put; + } } mutex_unlock(&priv->lock);
On 23 Apr 2021, at 06:40, Juergen Gross jgross@suse.com wrote:
Commit d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") introduced an error in gntdev_mmap(): in case the call of mmu_interval_notifier_insert_locked() fails the exit path should not call mmu_interval_notifier_remove(), as this might result in NULL dereferences.
One reason for failure is e.g. a signal pending for the running process.
Fixes: d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") Cc: stable@vger.kernel.org Reported-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Tested-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Signed-off-by: Juergen Gross jgross@suse.com
drivers/xen/gntdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index f01d58c7a042..a3e7be96527d 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -1017,8 +1017,10 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) err = mmu_interval_notifier_insert_locked( &map->notifier, vma->vm_mm, vma->vm_start, vma->vm_end - vma->vm_start, &gntdev_mmu_ops);
if (err)
if (err) {map->vma = NULL; goto out_unlock_put;}-- 2.26.2
Hi Juergen,
I can see from the code that there is another path to out_unlock_put label some lines before:
[…] vma->vm_private_data = map; if (map->flags) { if ((vma->vm_flags & VM_WRITE) && (map->flags & GNTMAP_readonly)) goto out_unlock_put; } else { map->flags = GNTMAP_host_map; if (!(vma->vm_flags & VM_WRITE)) map->flags |= GNTMAP_readonly; } […]
Can be the case that use_ptemod is != 0 also for that path?
Cheers, Luca
On 23.04.21 08:55, Luca Fancellu wrote:
On 23 Apr 2021, at 06:40, Juergen Gross jgross@suse.com wrote:
Commit d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") introduced an error in gntdev_mmap(): in case the call of mmu_interval_notifier_insert_locked() fails the exit path should not call mmu_interval_notifier_remove(), as this might result in NULL dereferences.
One reason for failure is e.g. a signal pending for the running process.
Fixes: d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") Cc: stable@vger.kernel.org Reported-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Tested-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Signed-off-by: Juergen Gross jgross@suse.com
drivers/xen/gntdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index f01d58c7a042..a3e7be96527d 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -1017,8 +1017,10 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) err = mmu_interval_notifier_insert_locked( &map->notifier, vma->vm_mm, vma->vm_start, vma->vm_end - vma->vm_start, &gntdev_mmu_ops);
if (err)
if (err) {map->vma = NULL; goto out_unlock_put;}-- 2.26.2
Hi Juergen,
I can see from the code that there is another path to out_unlock_put label some lines before:
[…] vma->vm_private_data = map;if (map->flags) { if ((vma->vm_flags & VM_WRITE) && (map->flags & GNTMAP_readonly)) goto out_unlock_put; } else { map->flags = GNTMAP_host_map; if (!(vma->vm_flags & VM_WRITE)) map->flags |= GNTMAP_readonly; } […]
Can be the case that use_ptemod is != 0 also for that path?
map->vma will be NULL in this case, so there will be no problem resulting from that path.
Juergen
On 23 Apr 2021, at 08:00, Juergen Gross jgross@suse.com wrote:
On 23.04.21 08:55, Luca Fancellu wrote:
On 23 Apr 2021, at 06:40, Juergen Gross jgross@suse.com wrote:
Commit d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") introduced an error in gntdev_mmap(): in case the call of mmu_interval_notifier_insert_locked() fails the exit path should not call mmu_interval_notifier_remove(), as this might result in NULL dereferences.
One reason for failure is e.g. a signal pending for the running process.
Fixes: d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") Cc: stable@vger.kernel.org Reported-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Tested-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Signed-off-by: Juergen Gross jgross@suse.com
drivers/xen/gntdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index f01d58c7a042..a3e7be96527d 100644 --- a/drivers/xen/gntdev.c +++ b/drivers/xen/gntdev.c @@ -1017,8 +1017,10 @@ static int gntdev_mmap(struct file *flip, struct vm_area_struct *vma) err = mmu_interval_notifier_insert_locked( &map->notifier, vma->vm_mm, vma->vm_start, vma->vm_end - vma->vm_start, &gntdev_mmu_ops);
if (err)
if (err) {map->vma = NULL; goto out_unlock_put;}-- 2.26.2
Hi Juergen, I can see from the code that there is another path to out_unlock_put label some lines before: […] vma->vm_private_data = map; if (map->flags) { if ((vma->vm_flags & VM_WRITE) && (map->flags & GNTMAP_readonly)) goto out_unlock_put; } else { map->flags = GNTMAP_host_map; if (!(vma->vm_flags & VM_WRITE)) map->flags |= GNTMAP_readonly; } […] Can be the case that use_ptemod is != 0 also for that path?
map->vma will be NULL in this case, so there will be no problem resulting from that path.
Right, thanks, seems good to me.
Reviewed-by: Luca Fancellu luca.fancellu@arm.com
Cheers, Luca
Juergen <OpenPGP_0xB0DE9DD628BF132F.asc>
On Fri, Apr 23, 2021 at 08:04:57AM +0100, Luca Fancellu wrote:
On 23 Apr 2021, at 08:00, Juergen Gross jgross@suse.com wrote: On 23.04.21 08:55, Luca Fancellu wrote:
On 23 Apr 2021, at 06:40, Juergen Gross jgross@suse.com wrote:
Commit d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") introduced an error in gntdev_mmap(): in case the call of mmu_interval_notifier_insert_locked() fails the exit path should not call mmu_interval_notifier_remove(), as this might result in NULL dereferences.
One reason for failure is e.g. a signal pending for the running process.
Fixes: d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") Cc: stable@vger.kernel.org Reported-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Tested-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Signed-off-by: Juergen Gross jgross@suse.com
(...)
Right, thanks, seems good to me.
Reviewed-by: Luca Fancellu luca.fancellu@arm.com
Can somebody ack this fix please?
On 23.04.21 07:40, Juergen Gross wrote:
Commit d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") introduced an error in gntdev_mmap(): in case the call of mmu_interval_notifier_insert_locked() fails the exit path should not call mmu_interval_notifier_remove(), as this might result in NULL dereferences.
One reason for failure is e.g. a signal pending for the running process.
Fixes: d3eeb1d77c5d0af ("xen/gntdev: use mmu_interval_notifier_insert") Cc: stable@vger.kernel.org Reported-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Tested-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Signed-off-by: Juergen Gross jgross@suse.com
Pushed to xen/tip.git for-linus-5.13b
Juergen
linux-stable-mirror@lists.linaro.org
-
Juergen Gross -
Luca Fancellu -
Marek Marczykowski-Górecki