Skip to content

fix: prevent SIGBUS crash when reading images from external volumes #1387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
olegsatov wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: prevent SIGBUS crash when reading images from external volumes #1387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src-tauri/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion src-tauri/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ tempfile = "3.27.0"
mimalloc = "0.1.52"
image_hasher = "3.1.1"
regex = "1.12.4"
memmap2 = "0.9.11"
half = { version = "2.7.1", features = ["bytemuck"] }
glam = "0.33.2"
quick-xml = { version = "0.41", features = ["serialize"] }
Expand Down
52 changes: 15 additions & 37 deletions src-tauri/src/export_processing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use tauri::Manager;
use crate::AppState;
use crate::exif_processing;
use crate::file_management::{
generate_filename_from_template, parse_virtual_path, read_file_mapped,
generate_filename_from_template, parse_virtual_path, read_file_bytes,
};
use crate::formats::is_raw_file;
use crate::image_loader::{
Expand Down Expand Up @@ -915,30 +915,17 @@ pub async fn export_images(
}
}
} else {
match read_file_mapped(Path::new(&source_path_str)) {
Ok(mmap) => load_and_composite(
&mmap,
&source_path_str,
&js_adjustments,
false,
&settings,
None,
)
.map_err(|e| format!("Failed to load from mmap: {}", e))?,
Err(_) => {
let bytes =
fs::read(&source_path_str).map_err(|e| e.to_string())?;
load_and_composite(
&bytes,
&source_path_str,
&js_adjustments,
false,
&settings,
None,
)
.map_err(|e| format!("Failed to load from bytes: {}", e))?
}
}
let bytes = read_file_bytes(Path::new(&source_path_str))
.map_err(|e| e.to_string())?;
load_and_composite(
&bytes,
&source_path_str,
&js_adjustments,
false,
&settings,
None,
)
.map_err(|e| format!("Failed to load image: {}", e))?
};

