Olli, can you take a look and maybe report it to NVidia driver team?

I can also repro with driver 390.30 (GTX1070)

This is a serious regression. Raising to P1.

Submitter, I suggest you recommend to your users to downgrade to an earlier driver version until this is diagnosed and ideally fixed on NVIDIA's side. I would not want to have to figure out a driver bug workaround for this.

For what is worth, Firefox works fine in the same environment.

I have been able to reproduce the issue on the Linux driver version 390.25 inside Chrome.

The failure seems to be related to GPU process sandboxing - if I use the --disable-gpu-sandbox flag, then all of the readPixels related tests pass. Obviously disabling the sandbox in regular usage is a bad idea. Ken or Mo, is there a summary somewhere about how the GPU process sandboxing is implemented on Linux? It might be possible to address this on the driver side, or some part of the config might need to be changed on the side of Chromium. Assigning back to you. I'll see if I can find out what change in the driver affected this.

Oh, got some more information already: We've identified a change that caused this. It's related to changes in how some allocations are handled in the driver. It's possible that Chromium needs to allow more system calls in the GPU sandbox. Me or someone else from our side will provide more details as they become available.

Thanks Olli for confirming. Let me assign this to you and give it back to me when you have more information about any sandbox changes that are needed. It would be ideal if NVIDIA could add Linux to the testing configuration for beta drivers instead of just Windows with the OpenGL backend, in the spirit of testing what we ship.

Allowing stat() system call could fix this for 390.25 drivers - still need to get confirmation for this. Can you give me a pointer to where the GPU sandbox settings are in Chromium code, so I could also experiment with the settings myself?

We're looking into getting rid of the disallowed system calls in the driver.

The linux sandbox code is under src/sandbox/linux/, but I don't know the details of how it works.

palmer@, could you help Olli out here?

Oh dear, I'm not sure we want to allow `stat` from within the sandbox. Let me confer with my crew and I'll get back to you.

What file(s) does the driver `stat`, and why? Knowing that will help us resolve this one way or another.

palmer: it will likely be a driver bug workaround for a specific NVidia driver version, not in general. Still, I fully understand the security concern.

But can you at least point to Olli where he can locally modify the sandbox to allow `stat`? So he could confirm if that's truly the issue.

#12: In https://cs.chromium.org/chromium/src/services/service_manager/sandbox/linux/bpf_gpu_policy_linux.cc?type=cs&ssfr=1&sq=package:chromium&l=40, you should be able to add a case for `__NR_stat`.

The GPU policy inherits from the baseline policy, so this might also work: In seccomp-bpf-helpers/baseline_policy.cc, in IsBaselinePolicyAllowed, add SyscallSets::IsFileSystem(sysno) || somewhere in there.

The stat() call is used on /dev/nvidiactl - it's basically just checking whether the kernel driver is loaded. GPU process already allows read/write access to that file, so it seems like the only thing missing in Chromium is handling stat() calls similarly to other file access calls in GpuProcessPolicy::EvaluateSysCall.

I can try to verify this on my end, but this is probably enough information for you to fix this in Chromium - Mo, could you take this?

tsepez notes that we already broker `stat` for Chrome OS. The brokered call is subject to our path restrictions; see
https://cs.chromium.org/chromium/src/content/gpu/gpu_sandbox_hook_linux.cc?rcl=c2f61690f7a59a4c6ac7cc4b99dd92b44e810f23&l=260.

That might be the way to go here, too.

palmer: are you saying we just broker `stat` on Linux also, always?

If yes, I can prepare a simple CL.

Yeah, I think you can just do that. Then, on the browser side, require that the pathname be the specific one mentioned in #14 (dev/nvidiactl), and refuse all others. I'd be happy to review the CL, or another security person can.

tsepez: where in the codebase do I limit the paths `stat` can access?

Thanks for looking into this.

From a quick test, it looks like in addition to /dev/nvidiactl, we also stat() /dev/nvidia[0-9]+, /dev/nvidia-modeset (for display-related stuff) and $HOME/.nv and its subdirectories for the shader cache. In some cases we apparently will also stat() /dev/zero, although it doesn't look like that should be needed for Chrome.

That's probably not an exhaustive list, so I'd recommend `strace`ing with the sandbox disabled to see what else the driver is trying to do.

Currently we grant read/write for
  /dev/nvidiactl
  /dev/nvidia[0-9]+
and read-only for
  /proc/driver/nvidia/params

It's been working for a long time until recently. Olli, can you provide a complete list of files that NVidia driver would need? My driver doesn't reproduce this and it seems you can reproduce on your side.

Security team, will it be OK for us to expand our sandbox broker to allow more files?

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

commit 6b1b6d3a8555075e23cca89335e855d55f35fba9
Author: Zhenyao Mo <zmo@chromium.org>
Date: Thu Mar 29 23:48:19 2018

Allow `stat` in Linux for GPU process for a list of files.

This is to unblock certain NVidia driver's glReadPixels calls in the sandboxed
GPU process.

Note that the needed file /dev/nvidiactl is already in the list for read/write.

BUG= 817400 
TEST=manual
R=tsepez@chromium.org

Change-Id: I9074a8335a9c4df1487f5a288d5e284bbedf67c3
Reviewed-on: https://chromium-review.googlesource.com/965462
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547027}
[modify] https://crrev.com/6b1b6d3a8555075e23cca89335e855d55f35fba9/content/gpu/gpu_sandbox_hook_linux.cc
[modify] https://crrev.com/6b1b6d3a8555075e23cca89335e855d55f35fba9/services/service_manager/sandbox/linux/bpf_gpu_policy_linux.cc

This was regressed in M64 and we're not planning any further M65 releases. Hence, rejecting merge to M65.

