Message ID | 20190806210452.14708-1-connor.kuehl@canonical.com |
---|---|
State | New |
Headers | show |
Series | [Pull,SRU,CVE-2019-11487,Xenial] Avoid overflowing page reference count | expand |
On 2019-08-06 14:04:52, Connor Kuehl wrote: > https://people.canonical.com/~ubuntu-security/cve/2019/CVE-2019-11487.html > > From the link above: > > "The Linux kernel before 5.1-rc5 allows page->_refcount reference count > overflow, with resultant use-after-free issues, if about 140 GiB of RAM > exists. This is related to fs/fuse/dev.c, fs/pipe.c, fs/splice.c, > include/linux/mm.h, include/linux/pipe_fs_i.h, kernel/trace/trace.c, > mm/gup.c, and mm/hugetlb.c. It can occur with FUSE requests." > > Bionic has already received these patches during an upstream sync: > https://bugs.launchpad.net/bugs/1838459 > > Xenial felt like a bit of an involved backport that touched the mm subsystem, > and because of that, I think it is possible there's a breakage risk here that > I don't see as I'm not very familiar with mm. This all looks good to me. The only thing that I noticed was a spaces vs tab issue on the line that calls atomic_inc() in "mm: add 'try_get_page()' helper function". That's not a blocker, IMO, but maybe you could fix it up before it is pulled. Acked-by: Tyler Hicks <tyhicks@canonical.com> Tyler > > It builds on all arches and I boot-tested amd64. > > * mm: prevent get_user_pages() from overflowing page refcount > > This is the patch I'm most concerned about. It appears to be based on a > refactoring patch which split a function into a number of others. Rather > than try to backport the refactoring patch(es). I've backported the code > to what I think is equivalent to what the refactored one does. > > In my backport, I didn't see an equivalent error path for this hunk from > the original patch, so it was omitted: > > diff --git a/mm/gup.c b/mm/gup.c > index 75029649baca..81e0bdefa2cc 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -295,7 +299,10 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma, > if (pmd_trans_unstable(pmd)) > ret = -EBUSY; > } else { > - get_page(page); > + if (unlikely(!try_get_page(page))) { > + spin_unlock(ptl); > + return ERR_PTR(-ENOMEM); > + } > spin_unlock(ptl); > lock_page(page); > ret = split_huge_page(page); > > * mm, gup: ensure real head page is ref-counted when using hugepages > > This patch wasn't named as part of the CVE fix itself, but bringing this one in > made it easier to apply the last fix "mm: prevent get_user_pages() from overflowing page refcount" > > * mm: make page ref count overflow check tighter and more explicit > > Minor offset adjustments, and used atomic_read instead of a wrapper function > that is introduced in a later commit (that commit makes a large number of changes > so I felt it was better to just use the mechanism that it ultimately uses). > > * mm: add 'try_get_page()' helper function > > Offset adjustment, directly use atomic_read and atomic_inc rather than the > page_ref wrappers that aren't introduced until a later and larger commit. > > * fs: prevent page refcount overflow in pipe_buf_get > > Offset adjustment and manually change the function signature for buffer_pipe_buf_get > > * pipe: add pipe_buf_get() helper > > Clean cherry pick. Needed for "fs: prevent page refcount overflow in pipe_buf_get" > > ---------------------------------------------------------------- > The following changes since commit 1ef87ecb69472da81f394f8229ec3e100b306252: > > Linux 4.4.186 (2019-08-05 18:19:52 +0200) > > are available in the Git repository at: > > git://git.launchpad.net/~connork/+git/xenial CVE-2019-11487 > > for you to fetch changes up to 0af916186a039ec8029b12f64023f2786ab8fa6c: > > mm: prevent get_user_pages() from overflowing page refcount (2019-08-06 13:29:12 -0700) > > ---------------------------------------------------------------- > Linus Torvalds (3): > mm: add 'try_get_page()' helper function > mm: make page ref count overflow check tighter and more explicit > mm: prevent get_user_pages() from overflowing page refcount > > Matthew Wilcox (1): > fs: prevent page refcount overflow in pipe_buf_get > > Miklos Szeredi (1): > pipe: add pipe_buf_get() helper > > Punit Agrawal (1): > mm, gup: ensure real head page is ref-counted when using hugepages > > fs/fuse/dev.c | 12 ++++++------ > fs/pipe.c | 4 ++-- > fs/splice.c | 12 ++++++++++-- > include/linux/mm.h | 15 +++++++++++++- > include/linux/pipe_fs_i.h | 17 ++++++++++++++-- > kernel/trace/trace.c | 6 +++++- > mm/gup.c | 50 ++++++++++++++++++++++++++++++++++------------- > mm/hugetlb.c | 16 ++++++++++++++- > 8 files changed, 103 insertions(+), 29 deletions(-) > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 06.08.19 23:04, Connor Kuehl wrote: > https://people.canonical.com/~ubuntu-security/cve/2019/CVE-2019-11487.html When I fetch this feels like the sequence might be wrong too... <HEAD> 0af916186a03 mm: prevent get_user_pages() from overflowing page refcount e05ca882af4a mm, gup: ensure real head page is ref-counted when using hugepages 8fc77f9e88d3 mm: make page ref count overflow check tighter and more explicit 4fdb44fce583 mm: add 'try_get_page()' helper function ^ this seems to add the try_get_page helper after below already added use of it e5e2ab21b1d8 fs: prevent page refcount overflow in pipe_buf_get 9efe377b196d pipe: add pipe_buf_get() helper <older> -Stefan > > From the link above: > > "The Linux kernel before 5.1-rc5 allows page->_refcount reference count > overflow, with resultant use-after-free issues, if about 140 GiB of RAM > exists. This is related to fs/fuse/dev.c, fs/pipe.c, fs/splice.c, > include/linux/mm.h, include/linux/pipe_fs_i.h, kernel/trace/trace.c, > mm/gup.c, and mm/hugetlb.c. It can occur with FUSE requests." > > Bionic has already received these patches during an upstream sync: > https://bugs.launchpad.net/bugs/1838459 > > Xenial felt like a bit of an involved backport that touched the mm subsystem, > and because of that, I think it is possible there's a breakage risk here that > I don't see as I'm not very familiar with mm. > > It builds on all arches and I boot-tested amd64. > > * mm: prevent get_user_pages() from overflowing page refcount > > This is the patch I'm most concerned about. It appears to be based on a > refactoring patch which split a function into a number of others. Rather > than try to backport the refactoring patch(es). I've backported the code > to what I think is equivalent to what the refactored one does. > > In my backport, I didn't see an equivalent error path for this hunk from > the original patch, so it was omitted: > > diff --git a/mm/gup.c b/mm/gup.c > index 75029649baca..81e0bdefa2cc 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -295,7 +299,10 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma, > if (pmd_trans_unstable(pmd)) > ret = -EBUSY; > } else { > - get_page(page); > + if (unlikely(!try_get_page(page))) { > + spin_unlock(ptl); > + return ERR_PTR(-ENOMEM); > + } > spin_unlock(ptl); > lock_page(page); > ret = split_huge_page(page); > > * mm, gup: ensure real head page is ref-counted when using hugepages > > This patch wasn't named as part of the CVE fix itself, but bringing this one in > made it easier to apply the last fix "mm: prevent get_user_pages() from overflowing page refcount" > > * mm: make page ref count overflow check tighter and more explicit > > Minor offset adjustments, and used atomic_read instead of a wrapper function > that is introduced in a later commit (that commit makes a large number of changes > so I felt it was better to just use the mechanism that it ultimately uses). > > * mm: add 'try_get_page()' helper function > > Offset adjustment, directly use atomic_read and atomic_inc rather than the > page_ref wrappers that aren't introduced until a later and larger commit. > > * fs: prevent page refcount overflow in pipe_buf_get > > Offset adjustment and manually change the function signature for buffer_pipe_buf_get > > * pipe: add pipe_buf_get() helper > > Clean cherry pick. Needed for "fs: prevent page refcount overflow in pipe_buf_get" > > ---------------------------------------------------------------- > The following changes since commit 1ef87ecb69472da81f394f8229ec3e100b306252: > > Linux 4.4.186 (2019-08-05 18:19:52 +0200) > > are available in the Git repository at: > > git://git.launchpad.net/~connork/+git/xenial CVE-2019-11487 > > for you to fetch changes up to 0af916186a039ec8029b12f64023f2786ab8fa6c: > > mm: prevent get_user_pages() from overflowing page refcount (2019-08-06 13:29:12 -0700) > > ---------------------------------------------------------------- > Linus Torvalds (3): > mm: add 'try_get_page()' helper function > mm: make page ref count overflow check tighter and more explicit > mm: prevent get_user_pages() from overflowing page refcount > > Matthew Wilcox (1): > fs: prevent page refcount overflow in pipe_buf_get > > Miklos Szeredi (1): > pipe: add pipe_buf_get() helper > > Punit Agrawal (1): > mm, gup: ensure real head page is ref-counted when using hugepages > > fs/fuse/dev.c | 12 ++++++------ > fs/pipe.c | 4 ++-- > fs/splice.c | 12 ++++++++++-- > include/linux/mm.h | 15 +++++++++++++- > include/linux/pipe_fs_i.h | 17 ++++++++++++++-- > kernel/trace/trace.c | 6 +++++- > mm/gup.c | 50 ++++++++++++++++++++++++++++++++++------------- > mm/hugetlb.c | 16 ++++++++++++++- > 8 files changed, 103 insertions(+), 29 deletions(-) >
On 8/13/19 4:42 AM, Stefan Bader wrote: > On 06.08.19 23:04, Connor Kuehl wrote: >> https://people.canonical.com/~ubuntu-security/cve/2019/CVE-2019-11487.html > > When I fetch this feels like the sequence might be wrong too... > > <HEAD> > 0af916186a03 mm: prevent get_user_pages() from overflowing page refcount > e05ca882af4a mm, gup: ensure real head page is ref-counted when using hugepages > 8fc77f9e88d3 mm: make page ref count overflow check tighter and more explicit > 4fdb44fce583 mm: add 'try_get_page()' helper function > > ^ this seems to add the try_get_page helper after below already added use > of it I will fix this in a V2. > > e5e2ab21b1d8 fs: prevent page refcount overflow in pipe_buf_get > 9efe377b196d pipe: add pipe_buf_get() helper > <older> > > -Stefan > >> >> From the link above: >> >> "The Linux kernel before 5.1-rc5 allows page->_refcount reference count >> overflow, with resultant use-after-free issues, if about 140 GiB of RAM >> exists. This is related to fs/fuse/dev.c, fs/pipe.c, fs/splice.c, >> include/linux/mm.h, include/linux/pipe_fs_i.h, kernel/trace/trace.c, >> mm/gup.c, and mm/hugetlb.c. It can occur with FUSE requests." >> >> Bionic has already received these patches during an upstream sync: >> https://bugs.launchpad.net/bugs/1838459 >> >> Xenial felt like a bit of an involved backport that touched the mm subsystem, >> and because of that, I think it is possible there's a breakage risk here that >> I don't see as I'm not very familiar with mm. >> >> It builds on all arches and I boot-tested amd64. >> >> * mm: prevent get_user_pages() from overflowing page refcount >> >> This is the patch I'm most concerned about. It appears to be based on a >> refactoring patch which split a function into a number of others. Rather >> than try to backport the refactoring patch(es). I've backported the code >> to what I think is equivalent to what the refactored one does. >> >> In my backport, I didn't see an equivalent error path for this hunk from >> the original patch, so it was omitted: >> >> diff --git a/mm/gup.c b/mm/gup.c >> index 75029649baca..81e0bdefa2cc 100644 >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -295,7 +299,10 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma, >> if (pmd_trans_unstable(pmd)) >> ret = -EBUSY; >> } else { >> - get_page(page); >> + if (unlikely(!try_get_page(page))) { >> + spin_unlock(ptl); >> + return ERR_PTR(-ENOMEM); >> + } >> spin_unlock(ptl); >> lock_page(page); >> ret = split_huge_page(page); >> >> * mm, gup: ensure real head page is ref-counted when using hugepages >> >> This patch wasn't named as part of the CVE fix itself, but bringing this one in >> made it easier to apply the last fix "mm: prevent get_user_pages() from overflowing page refcount" >> >> * mm: make page ref count overflow check tighter and more explicit >> >> Minor offset adjustments, and used atomic_read instead of a wrapper function >> that is introduced in a later commit (that commit makes a large number of changes >> so I felt it was better to just use the mechanism that it ultimately uses). >> >> * mm: add 'try_get_page()' helper function >> >> Offset adjustment, directly use atomic_read and atomic_inc rather than the >> page_ref wrappers that aren't introduced until a later and larger commit. >> >> * fs: prevent page refcount overflow in pipe_buf_get >> >> Offset adjustment and manually change the function signature for buffer_pipe_buf_get >> >> * pipe: add pipe_buf_get() helper >> >> Clean cherry pick. Needed for "fs: prevent page refcount overflow in pipe_buf_get" >> >> ---------------------------------------------------------------- >> The following changes since commit 1ef87ecb69472da81f394f8229ec3e100b306252: >> >> Linux 4.4.186 (2019-08-05 18:19:52 +0200) >> >> are available in the Git repository at: >> >> git://git.launchpad.net/~connork/+git/xenial CVE-2019-11487 >> >> for you to fetch changes up to 0af916186a039ec8029b12f64023f2786ab8fa6c: >> >> mm: prevent get_user_pages() from overflowing page refcount (2019-08-06 13:29:12 -0700) >> >> ---------------------------------------------------------------- >> Linus Torvalds (3): >> mm: add 'try_get_page()' helper function >> mm: make page ref count overflow check tighter and more explicit >> mm: prevent get_user_pages() from overflowing page refcount >> >> Matthew Wilcox (1): >> fs: prevent page refcount overflow in pipe_buf_get >> >> Miklos Szeredi (1): >> pipe: add pipe_buf_get() helper >> >> Punit Agrawal (1): >> mm, gup: ensure real head page is ref-counted when using hugepages >> >> fs/fuse/dev.c | 12 ++++++------ >> fs/pipe.c | 4 ++-- >> fs/splice.c | 12 ++++++++++-- >> include/linux/mm.h | 15 +++++++++++++- >> include/linux/pipe_fs_i.h | 17 ++++++++++++++-- >> kernel/trace/trace.c | 6 +++++- >> mm/gup.c | 50 ++++++++++++++++++++++++++++++++++------------- >> mm/hugetlb.c | 16 ++++++++++++++- >> 8 files changed, 103 insertions(+), 29 deletions(-) >> >
On 8/9/19 3:03 PM, Tyler Hicks wrote: > On 2019-08-06 14:04:52, Connor Kuehl wrote: >> https://people.canonical.com/~ubuntu-security/cve/2019/CVE-2019-11487.html >> >> From the link above: >> >> "The Linux kernel before 5.1-rc5 allows page->_refcount reference count >> overflow, with resultant use-after-free issues, if about 140 GiB of RAM >> exists. This is related to fs/fuse/dev.c, fs/pipe.c, fs/splice.c, >> include/linux/mm.h, include/linux/pipe_fs_i.h, kernel/trace/trace.c, >> mm/gup.c, and mm/hugetlb.c. It can occur with FUSE requests." >> >> Bionic has already received these patches during an upstream sync: >> https://bugs.launchpad.net/bugs/1838459 >> >> Xenial felt like a bit of an involved backport that touched the mm subsystem, >> and because of that, I think it is possible there's a breakage risk here that >> I don't see as I'm not very familiar with mm. > > This all looks good to me. The only thing that I noticed was a spaces vs > tab issue on the line that calls atomic_inc() in "mm: add > 'try_get_page()' helper function". That's not a blocker, IMO, but maybe > you could fix it up before it is pulled. I'll fix this in a V2. > > Acked-by: Tyler Hicks <tyhicks@canonical.com> > > Tyler > >> >> It builds on all arches and I boot-tested amd64. >> >> * mm: prevent get_user_pages() from overflowing page refcount >> >> This is the patch I'm most concerned about. It appears to be based on a >> refactoring patch which split a function into a number of others. Rather >> than try to backport the refactoring patch(es). I've backported the code >> to what I think is equivalent to what the refactored one does. >> >> In my backport, I didn't see an equivalent error path for this hunk from >> the original patch, so it was omitted: >> >> diff --git a/mm/gup.c b/mm/gup.c >> index 75029649baca..81e0bdefa2cc 100644 >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -295,7 +299,10 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma, >> if (pmd_trans_unstable(pmd)) >> ret = -EBUSY; >> } else { >> - get_page(page); >> + if (unlikely(!try_get_page(page))) { >> + spin_unlock(ptl); >> + return ERR_PTR(-ENOMEM); >> + } >> spin_unlock(ptl); >> lock_page(page); >> ret = split_huge_page(page); >> >> * mm, gup: ensure real head page is ref-counted when using hugepages >> >> This patch wasn't named as part of the CVE fix itself, but bringing this one in >> made it easier to apply the last fix "mm: prevent get_user_pages() from overflowing page refcount" >> >> * mm: make page ref count overflow check tighter and more explicit >> >> Minor offset adjustments, and used atomic_read instead of a wrapper function >> that is introduced in a later commit (that commit makes a large number of changes >> so I felt it was better to just use the mechanism that it ultimately uses). >> >> * mm: add 'try_get_page()' helper function >> >> Offset adjustment, directly use atomic_read and atomic_inc rather than the >> page_ref wrappers that aren't introduced until a later and larger commit. >> >> * fs: prevent page refcount overflow in pipe_buf_get >> >> Offset adjustment and manually change the function signature for buffer_pipe_buf_get >> >> * pipe: add pipe_buf_get() helper >> >> Clean cherry pick. Needed for "fs: prevent page refcount overflow in pipe_buf_get" >> >> ---------------------------------------------------------------- >> The following changes since commit 1ef87ecb69472da81f394f8229ec3e100b306252: >> >> Linux 4.4.186 (2019-08-05 18:19:52 +0200) >> >> are available in the Git repository at: >> >> git://git.launchpad.net/~connork/+git/xenial CVE-2019-11487 >> >> for you to fetch changes up to 0af916186a039ec8029b12f64023f2786ab8fa6c: >> >> mm: prevent get_user_pages() from overflowing page refcount (2019-08-06 13:29:12 -0700) >> >> ---------------------------------------------------------------- >> Linus Torvalds (3): >> mm: add 'try_get_page()' helper function >> mm: make page ref count overflow check tighter and more explicit >> mm: prevent get_user_pages() from overflowing page refcount >> >> Matthew Wilcox (1): >> fs: prevent page refcount overflow in pipe_buf_get >> >> Miklos Szeredi (1): >> pipe: add pipe_buf_get() helper >> >> Punit Agrawal (1): >> mm, gup: ensure real head page is ref-counted when using hugepages >> >> fs/fuse/dev.c | 12 ++++++------ >> fs/pipe.c | 4 ++-- >> fs/splice.c | 12 ++++++++++-- >> include/linux/mm.h | 15 +++++++++++++- >> include/linux/pipe_fs_i.h | 17 ++++++++++++++-- >> kernel/trace/trace.c | 6 +++++- >> mm/gup.c | 50 ++++++++++++++++++++++++++++++++++------------- >> mm/hugetlb.c | 16 ++++++++++++++- >> 8 files changed, 103 insertions(+), 29 deletions(-) >> >> -- >> kernel-team mailing list >> kernel-team@lists.ubuntu.com >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/mm/gup.c b/mm/gup.c index 75029649baca..81e0bdefa2cc 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -295,7 +299,10 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma, if (pmd_trans_unstable(pmd)) ret = -EBUSY; } else { - get_page(page); + if (unlikely(!try_get_page(page))) { + spin_unlock(ptl); + return ERR_PTR(-ENOMEM); + } spin_unlock(ptl); lock_page(page); ret = split_huge_page(page);