You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2017/01/25 01:30:07 UTC
[Impala-ASF-CR] IMPALA-4820: avoid writing unencrypted data during write cancellation
Tim Armstrong has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5788
Change subject: IMPALA-4820: avoid writing unencrypted data during write cancellation
......................................................................
IMPALA-4820: avoid writing unencrypted data during write cancellation
The bug was that unencrypted data could be written to disk if
the write was cancelled before it completed.
The fix is to only start decrypting data after the write is
complete.
Testing:
Added a regression test that reproduced the problem (after adding a
delay to the write).
Change-Id: I956bae0685e863f30be23634b29aa076394db184
---
M be/src/common/global-flags.cc
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
4 files changed, 72 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/5788/1
--
To view, visit http://gerrit.cloudera.org:8080/5788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I956bae0685e863f30be23634b29aa076394db184
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4820: avoid writing unencrypted data during write cancellation
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-4820: avoid writing unencrypted data during write cancellation
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5788/1//COMMIT_MSG
Commit Message:
Since this is a security issue, maybe worth noting when this was introduced here.
http://gerrit.cloudera.org:8080/#/c/5788/1/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
Line 94: // Stress option for testing failed memory allocation. Debug builds only.
This comment seems weird, since it looks like it applies to the whole #ifndef block, but its really just referring to the first one.
Maybe update it to "Stress options for debug builds only", or even just remove it.
--
To view, visit http://gerrit.cloudera.org:8080/5788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I956bae0685e863f30be23634b29aa076394db184
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4820: avoid writing unencrypted data during write cancellation
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has submitted this change and it was merged.
Change subject: IMPALA-4820: avoid writing unencrypted data during write cancellation
......................................................................
IMPALA-4820: avoid writing unencrypted data during write cancellation
The bug was that unencrypted data could be written to disk if
the write was cancelled before it completed. This bug was introduced
after Impala 2.8.0 was branched in the commit "IMPALA-3202,IMPALA-2079:
rework scratch file I/O", so does not appear in any released versions
of Impala.
The fix is to only start decrypting data after the write is
complete.
Testing:
Added a regression test that reproduced the problem (after adding a
delay to the write).
Change-Id: I956bae0685e863f30be23634b29aa076394db184
Reviewed-on: http://gerrit.cloudera.org:8080/5788
Tested-by: Impala Public Jenkins
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
M be/src/common/global-flags.cc
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
4 files changed, 73 insertions(+), 5 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/5788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I956bae0685e863f30be23634b29aa076394db184
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4820: avoid writing unencrypted data during write cancellation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4820: avoid writing unencrypted data during write cancellation
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/5788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I956bae0685e863f30be23634b29aa076394db184
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4820: avoid writing unencrypted data during write cancellation
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has uploaded a new patch set (#2).
Change subject: IMPALA-4820: avoid writing unencrypted data during write cancellation
......................................................................
IMPALA-4820: avoid writing unencrypted data during write cancellation
The bug was that unencrypted data could be written to disk if
the write was cancelled before it completed. This bug was introduced
after Impala 2.8.0 was branched in the commit "IMPALA-3202,IMPALA-2079:
rework scratch file I/O", so does not appear in any released versions
of Impala.
The fix is to only start decrypting data after the write is
complete.
Testing:
Added a regression test that reproduced the problem (after adding a
delay to the write).
Change-Id: I956bae0685e863f30be23634b29aa076394db184
---
M be/src/common/global-flags.cc
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
4 files changed, 73 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/5788/2
--
To view, visit http://gerrit.cloudera.org:8080/5788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I956bae0685e863f30be23634b29aa076394db184
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-4820: avoid writing unencrypted data during write cancellation
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4820: avoid writing unencrypted data during write cancellation
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I956bae0685e863f30be23634b29aa076394db184
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4820: avoid writing unencrypted data during write cancellation
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4820: avoid writing unencrypted data during write cancellation
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5788/1//COMMIT_MSG
Commit Message:
> Since this is a security issue, maybe worth noting when this was introduced
Done
http://gerrit.cloudera.org:8080/#/c/5788/1/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
Line 94: // Stress option for testing failed memory allocation. Debug builds only.
> This comment seems weird, since it looks like it applies to the whole #ifnd
Done
--
To view, visit http://gerrit.cloudera.org:8080/5788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I956bae0685e863f30be23634b29aa076394db184
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4820: avoid writing unencrypted data during write cancellation
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4820: avoid writing unencrypted data during write cancellation
......................................................................
Patch Set 3: Code-Review+2
Carry +2
--
To view, visit http://gerrit.cloudera.org:8080/5788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I956bae0685e863f30be23634b29aa076394db184
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4820: avoid writing unencrypted data during write cancellation
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.
Change subject: IMPALA-4820: avoid writing unencrypted data during write cancellation
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/5788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I956bae0685e863f30be23634b29aa076394db184
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4820: avoid writing unencrypted data during write cancellation
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-4820: avoid writing unencrypted data during write cancellation
......................................................................
Patch Set 3:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/221/
--
To view, visit http://gerrit.cloudera.org:8080/5788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I956bae0685e863f30be23634b29aa076394db184
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No