This bug requires manual review: M66 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Tested the issue using #67.0.3386.0 on Linux Debian Rodete using NVIDIA driver version #384.111and could not reproduce the issue as per the steps mentioned below.

Steps:
1. Launched Browser
2. Navigated to https://www.khronos.org/registry/webgl/sdk/tests/conformance/more/functions/readPixels.html?webglVersion=1&quiet=0
3. Observed The "testReadPixelsRGBA" test Passed i.e, Issue is not seen

Note: Issue is not seen in reported version #64.0.3282.186 as well.

@enowak: Requesting you to check the issue on latest Canary and update the behavior?

Thanks!!

Please confirm if canary looks good with the change. 

Just a heads up, M66 Stable cut is on April 12th, 10 days away. This issue is marked as RB-Stable for 66. Please make sure to address this issue prior to stable cut. Thanks! 

Olli, can you help confirming the CL fixes the bug if original reporter doesn't confirm in time?

By the way, we don't have Canary on Linux, but you can download a chromium build that includes my fix for testing purpose, for example:

https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Linux_x64/547566/

This bug only affect NVIDIA driver version 390+. Drivers prior to it are not affected (384 & 387). 

Tested here using 67.0.3381.0 on OpenSUSE Tumbleweed using NVIDIA driver version 390.42 and testReadPixelsRGBA stills fails.

If you go to about:version, you will get something like

Revision	8eda42889581706bf2d49d6b5426ac186519661f-refs/heads/master@{#547349}

The # number is the last Chrome commit for this revision.

If that number is smaller than #547027, then it doesn't include my fix.

#31: can you confirm this?

I can confirm it, with the build you just posted (67.0.3387.0) the bug is fixed. 

OpenSUSE Tumbleweed using NVIDIA driver version 390.42

Hello,

I can confirm this has been solved for me and two other Arch Linux users [1]. We are still on Chromium 65.0.3325.181, but the maintainers of the chromium package included the patch in that version [2]. Thank you!

[1] https://bbs.archlinux.org/viewtopic.php?pid=1777262#p1777262
[2] https://git.archlinux.org/svntogit/packages.git/commit/trunk?h=packages/chromium&id=6b8aedc2012275151c30e1f908fa0ab53e2beefd

Thanks a bunch Mo for getting the patch in! I just got back from Easter break and I'm seeing the same results as the others - the patch has fixed the readPixels issue.

Is anything more required to get this merged to 66?

Adding Verified labels as per comment #33, 34, 35.

Thanks!!

Approving merge for M66. Branch:3359

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

commit f62fe2e89a1326f4cc08eb729b50eaa4d7f098e4
Author: Zhenyao Mo <zmo@chromium.org>
Date: Wed Apr 04 17:05:45 2018

Allow `stat` in Linux for GPU process for a list of files.

This is to unblock certain NVidia driver's glReadPixels calls in the sandboxed
GPU process.

Note that the needed file /dev/nvidiactl is already in the list for read/write.

BUG= 817400 
TEST=manual
R=tsepez@chromium.org
TBR=zmo@chromium.org

(cherry picked from commit 6b1b6d3a8555075e23cca89335e855d55f35fba9)

Change-Id: I9074a8335a9c4df1487f5a288d5e284bbedf67c3
Reviewed-on: https://chromium-review.googlesource.com/965462
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#547027}
Reviewed-on: https://chromium-review.googlesource.com/996027
Cr-Commit-Position: refs/branch-heads/3359@{#576}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/f62fe2e89a1326f4cc08eb729b50eaa4d7f098e4/content/gpu/gpu_sandbox_hook_linux.cc
[modify] https://crrev.com/f62fe2e89a1326f4cc08eb729b50eaa4d7f098e4/services/service_manager/sandbox/linux/bpf_gpu_policy_linux.cc

adding information in case it's useful for triaging other real world issues related to this:
GTX 960, Gentoo ~amd64, nVidia driver 390.48, chromium 66.0.3359.66 .
On https://web.whatsapp.com/ couldn't get the QR code to show - it was blinking for a few millisecs then disappearing sometime leaving a strange pattern behind (variable. See attachment).
As a first attempt, I've disabled #disable-accelerated-2d-canvas via chrome://flags. That fixed the issue. Then I started to dig in bugs.chromium.org, found this bug, re-enabled the experiment, reproduced the issue, started with --disable-gpu-sandbox, and it worked (with the experiment enabled). Re-adding sandbox again, same as before. So I assume it's related to this.
Also previews of web sites in the new tab are corrupted since long (possibly since M64, but I didn't care enough to investigate). Forcing regeneration of thumbnails while executing with --disable-gpu-sandbox produces good thumbnails, while regenerating with the sandbox generated corrupted ones again.

Deleted: Screenshot_20180405_001003.png 137 KB

	Screenshot_20180405_001003.png 137 KB View Download

Since TE doesn't have a linux machine with specified GEForce drivers to verify the fix, hence requesting zmo@ to verify this issue on #66.0.3359.106 and update the observations.

Thank You...

Let's make a beta candidate downloadable and maybe Olli can download and verify it?

I'm happy to try the beta out if needed.

Issue 802209 has been merged into this issue.

The symptom of this bug which I had (couldn't not record tabs in a Chrome screen recorder extension) no longer appears in the 106 beta (Ubuntu 17.10). Thanks. 

tim@, thank you so much for the confirmation!

I also tried out the beta, WebGL conformance tests for readPixels that were failing with Chrome 65 are now passing and performance is also better. Looks good!

 Issue 827578  has been merged into this issue.

 Issue 833984  has been merged into this issue.

Updated to 66.0.3359.117  today. I can confirm the khronos test is passing as well as onshape is able to select elements in the UI.

Thank you all.