Skip to content
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.

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

fix: double free in putImageData #975

Merged
merged 1 commit into from
Jan 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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: 5 additions & 18 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,28 +185,15 @@ 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++");
env::set_var("TARGET_CC", "clang");
env::set_var("TARGET_CXX", "clang++");
build
.include("/usr/arm-linux-gnueabihf/include")
.include(format!(
"/usr/arm-linux-gnueabihf/include/c++/${gcc_version_trim}/arm-linux-gnueabihf"
"/usr/arm-linux-gnueabihf/lib/llvm-18/include/c++/v1"
));
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}");
};
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
Loading