sandbox: Migrate unsafe buffers to UNSAFE_TODO [chromium/src : main]

0 views

Skip to first unread message

Arthur Sonzogni (Gerrit)

unread,

Apr 10, 2026, 10:05:02 AM (3 days ago) Apr 10

to Andrew Rayskiy, Simon Hangl, James Maclean, Chromium Metrics Reviews, Peter Beverloo, Menard, Alexis, Mirko Bonadei, Jerome Jiang, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, derinel+wat...@google.com, aixba+wat...@chromium.org, ios-r...@chromium.org, blink-revie...@chromium.org, mek+w...@chromium.org, rmcelra...@chromium.org, webap...@microsoft.com, dibyapal+wa...@chromium.org, network-ser...@chromium.org, marq+...@chromium.org, webauthn...@chromium.org, creis...@chromium.org, asvitkine...@chromium.org, alexmo...@chromium.org, lizeb+watch...@chromium.org, blink-work...@chromium.org, browser-comp...@chromium.org, japhet+...@chromium.org, zelin+watch-we...@chromium.org, dmurph+watc...@chromium.org, apavlo...@chromium.org, kinuko...@chromium.org, eic+...@google.com, loyso...@chromium.org, philli...@chromium.org, blink-...@chromium.org, kuragin+web-ap...@chromium.org, ios-revie...@chromium.org, mgiuca...@chromium.org, cbe-cep-eng...@google.com, navigation...@chromium.org, blink-re...@chromium.org, feature-me...@chromium.org, cblume...@chromium.org, jz...@chromium.org, devtools...@chromium.org, chrome-intelligence-te...@google.com, mar...@chromium.org, chrome-intell...@chromium.org, net-r...@chromium.org, penghuan...@chromium.org, fgal...@chromium.org, mac-r...@chromium.org, mpdento...@chromium.org, rsesek...@chromium.org, wfh+...@chromium.org

Arthur Sonzogni added 10 comments

Patchset-level comments

File-level comment, Patchset 13:

Arthur Sonzogni . resolved

