[PATCH 0/2][SRU][D/E] CVE-2019-15794: ovl/shiftfs refcount underflow

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2][SRU][D/E] CVE-2019-15794: ovl/shiftfs refcount underflow

Seth Forshee
BugLink: https://bugs.launchpad.net/bugs/1850994

Impact: overlayfs and shiftfs both replace vma->vm_file in their mmap
handlers. On error the original value is not restored, and the reference
is put for the file to which vm_file points. On upstream kernels this is
not an issue, as no callers dereference vm_file dereference vm_file
following after call_mmap() returns an error. However, the aufs patchs
change mmap_region() to replace the fput() using a local variable with
vma_fput(), which will fput() vm_file, leading to a refcount underflow.

Fix: Restore the original vma_file value on error.

Test Case: A reproducer is provided in the original bug report.

Regression Potential: Minimal. As stated above, other callers of
call_mmap() do not dereference vma->vm_file when it returns an error,
and the one which does is fixed by these patches.

Notes: Supported kernels prior to disco are not affected as overlayfs
did not support mmap until 4.19, and shiftfs was not present in Ubuntu
kernels before disco. The issue is mitigated for overlayfs by another
bug which is preventing unprivileged mounting; a patch for this issue
will be sent separately.

Thanks,
Seth


Seth Forshee (2):
  UBUNTU: SAUCE: shiftfs: Restore vm_file value when lower fs mmap fails
  UBUNTU: SAUCE: ovl: Restore vm_file value when lower fs mmap fails

 fs/overlayfs/file.c |  6 +++++-
 fs/shiftfs.c        | 15 +++++++++++----
 2 files changed, 16 insertions(+), 5 deletions(-)


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2][SRU][D/E] UBUNTU: SAUCE: shiftfs: Restore vm_file value when lower fs mmap fails

Seth Forshee
BugLink: https://bugs.launchpad.net/bugs/1850994

shiftfs_mmap() overwrites vma->vm_file before calling the lower
filesystem mmap but does not restore the original value on
failure. This means it is giving a pointer to the lower fs file
back to the caller with no reference, which is a bad practice.
However, it does not lead to any issues with upstream kernels as
no caller accesses vma->vm_file after call_mmap().

With the aufs patches applied the story is different. Whereas
mmap_region() previously fput a local variable containing the
file it assigned to vm_file, it now calls vma_fput() which will
fput vm_file, for which it has no reference, and the reference
for the original vm_file is not put.

Fix this by restoring vma->vm_file to the original value when the
mmap call into the lower fs fails.

CVE-2019-15794

Reported-by: Jann Horn <[hidden email]>
Signed-off-by: Seth Forshee <[hidden email]>
---
 fs/shiftfs.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/shiftfs.c b/fs/shiftfs.c
index 55bb32b611f2..57d84479026b 100644
--- a/fs/shiftfs.c
+++ b/fs/shiftfs.c
@@ -1289,10 +1289,17 @@ static int shiftfs_mmap(struct file *file, struct vm_area_struct *vma)
 
  shiftfs_file_accessed(file);
 
- if (ret)
- fput(realfile); /* Drop refcount from new vm_file value */
- else
- fput(file); /* Drop refcount from previous vm_file value */
+ if (ret) {
+ /*
+ * Drop refcount from new vm_file value and restore original
+ * vm_file value
+ */
+ vma->vm_file = file;
+ fput(realfile);
+ } else {
+ /* Drop refcount from previous vm_file value */
+ fput(file);
+ }
 
  return ret;
 }
--
2.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2][SRU][D/E] UBUNTU: SAUCE: ovl: Restore vm_file value when lower fs mmap fails

Seth Forshee
In reply to this post by Seth Forshee
BugLink: https://bugs.launchpad.net/bugs/1850994

ovl_mmap() overwrites vma->vm_file before calling the lower
filesystem mmap but does not restore the original value on
failure. This means it is giving a pointer to the lower fs file
back to the caller with no reference, which is a bad practice.
However, it does not lead to any issues with upstream kernels as
no caller accesses vma->vm_file after call_mmap().

With the aufs patches applied the story is different. Whereas
mmap_region() previously fput a local variable containing the
file it assigned to vm_file, it now calls vma_fput() which will
fput vm_file, for which it has no reference, and the reference
for the original vm_file is not put.

Fix this by restoring vma->vm_file to the original value when the
mmap call into the lower fs fails.

CVE-2019-15794

Reported-by: Jann Horn <[hidden email]>
Signed-off-by: Seth Forshee <[hidden email]>
---
 fs/overlayfs/file.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 895f2c5565d3..43ad47cc046f 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -334,7 +334,11 @@ static int ovl_mmap(struct file *file, struct vm_area_struct *vma)
  revert_creds(old_cred);
 
  if (ret) {
- /* Drop reference count from new vm_file value */
+ /*
+ * Drop reference count from new vm_file value and restore
+ * original vm_file value
+ */
+ vma->vm_file = file;
  fput(realfile);
  } else {
  /* Drop reference count from previous vm_file value */
--
2.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 0/2][SRU][D/E] CVE-2019-15794: ovl/shiftfs refcount underflow

Tyler Hicks-2
In reply to this post by Seth Forshee
On 2019-11-07 10:08:23, Seth Forshee wrote:

> BugLink: https://bugs.launchpad.net/bugs/1850994
>
> Impact: overlayfs and shiftfs both replace vma->vm_file in their mmap
> handlers. On error the original value is not restored, and the reference
> is put for the file to which vm_file points. On upstream kernels this is
> not an issue, as no callers dereference vm_file dereference vm_file
> following after call_mmap() returns an error. However, the aufs patchs
> change mmap_region() to replace the fput() using a local variable with
> vma_fput(), which will fput() vm_file, leading to a refcount underflow.
>
> Fix: Restore the original vma_file value on error.
>
> Test Case: A reproducer is provided in the original bug report.
>
> Regression Potential: Minimal. As stated above, other callers of
> call_mmap() do not dereference vma->vm_file when it returns an error,
> and the one which does is fixed by these patches.
>
> Notes: Supported kernels prior to disco are not affected as overlayfs
> did not support mmap until 4.19, and shiftfs was not present in Ubuntu
> kernels before disco. The issue is mitigated for overlayfs by another
> bug which is preventing unprivileged mounting; a patch for this issue
> will be sent separately.

Both patches look good.

 Acked-by: Tyler Hicks <[hidden email]>

Thanks!

Tyler

>
> Thanks,
> Seth
>
>
> Seth Forshee (2):
>   UBUNTU: SAUCE: shiftfs: Restore vm_file value when lower fs mmap fails
>   UBUNTU: SAUCE: ovl: Restore vm_file value when lower fs mmap fails
>
>  fs/overlayfs/file.c |  6 +++++-
>  fs/shiftfs.c        | 15 +++++++++++----
>  2 files changed, 16 insertions(+), 5 deletions(-)
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 0/2][SRU][D/E] CVE-2019-15794: ovl/shiftfs refcount underflow

Stefan Bader-2
In reply to this post by Seth Forshee
On 07.11.19 17:08, Seth Forshee wrote:

> BugLink: https://bugs.launchpad.net/bugs/1850994
>
> Impact: overlayfs and shiftfs both replace vma->vm_file in their mmap
> handlers. On error the original value is not restored, and the reference
> is put for the file to which vm_file points. On upstream kernels this is
> not an issue, as no callers dereference vm_file dereference vm_file
> following after call_mmap() returns an error. However, the aufs patchs
> change mmap_region() to replace the fput() using a local variable with
> vma_fput(), which will fput() vm_file, leading to a refcount underflow.
>
> Fix: Restore the original vma_file value on error.
>
> Test Case: A reproducer is provided in the original bug report.
>
> Regression Potential: Minimal. As stated above, other callers of
> call_mmap() do not dereference vma->vm_file when it returns an error,
> and the one which does is fixed by these patches.
>
> Notes: Supported kernels prior to disco are not affected as overlayfs
> did not support mmap until 4.19, and shiftfs was not present in Ubuntu
> kernels before disco. The issue is mitigated for overlayfs by another
> bug which is preventing unprivileged mounting; a patch for this issue
> will be sent separately.
>
> Thanks,
> Seth
>
>
> Seth Forshee (2):
>   UBUNTU: SAUCE: shiftfs: Restore vm_file value when lower fs mmap fails
>   UBUNTU: SAUCE: ovl: Restore vm_file value when lower fs mmap fails
>
>  fs/overlayfs/file.c |  6 +++++-
>  fs/shiftfs.c        | 15 +++++++++++----
>  2 files changed, 16 insertions(+), 5 deletions(-)
>
>
Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED: [PATCH 0/2][SRU][D/E] CVE-2019-15794: ovl/shiftfs refcount underflow

Khaled Elmously
In reply to this post by Seth Forshee
On 2019-11-07 10:08:23 , Seth Forshee wrote:

> BugLink: https://bugs.launchpad.net/bugs/1850994
>
> Impact: overlayfs and shiftfs both replace vma->vm_file in their mmap
> handlers. On error the original value is not restored, and the reference
> is put for the file to which vm_file points. On upstream kernels this is
> not an issue, as no callers dereference vm_file dereference vm_file
> following after call_mmap() returns an error. However, the aufs patchs
> change mmap_region() to replace the fput() using a local variable with
> vma_fput(), which will fput() vm_file, leading to a refcount underflow.
>
> Fix: Restore the original vma_file value on error.
>
> Test Case: A reproducer is provided in the original bug report.
>
> Regression Potential: Minimal. As stated above, other callers of
> call_mmap() do not dereference vma->vm_file when it returns an error,
> and the one which does is fixed by these patches.
>
> Notes: Supported kernels prior to disco are not affected as overlayfs
> did not support mmap until 4.19, and shiftfs was not present in Ubuntu
> kernels before disco. The issue is mitigated for overlayfs by another
> bug which is preventing unprivileged mounting; a patch for this issue
> will be sent separately.
>
> Thanks,
> Seth
>
>
> Seth Forshee (2):
>   UBUNTU: SAUCE: shiftfs: Restore vm_file value when lower fs mmap fails
>   UBUNTU: SAUCE: ovl: Restore vm_file value when lower fs mmap fails
>
>  fs/overlayfs/file.c |  6 +++++-
>  fs/shiftfs.c        | 15 +++++++++++----
>  2 files changed, 16 insertions(+), 5 deletions(-)
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team