Skip to content

Commit

Permalink
fix: double free in putImageData
Browse files Browse the repository at this point in the history
  • Loading branch information
Brooooooklyn committed Jan 26, 2025
1 parent 01a855d commit 814262a
Show file tree
Hide file tree
Showing 12 changed files with 1,020 additions and 124 deletions.
64 changes: 1 addition & 63 deletions .github/workflows/CI.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ jobs:
sudo docker cp ubuntu-jammy-armv7:/usr/lib/llvm-18/lib /usr/arm-linux-gnueabihf/lib/llvm-18/lib
sudo docker cp ubuntu-jammy-armv7:/usr/lib/llvm-18/include /usr/arm-linux-gnueabihf/lib/llvm-18/include
docker stop ubuntu-jammy-armv7
build: yarn build --target armv7-unknown-linux-gnueabihf --zig --zig-link-only --zig-abi-suffix=2.18
build: yarn build --target armv7-unknown-linux-gnueabihf --use-napi-cross
- host: ubuntu-latest
target: 'aarch64-linux-android'
downloadTarget: 'aarch64-linux-android'
Expand Down Expand Up @@ -525,67 +525,6 @@ jobs:
name: failure-images-aarch64-unknown-linux-musl-lts
path: __test__/failure/**

test-linux-arm-gnueabihf-binding:
name: Test bindings on armv7-unknown-linux-gnueabihf - node@${{ matrix.node }}
needs:
- build
strategy:
fail-fast: false
matrix:
node: ['18', '20', '22']
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: Download Apple Emoji font
uses: robinraju/release-downloader@v1
with:
repository: 'PoomSmart/EmojiLibrary'
tag: '0.15.4'
fileName: [email protected]
token: ${{ secrets.GITHUB_TOKEN }}
out-file-path: __test__/fonts/

- name: Download artifacts
uses: actions/download-artifact@v4
with:
name: bindings-armv7-unknown-linux-gnueabihf
path: .

- name: List packages
run: ls -R .
shell: bash

- name: Install dependencies
run: |
yarn config set supportedArchitectures.cpu "arm"
yarn install --immutable --mode=skip-build
- name: Set up QEMU
uses: docker/setup-qemu-action@v3
with:
platforms: arm

- run: docker run --rm --privileged multiarch/qemu-user-static --reset -p yes

- name: Setup and run tests
uses: addnab/docker-run-action@v3
with:
image: node:${{ matrix.node }}-bullseye-slim
options: '--platform linux/arm/v7 -v ${{ github.workspace }}:/skia -w /skia'
run: |
set -e && \
yarn test:ci && \
ls -la
- name: Test failed
if: ${{ failure() }}
uses: actions/upload-artifact@v4
with:
name: failure-images-armv7-unknown-linux-gnueabihf-${{ matrix.node }}
path: __test__/failure/**

rust-test:
name: stable - macOS - cargo - test
runs-on: macos-latest
Expand Down Expand Up @@ -658,7 +597,6 @@ jobs:
- test-linux-x64-musl-binding
- test-linux-aarch64-gnu-binding
- test-linux-aarch64-musl-binding
- test-linux-arm-gnueabihf-binding
- test-macOS-windows-binding

steps:
Expand Down
17 changes: 17 additions & 0 deletions __test__/regression.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,20 @@ test('shadow-blur-zero-with-text', async (t) => {
ctx.fillText('TEST', 100, 100)
await snapshotImage(t, { ctx, canvas })
})

// https://github.com/Brooooooklyn/canvas/issues/973
test('putImageData double free', (t) => {
const canvas = createCanvas(1920, 1080)
const ctx = canvas.getContext('2d')

const canvas2 = createCanvas(640, 480)
const ctx2 = canvas2.getContext('2d')
ctx2.fillStyle = 'white'
ctx2.fillRect(0, 0, canvas2.width, canvas2.height)

let imgData = ctx2.getImageData(0, 0, canvas2.width, canvas2.height)

t.notThrows(() => {
ctx.putImageData(imgData, 0, 0, 0, 0, canvas.width, canvas.height)
})
})
23 changes: 3 additions & 20 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,28 +185,11 @@ fn main() {
}
"arm" => {
link_libcxx(&mut build);
let gcc_version = String::from_utf8(
process::Command::new("ls")
.arg("/usr/arm-linux-gnueabihf/include/c++")
.output()
.unwrap()
.stdout,
)
.unwrap();
let gcc_version_trim = gcc_version.trim();
env::set_var("CC", "clang");
env::set_var("CXX", "clang++");
build
.include("/usr/arm-linux-gnueabihf/include")
.include(format!(
"/usr/arm-linux-gnueabihf/include/c++/${gcc_version_trim}/arm-linux-gnueabihf"
));
const CROSS_LIB_PATH: &str = "/usr/lib/gcc-cross/arm-linux-gnueabihf";
if let Ok(version) = std::process::Command::new("ls")
.arg(CROSS_LIB_PATH)
.output()
.map(|o| String::from_utf8(o.stdout).unwrap().trim().to_string())
{
println!("cargo:rustc-link-search={CROSS_LIB_PATH}/{version}");
};
.include(format!("/usr/lib/llvm-18/include/c++/v1"));
println!("cargo:rustc-link-search=/usr/arm-linux-gnueabihf/lib");
println!("cargo:rustc-link-search=/usr/arm-linux-gnueabihf/lib/llvm-18/lib");
}
Expand Down
28 changes: 14 additions & 14 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,19 @@
"load-image.js"
],
"napi": {
"name": "skia",
"triples": {
"defaults": true,
"additional": [
"armv7-unknown-linux-gnueabihf",
"x86_64-unknown-linux-musl",
"aarch64-unknown-linux-gnu",
"aarch64-unknown-linux-musl",
"aarch64-apple-darwin",
"aarch64-linux-android",
"riscv64-unknown-linux-gnu"
]
}
"binaryName": "skia",
"targets": [
"x86_64-unknown-linux-gnu",
"x86_64-apple-darwin",
"x86_64-pc-windows-msvc",
"armv7-unknown-linux-gnueabihf",
"x86_64-unknown-linux-musl",
"aarch64-unknown-linux-gnu",
"aarch64-unknown-linux-musl",
"aarch64-apple-darwin",
"aarch64-linux-android",
"riscv64-unknown-linux-gnu"
]
},
"engines": {
"node": ">= 10"
Expand Down Expand Up @@ -71,7 +71,7 @@
"@jimp/custom": "^0.22.10",
"@jimp/jpeg": "^0.22.10",
"@jimp/png": "^0.22.10",
"@napi-rs/cli": "^2.18.0",
"@napi-rs/cli": "^3.0.0-alpha.69",
"@octokit/rest": "^21.0.0",
"@swc-node/register": "^1.8.0",
"@swc/core": "^1.4.0",
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[toolchain]
channel = "1.82.0"
channel = "1.84.0"
profile = "default"
4 changes: 2 additions & 2 deletions skia-c/skia_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,8 +632,8 @@ extern "C"
{
auto color_space = COLOR_SPACE_CAST;
auto info = SkImageInfo::Make(width, height, SkColorType::kRGBA_8888_SkColorType, SkAlphaType::kUnpremul_SkAlphaType, color_space);
auto data = SkData::MakeFromMalloc(pixels, length);
auto image = SkImages::RasterFromData(info, data, row_bytes);
auto pixmap = SkPixmap(info, pixels, row_bytes);
auto image = SkImages::RasterFromPixmap(pixmap, nullptr, nullptr);
auto src_rect = SkRect::MakeXYWH(dirty_x, dirty_y, dirty_width, dirty_height);
auto dst_rect = SkRect::MakeXYWH(x + dirty_x, y + dirty_y, dirty_width, dirty_height);
const auto sampling = SkSamplingOptions(SkCubicResampler::Mitchell());
Expand Down
4 changes: 2 additions & 2 deletions src/ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2133,7 +2133,7 @@ impl ContextOutputData {
pub(crate) fn into_buffer_slice<'a>(self, env: Env) -> Result<BufferSlice<'a>> {
match self {
ContextOutputData::Skia(output) => unsafe {
BufferSlice::from_external(&env, output.0.ptr, output.0.size, output, |data_ref, _| {
BufferSlice::from_external(&env, output.0.ptr, output.0.size, output, |_, data_ref| {
mem::drop(data_ref)
})
},
Expand All @@ -2143,7 +2143,7 @@ impl ContextOutputData {
output.as_ptr().cast_mut(),
output.len(),
output,
|data_ref, _| mem::drop(data_ref),
|_, data_ref| mem::drop(data_ref),
)
},
}
Expand Down
2 changes: 1 addition & 1 deletion src/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl<'a> From<Err<Error<&'a str>>> for ParseFilterError<'a> {
}
}

impl<'a> From<ParseFloatError> for ParseFilterError<'a> {
impl From<ParseFloatError> for ParseFilterError<'_> {
fn from(value: ParseFloatError) -> Self {
Self::ParseFloatError(value)
}
Expand Down
8 changes: 4 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub struct CanvasElement<'scope> {
}

#[napi]
impl<'scope> CanvasElement<'scope> {
impl CanvasElement<'_> {
fn create_context(
env: &Env,
width: u32,
Expand Down Expand Up @@ -224,7 +224,7 @@ impl<'scope> CanvasElement<'scope> {
data_ref.0.ptr,
data_ref.0.size,
data_ref,
|data: SkiaDataRef, _| mem::drop(data),
|_, data: SkiaDataRef| mem::drop(data),
)
},
ContextOutputData::Avif(output) => unsafe {
Expand All @@ -233,7 +233,7 @@ impl<'scope> CanvasElement<'scope> {
output.as_ptr().cast_mut(),
output.len(),
output,
|data, _| mem::drop(data),
|_, data| mem::drop(data),
)
},
}
Expand Down Expand Up @@ -525,7 +525,7 @@ impl<'scope> SVGCanvas<'scope> {
let svg_data_stream = self.ctx.context.stream.as_ref().unwrap();
let svg_data = svg_data_stream.data(self.ctx.context.width, self.ctx.context.height);
unsafe {
BufferSlice::from_external(&env, svg_data.0.ptr, svg_data.0.size, svg_data, |d, _| {
BufferSlice::from_external(&env, svg_data.0.ptr, svg_data.0.size, svg_data, |_, d| {
mem::drop(d)
})
}
Expand Down
8 changes: 4 additions & 4 deletions src/sk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ pub enum ColorType {
RGBAF32,
/// pixel using C float for red, green, blue, alpha; in 128-bit word
/// The following 6 colortypes are just for reading from - not for rendering to
///
/// pixel with a uint8_t for red and green
R8G8Unorm,
// pixel with a half float for alpha
Expand Down Expand Up @@ -1903,7 +1903,7 @@ pub struct SurfaceData<'a> {
slice: &'a [u8],
}

impl<'a> Deref for SurfaceData<'a> {
impl Deref for SurfaceData<'_> {
type Target = [u8];

fn deref(&self) -> &[u8] {
Expand All @@ -1915,7 +1915,7 @@ pub struct SurfaceDataMut<'a> {
slice: &'a mut [u8],
}

impl<'a> Deref for SurfaceDataMut<'a> {
impl Deref for SurfaceDataMut<'_> {
type Target = [u8];

fn deref(&self) -> &[u8] {
Expand All @@ -1941,7 +1941,7 @@ impl Drop for SkiaDataRef {
unsafe impl Send for SkiaDataRef {}
unsafe impl Sync for SkiaDataRef {}

impl<'a> DerefMut for SurfaceDataMut<'a> {
impl DerefMut for SurfaceDataMut<'_> {
fn deref_mut(&mut self) -> &mut [u8] {
self.slice
}
Expand Down
2 changes: 1 addition & 1 deletion src/svg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn convert_svg_text_to_path(
)
})
.and_then(|v| unsafe {
BufferSlice::from_external(env, v.0.ptr, v.0.size, v, |d, _| mem::drop(d))
BufferSlice::from_external(env, v.0.ptr, v.0.size, v, |_, d| mem::drop(d))
})
}

Expand Down
Loading

0 comments on commit 814262a

Please sign in to comment.