[T/X][CVE-2017-15127] CVE-2017-15127 (false-positive?)

[Stefan Bader]

On 16.05.2018 06:40, Khalid Elmously wrote:
> CVE-2017-15127
> 
> I think this may be a false-positive.

Agreeing with the false positive. But this is not fixed by applying empty
patches but by fixing the triaging hints.

If you look at the breaks-fix links, you notice that the breaks link is pointing
to the first commit in git. The web view actually is a translation from the cve
tracker statement:

Patches_linux:
 break-fix: - 5af10dfd0afc559bb4b0f7e3e8227a1578333995

which means whoever triaged this did not really know what introduced the issue
and left a wildcard there. I think you were on the right track about the
functionality but I think below is the one introducing the problem:

commit 1c9e8def43a3452e7af658b340f5f4f4ecde5c38
Author: Mike Kravetz <mike.kravetz at oracle.com>
Date:   Wed Feb 22 15:43:43 2017 -0800

    userfaultfd: hugetlbfs: add UFFDIO_COPY support for shared mappings

This one adds the additional unlock, after the nounlock label. So the right
thing to do is to update the tracker with this sha1 as the introducing commit.

-Stefan

> 
> The CVE is marked as needed in trusty and xenial in the CVE matrix, but it seems to me that this doesn't actually affect trusty or xenial. This vulnerability appears to be related to the implementation of a feature called "UFFDIO_COPY operation for huge pages" - something which doesn't appear to exist in trusty or xenial. The fix for the CVE is to modify a function called hugetlb_mcopy_atomic_pte() (which gets called during the UFFDIO_COPY operation on a hugetlb vma) by changing the location of the goto label called 'out_release_nounlock' so as to avoid a call to unlock_page() if add_to_page_cache() returns with error - yet that 'out_release_nounlock' label doesn't exist in trusty or xenial's HEAD as it doesn't have the "UFFDIO_COPY for huge pages" feature at all (as implemented in 60d4d2d2b40e44cd36bfb6049e8d9e2055a24f8a). A function exists in current trusty/xenial called hugetlb_no_page() which shares some code structure with the vulnerable hugetlb_mcopy_atomic_pte() function, but it doesn't seem to suffer from the double unlock_page() problem. In the current trusty/xenial code, in hugetlb_no_page(), if huge_add_to_page_cache() returns an error, a call to put_page() is made and the error returned to the calling function, which is exactly what the new code does as well in both places where huge_add_to_page_cache() is called. I.e., no extraneous unlock_page() happens.
> 
> Based on the above, I believe trusty and xenial are not affected by this CVE.
> 
> (As I'm effectively proposing an empty patch as the 'fix' for this CVE, I thought I'd post my analysis to the mailing list and request 2 ACKs)
> 
> Thanks
> Khalid
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20180523/527b8987/attachment.sig>