New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 825177 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task

Blocking:
issue 795291



Sign in to add a comment

Implement platform-specific permission checks for shared memory regions

Project Member Reported by alexilin@chromium.org, Mar 23 2018

Issue description

Shared memory handles can be sent from an unprivileged process to a privileged one. It's important to check that the permission mode announced by a sender corresponds to the real one.
 
Blocking: 795291

Comment 2 by w...@chromium.org, Mar 27 2018

What is the risk to the privileged process of being passed a writable handle rather than a read-only one?

Aren't we mainly concerned about privileged processes incorrectly passing writable handles to unprivilged processes, when they should be read-only?
The privileged process can pass this handle further to another unprivileged process without noticing it has an incorrect type.

You're right about what's the main concern, the new shared memory API should force correct use of handle permission modes. It might be useful to perform the check before passing a handle as well.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5e98a98965d102f96de6235998bb33415d8bfca

commit b5e98a98965d102f96de6235998bb33415d8bfca
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Wed Mar 28 07:25:06 2018

PlatformSharedMemoryRegion permission checks on POSIX and Android

This CL implements the CheckPlatformHandlePermissionsCorrespondToMode() function
on POSIX and Android platforms.

On POSIX we get a descriptor access mode via fcntl(fd, F_GETFL) system call.

On Android we use ashmem_get_prot_region to get an ashmem region protection
mask.

Bug:  825177 
Change-Id: Ic6688995292b54dbf70f337023eb9f1e3219686a
Reviewed-on: https://chromium-review.googlesource.com/980560
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546430}
[modify] https://crrev.com/b5e98a98965d102f96de6235998bb33415d8bfca/base/memory/platform_shared_memory_region.cc
[modify] https://crrev.com/b5e98a98965d102f96de6235998bb33415d8bfca/base/memory/platform_shared_memory_region_android.cc
[modify] https://crrev.com/b5e98a98965d102f96de6235998bb33415d8bfca/base/memory/platform_shared_memory_region_fuchsia.cc
[modify] https://crrev.com/b5e98a98965d102f96de6235998bb33415d8bfca/base/memory/platform_shared_memory_region_mac.cc
[modify] https://crrev.com/b5e98a98965d102f96de6235998bb33415d8bfca/base/memory/platform_shared_memory_region_posix.cc
[modify] https://crrev.com/b5e98a98965d102f96de6235998bb33415d8bfca/base/memory/platform_shared_memory_region_unittest.cc
[modify] https://crrev.com/b5e98a98965d102f96de6235998bb33415d8bfca/base/memory/platform_shared_memory_region_win.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6deb7ef904c99c2dda3ad59bad3975bba6d10312

commit 6deb7ef904c99c2dda3ad59bad3975bba6d10312
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Wed Apr 11 08:17:57 2018

PlatformSharedMemoryRegion permission checks on Windows

This CL implements the CheckPlatformHandlePermissionsCorrespondToMode()
function on Windows.

There is no documented API to get the current access mask on a handle.
To understand whether a handle has write access, we check the success
status of the ::DuplicateHandle(dwDesiredAccess=FILE_MAP_WRITE) call.

Bug:  825177 
Change-Id: I6863e9ead390972601539c23f6e12674ee3b7736
Reviewed-on: https://chromium-review.googlesource.com/1005343
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549823}
[modify] https://crrev.com/6deb7ef904c99c2dda3ad59bad3975bba6d10312/base/memory/platform_shared_memory_region_win.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0b3ee5fa39838d60293792a9faa7bcf1955c582e

commit 0b3ee5fa39838d60293792a9faa7bcf1955c582e
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Wed Apr 18 23:52:24 2018

PlatformSharedMemoryRegion permission checks on Fuchsia

This CL implements the CheckPlatformHandlePermissionsCorrespondToMode()
function on Fuchsia.

Bug:  825177 
Change-Id: Ie14206f57464d6864a94ada82e87ee7f3db16346
Reviewed-on: https://chromium-review.googlesource.com/1012037
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551884}
[modify] https://crrev.com/0b3ee5fa39838d60293792a9faa7bcf1955c582e/base/memory/platform_shared_memory_region_fuchsia.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc89217b9886981ee1c21f0feb9f4f42d0feb579

commit bc89217b9886981ee1c21f0feb9f4f42d0feb579
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Thu Apr 19 18:44:36 2018

PlatformSharedMemoryRegion permission checks on Mac

This CL implements the CheckPlatformHandlePermissionsCorrespondToMode()
function on Mac.

This also enables extra testing of this function since it's implemented
on all supported platforms.

Bug:  825177 
Change-Id: I6982d62618a33b2c67091651c09c9b0d3a23326a
Reviewed-on: https://chromium-review.googlesource.com/1012075
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552097}
[modify] https://crrev.com/bc89217b9886981ee1c21f0feb9f4f42d0feb579/base/memory/platform_shared_memory_region_mac.cc
[modify] https://crrev.com/bc89217b9886981ee1c21f0feb9f4f42d0feb579/base/memory/platform_shared_memory_region_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, May 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b97525a24fc06440088d25fbc2b4c3539575e8be

commit b97525a24fc06440088d25fbc2b4c3539575e8be
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Tue May 29 17:25:53 2018

PlatformSharedMemoryRegion permission checks on NaCl

NaCl platform does not implement fcntl(_, F_GETFL) function that is used for
performing platform handle permissions check on POSIX.

This CL adds an alternative implementation of this check specifically for NaCl.

Bug:  825177 
Change-Id: Ifb779fc892f1e63a77dc07ff4fd83c2c7b0dd4f1
Reviewed-on: https://chromium-review.googlesource.com/1076467
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562484}
[modify] https://crrev.com/b97525a24fc06440088d25fbc2b4c3539575e8be/base/memory/platform_shared_memory_region_posix.cc

Sign in to add a comment