Skip to content

Commit

Permalink
[M116] wallpaper: safer saving policy/custom wallpaper
Browse files Browse the repository at this point in the history
There is a race condition when first logging in between
reading and saving wallpaper policy image. If the stars align,
the login sequence that reads policy-controlled.jpeg will
get a partial read while the sequence that refreshes policy
wallpaper is re-saving it.

Mitigate this by writing wallpaper data to a tmp file and then
moving it to the desired location.

(cherry picked from commit 35d9156)

Bug: b:280578317
Test: ash_unittests --gtest_filter=*WallpaperController*
Change-Id: If78ba85e0ebb7cb8bfab99f90a9deaa3a19da579
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4628564
Commit-Queue: Jeffrey Young <cowmoo@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1160809}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4651446
Cr-Commit-Position: refs/branch-heads/5845@{#196}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
  • Loading branch information
cwmoo740 authored and Chromium LUCI CQ committed Jun 28, 2023
1 parent 442fa70 commit 24f2df6
Showing 1 changed file with 25 additions and 2 deletions.
27 changes: 25 additions & 2 deletions ash/wallpaper/wallpaper_utils/wallpaper_file_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/logging.h"
#include "base/memory/ref_counted_memory.h"
#include "base/memory/scoped_refptr.h"
#include "third_party/skia/include/core/SkBitmap.h"
Expand Down Expand Up @@ -77,6 +78,7 @@ bool ResizeAndSaveWallpaper(const gfx::ImageSkia& image,
WallpaperLayout layout,
int preferred_width,
int preferred_height) {
DVLOG(3) << __func__ << " path=" << path;
if (layout == WALLPAPER_LAYOUT_CENTER) {
if (base::PathExists(path)) {
base::DeleteFile(path);
Expand All @@ -89,8 +91,29 @@ bool ResizeAndSaveWallpaper(const gfx::ImageSkia& image,
return false;
}

// Saves |data| to |path| in local file system.
return base::WriteFile(path, base::make_span(data->front(), data->size()));
// Write to `temp_path` to reduce the chance of read/write
// collisions from different sequences. This avoids issues with policy
// wallpaper at login: b/280578317.
base::FilePath temp_path;
if (!base::CreateTemporaryFileInDir(path.DirName(), &temp_path)) {
LOG(WARNING) << "Failed to create temporary file";
return false;
}

if (!base::WriteFile(temp_path,
base::make_span(data->front(), data->size()))) {
LOG(WARNING) << "Failed to write wallpaper data to temporary file";
base::DeleteFile(temp_path);
return false;
}

if (!base::Move(temp_path, path)) {
LOG(WARNING) << "Failed to copy temporary wallpaper data to " << path;
base::DeleteFile(temp_path);
return false;
}

return true;
}

} // namespace ash

0 comments on commit 24f2df6

Please sign in to comment.