let mut main_export_adjustments = js_adjustments.clone();
Expand Down Expand Up @@ -1212,18 +1199,9 @@ pub async fn estimate_export_sizes(

const ESTIMATE_DIM: u32 = 1280;

let file_slice: Vec<u8>;
let mmap_guard;
let file_data: &[u8] = match read_file_mapped(Path::new(&source_path_str)) {
Ok(mmap) => {
mmap_guard = Some(mmap);
mmap_guard.as_ref().unwrap()
}
Err(_) => {
file_slice = fs::read(&source_path_str).map_err(|io_err| io_err.to_string())?;
&file_slice
}
};
let file_bytes =
read_file_bytes(Path::new(&source_path_str)).map_err(|e| e.to_string())?;
let file_data: &[u8] = &file_bytes;

let original_image =
load_base_image_from_bytes(file_data, &source_path_str, true, &settings, None)
Expand Down
84 changes: 18 additions & 66 deletions src-tauri/src/file_management.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use memmap2::{Mmap, MmapOptions};
use std::borrow::Cow;
use std::collections::hash_map::DefaultHasher;
use std::collections::{HashMap, HashSet};
Expand Down Expand Up @@ -233,7 +232,6 @@ pub struct PresetFile {
#[derive(Debug)]
pub enum ReadFileError {
Io(std::io::Error),
Locked,
Empty,
NotFound,
Invalid,
Expand All @@ -243,7 +241,6 @@ impl fmt::Display for ReadFileError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ReadFileError::Io(err) => write!(f, "IO error: {}", err),
ReadFileError::Locked => write!(f, "File is locked"),
ReadFileError::Empty => write!(f, "File is empty"),
ReadFileError::NotFound => write!(f, "File not found"),
ReadFileError::Invalid => write!(f, "Invalid file"),
Expand Down Expand Up @@ -321,9 +318,7 @@ pub async fn read_exif_for_paths(
sidecar_exif
} else if is_cloud_placeholder(&source_path) {
HashMap::new()
} else if let Ok(mmap) = read_file_mapped(&source_path) {
crate::exif_processing::read_exif_data(&source_path_str, &mmap)
} else if let Ok(bytes) = fs::read(&source_path) {
} else if let Ok(bytes) = read_file_bytes(&source_path) {
crate::exif_processing::read_exif_data(&source_path_str, &bytes)
} else {
HashMap::new()
Expand Down Expand Up @@ -357,9 +352,7 @@ pub async fn update_exif_fields(
let mut exif_data = temp_metadata.exif.unwrap_or_else(|| {
if let Some(existing) = crate::exif_processing::read_rrexif_sidecar(original_path) {
existing
} else if let Ok(mmap) = read_file_mapped(original_path) {
crate::exif_processing::read_exif_data_from_bytes(path, &mmap)
} else if let Ok(bytes) = fs::read(original_path) {
} else if let Ok(bytes) = read_file_bytes(original_path) {
crate::exif_processing::read_exif_data_from_bytes(path, &bytes)
} else {
HashMap::new()
Expand Down Expand Up @@ -1178,7 +1171,7 @@ pub fn is_cloud_placeholder(_path: &Path) -> bool {
false
}

pub fn read_file_mapped(path: &Path) -> Result<Mmap, ReadFileError> {
pub fn read_file_bytes(path: &Path) -> Result<Vec<u8>, ReadFileError> {
if !path.is_file() {
return Err(ReadFileError::Invalid);
}
Expand All @@ -1188,17 +1181,7 @@ pub fn read_file_mapped(path: &Path) -> Result<Mmap, ReadFileError> {
if path.metadata().map_err(ReadFileError::Io)?.len() == 0 {
return Err(ReadFileError::Empty);
}
let file = fs::File::open(path).map_err(ReadFileError::Io)?;
if file.try_lock_shared().is_err() {
return Err(ReadFileError::Locked);
}
let mmap = unsafe {
MmapOptions::new()
.len(file.metadata().map_err(ReadFileError::Io)?.len() as usize)
.map(&file)
.map_err(ReadFileError::Io)?
};
Ok(mmap)
fs::read(path).map_err(ReadFileError::Io)
}

pub fn generate_thumbnail_data(
Expand Down Expand Up @@ -1274,29 +1257,10 @@ pub fn generate_thumbnail_data(
let composite_image = if let Some(img) = preloaded_image {
image_loader::composite_patches_on_image(img, &adjustments)?
} else {
let mmap_guard;
let vec_guard;

let file_slice: &[u8] = match read_file_mapped(&source_path) {
Ok(mmap) => {
mmap_guard = Some(mmap);
mmap_guard.as_ref().unwrap()
}
Err(e) => {
if preloaded_image.is_none() {
log::warn!("Fallback read for {}: {}", source_path_str, e);
}
let bytes = fs::read(&source_path).map_err(|io_err| {
anyhow::anyhow!(
"Fallback read failed for {}: {}",
source_path_str,
io_err
)
})?;
vec_guard = Some(bytes);
vec_guard.as_ref().unwrap()
}
};
let file_data = read_file_bytes(&source_path).map_err(|e| {
anyhow::anyhow!("Failed to read {}: {}", source_path_str, e)
})?;
let file_slice: &[u8] = &file_data;

let img = image_loader::load_and_composite(
file_slice,
Expand Down Expand Up @@ -1463,28 +1427,16 @@ pub fn generate_thumbnail_data(
let mut final_image = if let Some(img) = preloaded_image {
image_loader::composite_patches_on_image(img, &adjustments)?
} else {
match read_file_mapped(&source_path) {
Ok(mmap) => image_loader::load_and_composite(
&mmap,
&source_path_str,
&adjustments,
true,
&settings,
None,
)?,
Err(e) => {
log::warn!("Fallback read for {}: {}", source_path_str, e);
let bytes = fs::read(&source_path)?;
image_loader::load_and_composite(
&bytes,
&source_path_str,
&adjustments,
true,
&settings,
None,
)?
}
}
let bytes = read_file_bytes(&source_path)
.map_err(|e| anyhow::anyhow!("Failed to read {}: {}", source_path_str, e))?;
image_loader::load_and_composite(
&bytes,
&source_path_str,
&adjustments,
true,
&settings,
None,
)?
};

if adjustments.is_null() {
Expand Down
62 changes: 18 additions & 44 deletions src-tauri/src/image_loader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::Cursor;
use crate::app_settings::{AppSettings, load_settings};
use crate::app_state::{AppState, LoadedImage};
use crate::exif_processing;
use crate::file_management::{parse_virtual_path, read_file_mapped};
use crate::file_management::{parse_virtual_path, read_file_bytes};
use crate::formats::is_raw_file;
use crate::image_processing::ImageMetadata;
use crate::image_processing::{
Expand All @@ -18,7 +18,6 @@ use rayon::prelude::*;
use serde::Deserialize;
use serde_json::Value;
use std::collections::HashMap;
use std::fs;
use std::panic;
use std::path::Path;
use std::sync::OnceLock;
Expand Down Expand Up @@ -800,50 +799,25 @@ pub async fn load_image(
return Err("Load cancelled".to_string());
}

let result: Result<(DynamicImage, HashMap<String, String>), String> =
(|| match read_file_mapped(Path::new(&path_clone)) {
Ok(mmap) => {
if generation_tracker.load(Ordering::SeqCst) != my_generation {
return Err("Load cancelled".to_string());
}
let result: Result<(DynamicImage, HashMap<String, String>), String> = (|| {
let bytes = read_file_bytes(Path::new(&path_clone))
.map_err(|e| format!("Failed to read {}: {}", path_clone, e))?;

let img = load_base_image_from_bytes(
&mmap,
&path_clone,
false,
&settings,
cancel_token.clone(),
)
.map_err(|e| e.to_string())?;
let exif = exif_processing::read_exif_data(&path_clone, &mmap);
Ok((img, exif))
}
Err(e) => {
log::warn!(
"Failed to memory-map file '{}': {}. Falling back to standard read.",
path_clone,
e
);
let bytes = fs::read(&path_clone).map_err(|io_err| {
format!("Fallback read failed for {}: {}", path_clone, io_err)
})?;

if generation_tracker.load(Ordering::SeqCst) != my_generation {
return Err("Load cancelled".to_string());
}
if generation_tracker.load(Ordering::SeqCst) != my_generation {
return Err("Load cancelled".to_string());
}

let img = load_base_image_from_bytes(
&bytes,
&path_clone,
false,
&settings,
cancel_token.clone(),
)
.map_err(|e| e.to_string())?;
let exif = exif_processing::read_exif_data(&path_clone, &bytes);
Ok((img, exif))
}
})();
let img = load_base_image_from_bytes(
&bytes,
&path_clone,
false,
&settings,
cancel_token.clone(),
)
.map_err(|e| e.to_string())?;
let exif = exif_processing::read_exif_data(&path_clone, &bytes);
Ok((img, exif))
})();
result
})
.await
Expand Down
40 changes: 11 additions & 29 deletions src-tauri/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ use crate::cache_utils::{
DecodedImageCache, GEOMETRY_KEYS, calculate_full_job_hash, calculate_geometry_hash,
calculate_transform_hash, calculate_visual_hash,
};
use crate::file_management::{parse_virtual_path, read_file_mapped};
use crate::file_management::{parse_virtual_path, read_file_bytes};
use crate::formats::is_raw_file;
use crate::hdr_deghosting::{align_hdr_frames, assert_uniform_dimensions, load_hdr_frames};
use crate::image_loader::{composite_patches_on_image, load_and_composite};
Expand Down Expand Up @@ -1522,34 +1522,16 @@ fn generate_preview_for_path(
let is_raw = is_raw_file(&source_path_str);
let settings = load_settings(app_handle.clone()).unwrap_or_default();

let base_image = match read_file_mapped(&source_path) {
Ok(mmap) => load_and_composite(
&mmap,
&source_path_str,
&js_adjustments,
false,
&settings,
None,
)
.map_err(|e| e.to_string())?,
Err(e) => {
log::warn!(
"Failed to memory-map file '{}': {}. Falling back to standard read.",
source_path_str,
e
);
let bytes = fs::read(&source_path).map_err(|io_err| io_err.to_string())?;
load_and_composite(
&bytes,
&source_path_str,
&js_adjustments,
false,
&settings,
None,
)
.map_err(|e| e.to_string())?
}
};
let bytes = read_file_bytes(&source_path).map_err(|e| e.to_string())?;
let base_image = load_and_composite(
&bytes,
&source_path_str,
&js_adjustments,
false,
&settings,
None,
)
.map_err(|e| e.to_string())?;

let (transformed_image, unscaled_crop_offset) =
apply_all_transformations(Cow::Borrowed(&base_image), &js_adjustments);
Expand Down
Loading