Sorry for the noise ;-(

Commit Message

Line 16, Patchset 6:

Arthur Sonzogni . resolved

gemini: You are missing many files. here there are:
arthursonzogni@arthursonzogni:/usr/local/google/btrfs_mount/7726244/src/sandbox$ git grep UNSAFE_BUFFERS_BUILD
linux/seccomp-bpf-helpers/baseline_policy_unittest.cc:#ifdef UNSAFE_BUFFERS_BUILD
linux/suid/common/suid_unsafe_environment_variables.h:#ifdef UNSAFE_BUFFERS_BUILD
linux/suid/sandbox.c:#ifdef UNSAFE_BUFFERS_BUILD
mac/sandbox_logging.cc:#ifdef UNSAFE_BUFFERS_BUILD
mac/seatbelt_exec.cc:#ifdef UNSAFE_BUFFERS_BUILD
policy/win/hook_util/hook_util.cc:#ifdef UNSAFE_BUFFERS_BUILD
win/src/lpc_policy_test.cc:#ifdef UNSAFE_BUFFERS_BUILD
win/src/sandbox_nt_util.cc:#ifdef UNSAFE_BUFFERS_BUILD
win/src/sandbox_nt_util_unittest.cc:#ifdef UNSAFE_BUFFERS_BUILD
win/src/sandbox_policy_base.cc:#ifdef UNSAFE_BUFFERS_BUILD
win/src/target_process.cc:#ifdef UNSAFE_BUFFERS_BUILD

For C file (.c extension or .h depending on usage), you keep the UNSAFE_BUFFERS_BUILD block but add this marker:

```
#ifdef UNSAFE_BUFFERS_BUILD
// PragmaAllocUnsafeBufferExclusion: C file can't be converted yet.
// TODO(crbug.com/351564777): Remove this and convert code to safer constructs.
#pragma allow_unsafe_buffers
#endif
```

Arthur Sonzogni

Done

File sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc

Line 85, Patchset 14 (Latest): const size_t kTestTransferSize = 4;

Arthur Sonzogni . unresolved

gemini: remove this. You can use kTestString.size() instead.

Line 94, Patchset 5:

  BPF_ASSERT_EQ(0,
                UNSAFE_TODO(memcmp(kTestString, read_buf, kTestTransferSize)));

Arthur Sonzogni . resolved

gemini: follow-up fix the UNSAFE_TODO, by turning kTestString and read_buff into span and comparing the span prefixes

Arthur Sonzogni

Done

File sandbox/linux/suid/common/suid_unsafe_environment_variables.h

Line 81, Patchset 14 (Latest):

  UNSAFE_TODO(memcpy(saved_envvar, "SANDBOX_", 8));
  UNSAFE_TODO(memcpy(saved_envvar + 8, envvar, envvar_len));
  UNSAFE_TODO(saved_envvar[8 + envvar_len] = 0);

Arthur Sonzogni . unresolved

gemini: Revert this.

Line 11, Patchset 14 (Latest):

#ifdef __cplusplus
#include "base/compiler_specific.h"
#else
#define UNSAFE_TODO(...)                     \
  _Pragma("clang unsafe_buffer_usage begin") \
      __VA_ARGS__ _Pragma("clang unsafe_buffer_usage end")
#endif

Arthur Sonzogni . unresolved

gemini: Revert this.

Line 24, Patchset 5:

#ifndef UNSAFE_TODO
#if defined(__clang__)
#define UNSAFE_TODO(...)                     \
  _Pragma("clang unsafe_buffer_usage begin") \
      __VA_ARGS__ _Pragma("clang unsafe_buffer_usage end")
#else
#define UNSAFE_TODO(...) __VA_ARGS__
#endif
#endif

Arthur Sonzogni . resolved

gemini: please include the compiler_specific.h file defining this here. If you need to update the BUILD.gn file, you can.

Arthur Sonzogni

Done

File sandbox/mac/sandbox_logging.cc

Line 5, Patchset 14 (Latest):// PragmaAllowUnsafeBufferException: DEPS violation for base/

Arthur Sonzogni . unresolved

gemini: PragmaAllowUnsafeBufferException should be **inside** the UNSAFE_BUFFERS_BUILD block

File sandbox/mac/seatbelt_exec.cc

Line 5, Patchset 14 (Latest):// PragmaAllowUnsafeBufferException: DEPS violation for base/

Arthur Sonzogni . unresolved

gemini: PragmaAllowUnsafeBufferException should be **inside** the UNSAFE_BUFFERS_BUILD block

File sandbox/win/src/lpc_policy_test.cc

Line 14, Patchset 14 (Latest):#include "base/compiler_specific.h"

Arthur Sonzogni . unresolved

gemini: No need for this include if we don't add UNSAFE_TODO.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Arthur Sonzogni (Gerrit)

unread,

5:48 AM (4 hours ago) 5:48 AM

to Dirk Schulze, Stephen Chenney, Jiewei Qian, Hu, Ningxin, (Julie)Jeongeun Kim, Andrew Rayskiy, Simon Hangl, James Maclean, Chromium Metrics Reviews, Peter Beverloo, Menard, Alexis, Mirko Bonadei, Jerome Jiang, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, blink-reviews-p...@chromium.org, fserb...@chromium.org, drott+bl...@chromium.org, extension...@chromium.org, chromium-a...@chromium.org, fmalit...@chromium.org, kyungjunle...@google.com, josiah...@chromium.org, accessibility-a...@google.com, francisjp...@google.com, nektar...@chromium.org, dtseng...@chromium.org, abigailbk...@google.com, yuzo+...@chromium.org, blink-revi...@chromium.org, zol...@webkit.org, oshima...@chromium.org, derinel+wat...@google.com, aixba+wat...@chromium.org, ios-r...@chromium.org, blink-revie...@chromium.org, mek+w...@chromium.org, rmcelra...@chromium.org, webap...@microsoft.com, dibyapal+wa...@chromium.org, network-ser...@chromium.org, marq+...@chromium.org, webauthn...@chromium.org, creis...@chromium.org, asvitkine...@chromium.org, alexmo...@chromium.org, lizeb+watch...@chromium.org, blink-work...@chromium.org, browser-comp...@chromium.org, japhet+...@chromium.org, zelin+watch-we...@chromium.org, dmurph+watc...@chromium.org, apavlo...@chromium.org, kinuko...@chromium.org, eic+...@google.com, loyso...@chromium.org, philli...@chromium.org, blink-...@chromium.org, kuragin+web-ap...@chromium.org, ios-revie...@chromium.org, mgiuca...@chromium.org, cbe-cep-eng...@google.com, navigation...@chromium.org, blink-re...@chromium.org, feature-me...@chromium.org, cblume...@chromium.org, jz...@chromium.org, devtools...@chromium.org, chrome-intelligence-te...@google.com, mar...@chromium.org, chrome-intell...@chromium.org, net-r...@chromium.org, penghuan...@chromium.org, fgal...@chromium.org, mac-r...@chromium.org, mpdento...@chromium.org, rsesek...@chromium.org, wfh+...@chromium.org

Arthur Sonzogni abandoned this change.

View Change

Abandoned

Arthur Sonzogni abandoned this change

Related details

Attention set is empty

Submit Requirements:

Code-Coverage

Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages