mm/mmap.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/mm/mmap.c b/mm/mmap.c
index 42cd2c260898..22010e13f1a1 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2625,14 +2625,14 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
if (error)
goto unmap_and_free_vma;
- /* Can addr have changed??
- *
- * Answer: Yes, several device drivers can do it in their
- * f_op->mmap method. -DaveM
+ /*
+ * Expansion is handled above, merging is handled below.
+ * Drivers should not alter the address of the VMA.
*/
- WARN_ON_ONCE(addr != vma->vm_start);
-
- addr = vma->vm_start;
+ if (WARN_ON((addr != vma->vm_start))) {
+ error = -EINVAL;
+ goto close_and_free_vma;
+ }
If this is something that user space can trigger, WARN_* is the wrong
choice. But what I understand from the comment change is that this must not
happen at that point unless there is a real issue.
The VMA start address could be changed in call_mmap() which is a driver
call. I guess someone could write a driver to mmap by a users action?
I don't think it can be reached other ways. In any case, I'm changing a
WARN_ON_ONCE() to a WARN_ON() and undoing the badness instead of
marching forwards.
WARN_ON_ONCE() can also be used in conditionals if that's what you were
concerned about, but ...
Why not "if (WARN_ON_ONCE)" ?
I was thinking it was harder to ignore if it happen more frequently?
There isn't a driver that does this now, but I'm not picky over which
variant to call.
.. I think the assumption really is that we won't see (m)any these
calls. And if we do, it's a bad bad driver. So WARN_ON() might be just fine.
If this would be easy to trigger by any user space, WARN* would have
been the wrong choice, that's why I was asking.
--
Thanks,
David / dhildenb
