Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(200)

Issue 1690983004: Extended restricted filesystem to support relative paths. (Closed)

Created:
4 years, 10 months ago by Sean Klein
Modified:
4 years, 10 months ago
Reviewers:
Mark Seaborn, Sam Clegg
CC:
native-client-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/native_client/src/native_client.git@master
Target Ref:
refs/heads/master
Project:
nacl
Visibility:
Public.

Description

Extended restricted filesystem to support relative paths. Usage of ".." is still disallowed. BUG=https://code.google.com/p/nativeclient/issues/detail?id=4152 TESTS=limited_file_access_test Committed: https://chromium.googlesource.com/native_client/src/native_client/+/9b1c913840204c4794106c2a6db0068e5be5377a

Patch Set 1 #

Patch Set 2 : #

Total comments: 4

Patch Set 3 : Using strcat instead of memcpy #

Total comments: 4

Patch Set 4 : Allow relative paths without prepending CWD #

Total comments: 8

Patch Set 5 : Strcpy, Testing cwd #

Total comments: 2

Patch Set 6 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+75 lines, -55 lines) Patch
M documentation/filesystem_access.txt View 1 2 3 4 2 chunks +4 lines, -5 lines 0 comments Download
M src/trusted/service_runtime/sel_ldr_filename.c View 1 2 3 4 4 chunks +37 lines, -35 lines 0 comments Download
M src/trusted/service_runtime/sel_main.c View 1 2 3 4 5 1 chunk +3 lines, -2 lines 0 comments Download
M tests/limited_file_access/limited_file_access.cc View 1 2 3 4 5 5 chunks +31 lines, -13 lines 0 comments Download

Messages

Total messages: 22 (6 generated)
Sean Klein
4 years, 10 months ago (2016-02-11 22:31:02 UTC) #2
Sam Clegg
Thanks for doing this. I think I can live without '..' for not but it ...
4 years, 10 months ago (2016-02-11 22:45:26 UTC) #3
Sean Klein
Adding Mark. I can look into adding ".." support in an upcoming CL. https://codereview.chromium.org/1690983004/diff/20001/src/trusted/service_runtime/sel_ldr_filename.c File ...
4 years, 10 months ago (2016-02-11 23:00:52 UTC) #5
Sam Clegg
On 2016/02/11 23:00:52, smklein1 wrote: > Adding Mark. > > I can look into adding ...
4 years, 10 months ago (2016-02-11 23:04:05 UTC) #6
Sean Klein
On 2016/02/11 23:04:05, Sam Clegg wrote: > On 2016/02/11 23:00:52, smklein1 wrote: > > Adding ...
4 years, 10 months ago (2016-02-12 01:17:08 UTC) #7
Mark Seaborn
https://codereview.chromium.org/1690983004/diff/40001/src/trusted/service_runtime/sel_ldr_filename.c File src/trusted/service_runtime/sel_ldr_filename.c (right): https://codereview.chromium.org/1690983004/diff/40001/src/trusted/service_runtime/sel_ldr_filename.c#newcode69 src/trusted/service_runtime/sel_ldr_filename.c:69: /* Absolute Path = Cwd + '/' + Relative ...
4 years, 10 months ago (2016-02-17 22:13:23 UTC) #9
Sean Klein
https://codereview.chromium.org/1690983004/diff/40001/src/trusted/service_runtime/sel_ldr_filename.c File src/trusted/service_runtime/sel_ldr_filename.c (right): https://codereview.chromium.org/1690983004/diff/40001/src/trusted/service_runtime/sel_ldr_filename.c#newcode69 src/trusted/service_runtime/sel_ldr_filename.c:69: /* Absolute Path = Cwd + '/' + Relative ...
4 years, 10 months ago (2016-02-17 22:42:39 UTC) #10
Mark Seaborn
https://codereview.chromium.org/1690983004/diff/40001/src/trusted/service_runtime/sel_ldr_filename.c File src/trusted/service_runtime/sel_ldr_filename.c (right): https://codereview.chromium.org/1690983004/diff/40001/src/trusted/service_runtime/sel_ldr_filename.c#newcode69 src/trusted/service_runtime/sel_ldr_filename.c:69: /* Absolute Path = Cwd + '/' + Relative ...
4 years, 10 months ago (2016-02-18 19:32:02 UTC) #11
Sean Klein
https://codereview.chromium.org/1690983004/diff/40001/src/trusted/service_runtime/sel_ldr_filename.c File src/trusted/service_runtime/sel_ldr_filename.c (right): https://codereview.chromium.org/1690983004/diff/40001/src/trusted/service_runtime/sel_ldr_filename.c#newcode69 src/trusted/service_runtime/sel_ldr_filename.c:69: /* Absolute Path = Cwd + '/' + Relative ...
4 years, 10 months ago (2016-02-19 18:49:06 UTC) #12
Sean Klein
Mark, could you take a look? I've simplified the relative path case.
4 years, 10 months ago (2016-02-23 23:21:25 UTC) #13
Mark Seaborn
https://codereview.chromium.org/1690983004/diff/60001/documentation/filesystem_access.txt File documentation/filesystem_access.txt (right): https://codereview.chromium.org/1690983004/diff/60001/documentation/filesystem_access.txt#newcode70 documentation/filesystem_access.txt:70: Requires that the cwd is within the mounted directory ...
4 years, 10 months ago (2016-02-24 21:19:19 UTC) #14
Sean Klein
https://codereview.chromium.org/1690983004/diff/60001/documentation/filesystem_access.txt File documentation/filesystem_access.txt (right): https://codereview.chromium.org/1690983004/diff/60001/documentation/filesystem_access.txt#newcode70 documentation/filesystem_access.txt:70: Requires that the cwd is within the mounted directory ...
4 years, 10 months ago (2016-02-24 23:40:43 UTC) #15
Mark Seaborn
LGTM https://codereview.chromium.org/1690983004/diff/60001/tests/limited_file_access/limited_file_access.cc File tests/limited_file_access/limited_file_access.cc (right): https://codereview.chromium.org/1690983004/diff/60001/tests/limited_file_access/limited_file_access.cc#newcode6 tests/limited_file_access/limited_file_access.cc:6: On 2016/02/24 23:40:43, Sean Klein wrote: > Updated ...
4 years, 10 months ago (2016-02-25 01:18:09 UTC) #16
Sean Klein
https://codereview.chromium.org/1690983004/diff/60001/tests/limited_file_access/limited_file_access.cc File tests/limited_file_access/limited_file_access.cc (right): https://codereview.chromium.org/1690983004/diff/60001/tests/limited_file_access/limited_file_access.cc#newcode6 tests/limited_file_access/limited_file_access.cc:6: On 2016/02/25 01:18:09, Mark Seaborn wrote: > On 2016/02/24 ...
4 years, 10 months ago (2016-02-25 01:36:28 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1690983004/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1690983004/100001
4 years, 10 months ago (2016-02-25 17:28:39 UTC) #20
commit-bot: I haz the power
4 years, 10 months ago (2016-02-25 17:29:44 UTC) #22
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/native_client/src/native_client/+/9b1c91384...

Powered by Google App Engine
This is Rietveld 408576698