| Message ID | YNIz5NVnZ5GiZ3u1@mwanda (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/rmap: fix signedness bug in make_device_exclusive_range() | expand |
On Tue, Jun 22, 2021 at 10:03:00PM +0300, Dan Carpenter wrote: > The get_user_pages_remote() function returns a long type, but we are > using "unsigned long i;" as the list iterator. If "npages" is -ENOMEM, > the comparison "i < npages" is type promoted and "npages" becomes a very > high positive value. The loop will then iterate until the kernel > crashes. > > There are two ways to fix this. Declare "i" as a long type or add an > explicit check for get_user_pages_remote() error returns. Either > approach will work so let's do both. > > Fixes: fa1e686e5f53 ("mm: device exclusive memory access") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > mm/rmap.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/mm/rmap.c b/mm/rmap.c > index e5210dde0c4d..fb5c59b95826 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -2187,11 +2187,14 @@ int make_device_exclusive_range(struct mm_struct *mm, unsigned long start, > void *owner) > { > long npages = (end - start) >> PAGE_SHIFT; > - unsigned long i; > + long i; > > npages = get_user_pages_remote(mm, start, npages, > FOLL_GET | FOLL_WRITE | FOLL_SPLIT_PMD, > pages, NULL, NULL); > + if (npages < 0) > + return npages; > + > for (i = 0; i < npages; i++, start += PAGE_SIZE) { > if (!trylock_page(pages[i])) { > put_page(pages[i]); > -- > 2.30.2 Ouch.. The check should be enough, imho; looping over an long seems a tiny little bit odd, but still looks okay. Reviewed-by: Peter Xu <peterx@redhat.com> Thanks,
diff --git a/mm/rmap.c b/mm/rmap.c index e5210dde0c4d..fb5c59b95826 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -2187,11 +2187,14 @@ int make_device_exclusive_range(struct mm_struct *mm, unsigned long start, void *owner) { long npages = (end - start) >> PAGE_SHIFT; - unsigned long i; + long i; npages = get_user_pages_remote(mm, start, npages, FOLL_GET | FOLL_WRITE | FOLL_SPLIT_PMD, pages, NULL, NULL); + if (npages < 0) + return npages; + for (i = 0; i < npages; i++, start += PAGE_SIZE) { if (!trylock_page(pages[i])) { put_page(pages[i]);
The get_user_pages_remote() function returns a long type, but we are using "unsigned long i;" as the list iterator. If "npages" is -ENOMEM, the comparison "i < npages" is type promoted and "npages" becomes a very high positive value. The loop will then iterate until the kernel crashes. There are two ways to fix this. Declare "i" as a long type or add an explicit check for get_user_pages_remote() error returns. Either approach will work so let's do both. Fixes: fa1e686e5f53 ("mm: device exclusive memory access") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- mm/rmap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)