Skip to content

Commit

Permalink
Merge pull request #39 from cryptomator/feature/dont-resolve-null-poi…
Browse files Browse the repository at this point in the history
…nters

Fix SIGSEV crashes by adding NULL pointer checks
  • Loading branch information
infeo authored Oct 20, 2023
2 parents 91e7b65 + c3bb154 commit 006b4c2
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 25 deletions.
1 change: 1 addition & 0 deletions jfuse-api/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

exports org.cryptomator.jfuse.api;
exports org.cryptomator.jfuse.api.platforms to org.cryptomator.jfuse.linux.aarch64, org.cryptomator.jfuse.linux.amd64, org.cryptomator.jfuse.mac, org.cryptomator.jfuse.win;
exports org.cryptomator.jfuse.api.util to org.cryptomator.jfuse.linux.aarch64, org.cryptomator.jfuse.linux.amd64, org.cryptomator.jfuse.mac, org.cryptomator.jfuse.win;

uses FuseBuilder;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.cryptomator.jfuse.api.util;

import org.jetbrains.annotations.Nullable;

import java.lang.foreign.MemorySegment;

public class MemoryUtils {

@Nullable
public static String toUtf8StringOrNull(MemorySegment string, long offset) {
return MemorySegment.NULL.equals(string) ? null : string.getUtf8String(offset);
}

@Nullable
public static String toUtf8StringOrNull(MemorySegment string) {
return toUtf8StringOrNull(string, 0);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package org.cryptomator.jfuse.api.util;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import java.lang.foreign.Arena;
import java.lang.foreign.MemorySegment;

public class MemoryUtilsTest {


@Test
@DisplayName("On MemorySegment != NULL pointer, method returns utf8 string in the memory region")
void testValidSegmentReturnsString() {
try (var arena = Arena.ofConfined()) {
var address = arena.allocate(4);
address.setUtf8String(0, "abc");
String result = MemoryUtils.toUtf8StringOrNull(address);
Assertions.assertEquals("abc", result);
}
}

@Test
@DisplayName("With offset, on MemorySegment != NULL pointer, method returns utf8 string in the memory region")
void testValidSegmentReturnsStringAtOffset() {
try (var arena = Arena.ofConfined()) {
var address = arena.allocate(10);
address.setUtf8String(5, "abc");
String result = MemoryUtils.toUtf8StringOrNull(address, 5);
Assertions.assertEquals("abc", result);
}
}

@Test
@DisplayName("When MemorySegment == NULL pointer, method returns null")
void testNullPointerSegmentReturnsNull() {
String result = MemoryUtils.toUtf8StringOrNull(MemorySegment.NULL, 0);
Assertions.assertNull(result);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.cryptomator.jfuse.api.FileInfo;
import org.cryptomator.jfuse.linux.aarch64.extr.fcntl.fcntl_h;
import org.cryptomator.jfuse.linux.aarch64.extr.fuse3.fuse_file_info;
import org.jetbrains.annotations.Nullable;

import java.lang.foreign.Arena;
import java.lang.foreign.MemorySegment;
Expand All @@ -22,6 +23,18 @@ record FileInfoImpl(MemorySegment segment) implements FileInfo {
private static final int O_SYNC = fcntl_h.O_SYNC();
private static final int O_DSYNC = fcntl_h.O_DSYNC();

/**
* Factory method to map native memory to an {@link FileInfo} object
*
* @param address the {@link MemorySegment} representing the starting address
* @param scope the {@link Arena} in which this object will be alive
* @return an {@link FileInfo} object or {@code null} if {@code address} is a NULL pointer
*/
@Nullable
public static FileInfoImpl of(MemorySegment address, Arena scope) {
return MemorySegment.NULL.equals(address) ? null : new FileInfoImpl(address, scope);
}

public FileInfoImpl(MemorySegment address, Arena scope) {
this(fuse_file_info.ofAddress(address, scope));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.cryptomator.jfuse.api.FuseMount;
import org.cryptomator.jfuse.api.FuseMountFailedException;
import org.cryptomator.jfuse.api.FuseOperations;
import org.cryptomator.jfuse.api.util.MemoryUtils;
import org.cryptomator.jfuse.linux.aarch64.extr.fuse3.fuse_args;
import org.cryptomator.jfuse.linux.aarch64.extr.fuse3.fuse_h;
import org.cryptomator.jfuse.linux.aarch64.extr.fuse3.fuse_operations;
Expand Down Expand Up @@ -116,14 +117,14 @@ private int access(MemorySegment path, int mask) {

private int chmod(MemorySegment path, int mode, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.chmod(path.getUtf8String(0), mode, new FileInfoImpl(fi, arena));
return fuseOperations.chmod(path.getUtf8String(0), mode, FileInfoImpl.of(fi, arena));
}
}

@VisibleForTesting
int chown(MemorySegment path, int uid, int gid, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.chown(path.getUtf8String(0), uid, gid, new FileInfoImpl(fi, arena));
return fuseOperations.chown(path.getUtf8String(0), uid, gid, FileInfoImpl.of(fi, arena));
}
}

Expand Down Expand Up @@ -154,13 +155,13 @@ int fsync(MemorySegment path, int datasync, MemorySegment fi) {
@VisibleForTesting
int fsyncdir(MemorySegment path, int datasync, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.fsyncdir(path.getUtf8String(0), datasync, new FileInfoImpl(fi, arena));
return fuseOperations.fsyncdir(MemoryUtils.toUtf8StringOrNull(path), datasync, new FileInfoImpl(fi, arena));
}
}

private int getattr(MemorySegment path, MemorySegment stat, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.getattr(path.getUtf8String(0), new StatImpl(stat, arena), new FileInfoImpl(fi, arena));
return fuseOperations.getattr(path.getUtf8String(0), new StatImpl(stat, arena), FileInfoImpl.of(fi, arena));
}
}

Expand Down Expand Up @@ -229,7 +230,7 @@ private int release(MemorySegment path, MemorySegment fi) {

private int releasedir(MemorySegment path, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.releasedir(path.getUtf8String(0), new FileInfoImpl(fi, arena));
return fuseOperations.releasedir(MemoryUtils.toUtf8StringOrNull(path), new FileInfoImpl(fi, arena));
}
}

Expand All @@ -253,7 +254,7 @@ private int symlink(MemorySegment linkname, MemorySegment target) {

private int truncate(MemorySegment path, long size, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.truncate(path.getUtf8String(0), size, new FileInfoImpl(fi, arena));
return fuseOperations.truncate(path.getUtf8String(0), size, FileInfoImpl.of(fi, arena));
}
}

Expand All @@ -271,11 +272,11 @@ int utimens(MemorySegment path, MemorySegment times, MemorySegment fi) {
timespec.tv_sec$set(segment, 0);
timespec.tv_nsec$set(segment, stat_h.UTIME_NOW());
var time = new TimeSpecImpl(segment);
return fuseOperations.utimens(path.getUtf8String(0), time, time, new FileInfoImpl(fi, arena));
return fuseOperations.utimens(path.getUtf8String(0), time, time, FileInfoImpl.of(fi, arena));
} else {
var time0 = times.asSlice(0, timespec.$LAYOUT().byteSize());
var time1 = times.asSlice(timespec.$LAYOUT().byteSize(), timespec.$LAYOUT().byteSize());
return fuseOperations.utimens(path.getUtf8String(0), new TimeSpecImpl(time0), new TimeSpecImpl(time1), new FileInfoImpl(fi, arena));
return fuseOperations.utimens(path.getUtf8String(0), new TimeSpecImpl(time0), new TimeSpecImpl(time1), FileInfoImpl.of(fi, arena));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.cryptomator.jfuse.api.FileInfo;
import org.cryptomator.jfuse.linux.amd64.extr.fcntl.fcntl_h;
import org.cryptomator.jfuse.linux.amd64.extr.fuse3.fuse_file_info;
import org.jetbrains.annotations.Nullable;

import java.lang.foreign.Arena;
import java.lang.foreign.MemorySegment;
Expand All @@ -22,6 +23,18 @@ record FileInfoImpl(MemorySegment segment) implements FileInfo {
private static final int O_SYNC = fcntl_h.O_SYNC();
private static final int O_DSYNC = fcntl_h.O_DSYNC();

/**
* Factory method to map native memory to an {@link FileInfo} object
*
* @param address the {@link MemorySegment} representing the starting address
* @param scope the {@link Arena} in which this object will be alive
* @return an {@link FileInfo} object or {@code null} if {@code address} is a NULL pointer
*/
@Nullable
public static FileInfoImpl of(MemorySegment address, Arena scope) {
return MemorySegment.NULL.equals(address) ? null : new FileInfoImpl(address, scope);
}

public FileInfoImpl(MemorySegment address, Arena scope) {
this(fuse_file_info.ofAddress(address, scope));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.cryptomator.jfuse.api.FuseMount;
import org.cryptomator.jfuse.api.FuseMountFailedException;
import org.cryptomator.jfuse.api.FuseOperations;
import org.cryptomator.jfuse.api.util.MemoryUtils;
import org.cryptomator.jfuse.linux.amd64.extr.fuse3.fuse_args;
import org.cryptomator.jfuse.linux.amd64.extr.fuse3.fuse_h;
import org.cryptomator.jfuse.linux.amd64.extr.fuse3.fuse_operations;
Expand Down Expand Up @@ -116,14 +117,14 @@ private int access(MemorySegment path, int mask) {

private int chmod(MemorySegment path, int mode, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.chmod(path.getUtf8String(0), mode, new FileInfoImpl(fi, arena));
return fuseOperations.chmod(path.getUtf8String(0), mode, FileInfoImpl.of(fi, arena));
}
}

@VisibleForTesting
int chown(MemorySegment path, int uid, int gid, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.chown(path.getUtf8String(0), uid, gid, new FileInfoImpl(fi, arena));
return fuseOperations.chown(path.getUtf8String(0), uid, gid, FileInfoImpl.of(fi, arena));
}
}

Expand Down Expand Up @@ -154,13 +155,13 @@ int fsync(MemorySegment path, int datasync, MemorySegment fi) {
@VisibleForTesting
int fsyncdir(MemorySegment path, int datasync, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.fsyncdir(path.getUtf8String(0), datasync, new FileInfoImpl(fi, arena));
return fuseOperations.fsyncdir(MemoryUtils.toUtf8StringOrNull(path), datasync, new FileInfoImpl(fi, arena));
}
}

private int getattr(MemorySegment path, MemorySegment stat, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.getattr(path.getUtf8String(0), new StatImpl(stat, arena), new FileInfoImpl(fi, arena));
return fuseOperations.getattr(path.getUtf8String(0), new StatImpl(stat, arena), FileInfoImpl.of(fi, arena));
}
}

Expand Down Expand Up @@ -229,7 +230,7 @@ private int release(MemorySegment path, MemorySegment fi) {

private int releasedir(MemorySegment path, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.releasedir(path.getUtf8String(0), new FileInfoImpl(fi, arena));
return fuseOperations.releasedir(MemoryUtils.toUtf8StringOrNull(path), new FileInfoImpl(fi, arena));
}
}

Expand All @@ -253,7 +254,7 @@ private int symlink(MemorySegment linkname, MemorySegment target) {

private int truncate(MemorySegment path, long size, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.truncate(path.getUtf8String(0), size, new FileInfoImpl(fi, arena));
return fuseOperations.truncate(path.getUtf8String(0), size, FileInfoImpl.of(fi, arena));
}
}

Expand All @@ -270,11 +271,11 @@ int utimens(MemorySegment path, MemorySegment times, MemorySegment fi) {
timespec.tv_sec$set(segment, 0);
timespec.tv_nsec$set(segment, stat_h.UTIME_NOW());
var time = new TimeSpecImpl(segment);
return fuseOperations.utimens(path.getUtf8String(0), time, time, new FileInfoImpl(fi, arena));
return fuseOperations.utimens(path.getUtf8String(0), time, time, FileInfoImpl.of(fi, arena));
} else {
var time0 = times.asSlice(0, timespec.$LAYOUT().byteSize());
var time1 = times.asSlice(timespec.$LAYOUT().byteSize(), timespec.$LAYOUT().byteSize());
return fuseOperations.utimens(path.getUtf8String(0), new TimeSpecImpl(time0), new TimeSpecImpl(time1), new FileInfoImpl(fi, arena));
return fuseOperations.utimens(path.getUtf8String(0), new TimeSpecImpl(time0), new TimeSpecImpl(time1), FileInfoImpl.of(fi, arena));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.cryptomator.jfuse.api.FuseMount;
import org.cryptomator.jfuse.api.FuseMountFailedException;
import org.cryptomator.jfuse.api.FuseOperations;
import org.cryptomator.jfuse.api.util.MemoryUtils;
import org.cryptomator.jfuse.mac.extr.fuse.fuse_args;
import org.cryptomator.jfuse.mac.extr.fuse.fuse_h;
import org.cryptomator.jfuse.mac.extr.fuse.fuse_operations;
Expand Down Expand Up @@ -154,7 +155,7 @@ int fsync(MemorySegment path, int datasync, MemorySegment fi) {
@VisibleForTesting
int fsyncdir(MemorySegment path, int datasync, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.fsyncdir(path.getUtf8String(0), datasync, new FileInfoImpl(fi, arena));
return fuseOperations.fsyncdir(MemoryUtils.toUtf8StringOrNull(path), datasync, new FileInfoImpl(fi, arena));
}
}

Expand Down Expand Up @@ -237,7 +238,7 @@ private int release(MemorySegment path, MemorySegment fi) {

private int releasedir(MemorySegment path, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.releasedir(path.getUtf8String(0), new FileInfoImpl(fi, arena));
return fuseOperations.releasedir(MemoryUtils.toUtf8StringOrNull(path), new FileInfoImpl(fi, arena));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.cryptomator.jfuse.api.FileInfo;
import org.cryptomator.jfuse.win.extr.fcntl.fcntl_h;
import org.cryptomator.jfuse.win.extr.fuse3.fuse3_file_info;
import org.jetbrains.annotations.Nullable;

import java.lang.foreign.Arena;
import java.lang.foreign.MemorySegment;
Expand All @@ -20,6 +21,18 @@ record FileInfoImpl(MemorySegment segment) implements FileInfo {
private static final int O_TRUNC = fcntl_h.O_TRUNC();
private static final int O_EXCL = fcntl_h.O_EXCL();

/**
* Factory method to map native memory to an {@link FileInfo} object
*
* @param address the {@link MemorySegment} representing the starting address
* @param scope the {@link Arena} in which this object will be alive
* @return an {@link FileInfo} object or {@code null} if {@code address} is a NULL pointer
*/
@Nullable
public static FileInfoImpl of(MemorySegment address, Arena scope) {
return MemorySegment.NULL.equals(address) ? null : new FileInfoImpl(address, scope);
}

public FileInfoImpl(MemorySegment address, Arena scope) {
this(fuse3_file_info.ofAddress(address, scope));
}
Expand Down
15 changes: 8 additions & 7 deletions jfuse-win/src/main/java/org/cryptomator/jfuse/win/FuseImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.cryptomator.jfuse.api.FuseMount;
import org.cryptomator.jfuse.api.FuseMountFailedException;
import org.cryptomator.jfuse.api.FuseOperations;
import org.cryptomator.jfuse.api.util.MemoryUtils;
import org.cryptomator.jfuse.win.extr.fuse2.fuse2_h;
import org.cryptomator.jfuse.win.extr.fuse2.fuse_args;
import org.cryptomator.jfuse.win.extr.fuse3.fuse3_operations;
Expand Down Expand Up @@ -125,14 +126,14 @@ MemorySegment init(MemorySegment conn, MemorySegment cfg) {

private int chmod(MemorySegment path, int mode, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.chmod(path.getUtf8String(0), mode, new FileInfoImpl(fi, arena));
return fuseOperations.chmod(path.getUtf8String(0), mode, FileInfoImpl.of(fi, arena));
}
}

@VisibleForTesting
int chown(MemorySegment path, int uid, int gid, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.chown(path.getUtf8String(0), uid, gid, new FileInfoImpl(fi, arena));
return fuseOperations.chown(path.getUtf8String(0), uid, gid, FileInfoImpl.of(fi, arena));
}
}

Expand Down Expand Up @@ -163,14 +164,14 @@ int fsync(MemorySegment path, int datasync, MemorySegment fi) {
@VisibleForTesting
int fsyncdir(MemorySegment path, int datasync, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.fsyncdir(path.getUtf8String(0), datasync, new FileInfoImpl(fi, arena));
return fuseOperations.fsyncdir(MemoryUtils.toUtf8StringOrNull(path), datasync, new FileInfoImpl(fi, arena));
}
}

@VisibleForTesting
int getattr(MemorySegment path, MemorySegment stat, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.getattr(path.getUtf8String(0), new StatImpl(stat, arena), new FileInfoImpl(fi, arena));
return fuseOperations.getattr(path.getUtf8String(0), new StatImpl(stat, arena), FileInfoImpl.of(fi, arena));
}
}

Expand Down Expand Up @@ -239,7 +240,7 @@ private int release(MemorySegment path, MemorySegment fi) {

private int releasedir(MemorySegment path, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.releasedir(path.getUtf8String(0), new FileInfoImpl(fi, arena));
return fuseOperations.releasedir(MemoryUtils.toUtf8StringOrNull(path), new FileInfoImpl(fi, arena));
}
}

Expand All @@ -264,7 +265,7 @@ private int symlink(MemorySegment linkname, MemorySegment target) {
@VisibleForTesting
int truncate(MemorySegment path, long size, MemorySegment fi) {
try (var arena = Arena.ofConfined()) {
return fuseOperations.truncate(path.getUtf8String(0), size, new FileInfoImpl(fi, arena));
return fuseOperations.truncate(path.getUtf8String(0), size, FileInfoImpl.of(fi, arena));
}
}

Expand All @@ -281,7 +282,7 @@ int utimens(MemorySegment path, MemorySegment times, MemorySegment fi) {
var segment = times.reinterpret(seq.byteSize());
var time0 = segment.asSlice(0, fuse_timespec.$LAYOUT().byteSize());
var time1 = segment.asSlice(fuse_timespec.$LAYOUT().byteSize(), fuse_timespec.$LAYOUT().byteSize());
return fuseOperations.utimens(path.getUtf8String(0), new TimeSpecImpl(time0), new TimeSpecImpl(time1), new FileInfoImpl(fi, arena));
return fuseOperations.utimens(path.getUtf8String(0), new TimeSpecImpl(time0), new TimeSpecImpl(time1), FileInfoImpl.of(fi, arena));
}
}

Expand Down

0 comments on commit 006b4c2

Please sign in to comment.