From 542b3761a06bbfe82a129eec71dbc355fbbde5c6 Mon Sep 17 00:00:00 2001 From: xDark <19853368+xxDark@users.noreply.github.com> Date: Sat, 21 May 2022 15:49:37 +0300 Subject: [PATCH 1/5] Add fallback path for locked-up environments --- .../software/coley/llzip/util/BufferData.java | 34 ++++++- .../software/coley/llzip/util/ByteData.java | 3 +- .../coley/llzip/util/CleanerUtil.java | 90 +++++++++++++++++++ .../coley/llzip/util/FileMapUtil.java | 67 ++++++++++---- .../coley/llzip/util/UnsafeMappedFile.java | 19 +++- 5 files changed, 190 insertions(+), 23 deletions(-) create mode 100644 src/main/java/software/coley/llzip/util/CleanerUtil.java diff --git a/src/main/java/software/coley/llzip/util/BufferData.java b/src/main/java/software/coley/llzip/util/BufferData.java index 7855714..e4141b3 100644 --- a/src/main/java/software/coley/llzip/util/BufferData.java +++ b/src/main/java/software/coley/llzip/util/BufferData.java @@ -6,12 +6,18 @@ import java.nio.ByteOrder; /** - * Mapped file that is backed by byte buffer. + * File that is backed by a byte buffer. * * @author xDark */ public final class BufferData implements ByteData { private final ByteBuffer buffer; + private volatile boolean cleaned; + + private BufferData(ByteBuffer buffer, Void slice) { + this.buffer = buffer; + cleaned = true; + } private BufferData(ByteBuffer buffer) { this.buffer = buffer; @@ -62,7 +68,7 @@ public void transferTo(OutputStream out, byte[] buf) throws IOException { @Override public ByteData slice(long startIndex, long endIndex) { - return new BufferData(ByteDataUtil.sliceExact(buffer, validate(startIndex), validate(endIndex))); + return new BufferData(ByteDataUtil.sliceExact(buffer, validate(startIndex), validate(endIndex)), null); } @Override @@ -83,6 +89,30 @@ public int hashCode() { return buffer.hashCode(); } + @Override + public void close() { + if (!cleaned) { + synchronized (this) { + if (cleaned) + return; + cleaned = true; + } + ByteBuffer buffer = this.buffer; + if (buffer.isDirect()) { + CleanerUtil.invokeCleaner(buffer); + } + } + } + + @Override + protected void finalize() throws Throwable { + try { + close(); + } finally { + super.finalize(); + } + } + private static int validate(long v) { if (v < 0L || v > Integer.MAX_VALUE) { throw new IllegalArgumentException(Long.toString(v)); diff --git a/src/main/java/software/coley/llzip/util/ByteData.java b/src/main/java/software/coley/llzip/util/ByteData.java index 14e9b2b..4f362c6 100644 --- a/src/main/java/software/coley/llzip/util/ByteData.java +++ b/src/main/java/software/coley/llzip/util/ByteData.java @@ -1,5 +1,6 @@ package software.coley.llzip.util; +import java.io.Closeable; import java.io.IOException; import java.io.OutputStream; @@ -8,7 +9,7 @@ * * @author xDark */ -public interface ByteData { +public interface ByteData extends Closeable { /** * Gets int at specific position. * diff --git a/src/main/java/software/coley/llzip/util/CleanerUtil.java b/src/main/java/software/coley/llzip/util/CleanerUtil.java new file mode 100644 index 0000000..400e9fd --- /dev/null +++ b/src/main/java/software/coley/llzip/util/CleanerUtil.java @@ -0,0 +1,90 @@ +package software.coley.llzip.util; + +import sun.misc.Unsafe; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.nio.ByteBuffer; + +/** + * Utility to invoke cleaners in a {@link ByteBuffer}. + * + * @author xDark + */ +public final class CleanerUtil { + + private static final Method INVOKE_CLEANER; + private static final Method GET_CLEANER; + private static final boolean SUPPORTED; + + private CleanerUtil() { + } + + /** + * Attempts to clean direct buffer. + * + * @param buffer + * Buffer to clean. + * + * @throws IllegalStateException + * If buffer is not direct, slice or duplicate, or + * cleaner failed to invoke. + */ + public static void invokeCleaner(ByteBuffer buffer) { + if (!buffer.isDirect()) { + throw new IllegalStateException("buffer is not direct"); + } + if (!SUPPORTED) { + return; + } + Method getCleaner = GET_CLEANER; + Method invokeCleaner = INVOKE_CLEANER; + try { + if (getCleaner != null) { + Object cleaner = getCleaner.invoke(buffer); + if (cleaner == null) { + throw new IllegalStateException("slice or duplicate"); + } + invokeCleaner.invoke(cleaner); + } else { + invokeCleaner.invoke(UnsafeUtil.get(), buffer); + } + } catch(InvocationTargetException ex) { + throw new IllegalStateException("Failed to invoke clean method", ex.getTargetException()); + } catch(IllegalAccessException ex) { + throw new IllegalStateException("cleaner became inaccessible", ex); + } + } + + static { + boolean supported = false; + Method invokeCleaner; + Method getCleaner = null; + try { + invokeCleaner = Unsafe.class.getDeclaredMethod("invokeCleaner", ByteBuffer.class); + invokeCleaner.setAccessible(true); + ByteBuffer tmp = ByteBuffer.allocateDirect(1); + invokeCleaner.invoke(UnsafeUtil.get(), tmp); + supported = true; + } catch(NoSuchMethodException ignored) { + supported = true; + ByteBuffer tmp = ByteBuffer.allocateDirect(1); + try { + Class directBuffer = Class.forName("sun.nio.ch.DirectBuffer"); + getCleaner = directBuffer.getDeclaredMethod("cleaner"); + invokeCleaner = getCleaner.getReturnType().getDeclaredMethod("clean"); + invokeCleaner.setAccessible(true); + getCleaner.setAccessible(true); + invokeCleaner.invoke(getCleaner.invoke(tmp)); + } catch(Exception ignored1) { + invokeCleaner = null; + getCleaner = null; + } + } catch(Exception ex) { + invokeCleaner = null; + } + INVOKE_CLEANER = invokeCleaner; + GET_CLEANER = getCleaner; + SUPPORTED = supported; + } +} diff --git a/src/main/java/software/coley/llzip/util/FileMapUtil.java b/src/main/java/software/coley/llzip/util/FileMapUtil.java index 217ae8a..4f46fb9 100644 --- a/src/main/java/software/coley/llzip/util/FileMapUtil.java +++ b/src/main/java/software/coley/llzip/util/FileMapUtil.java @@ -3,7 +3,10 @@ import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.nio.ByteOrder; +import java.nio.MappedByteBuffer; import java.nio.channels.FileChannel; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; @@ -27,9 +30,24 @@ public class FileMapUtil { * * @throws IOException * If any I/O error occurs. + * @throws IllegalStateException + * If the environment is locked up and + * file is larger than 2GB. */ public static ByteData map(Path path) throws IOException { - try (FileChannel fc = FileChannel.open(path, StandardOpenOption.READ)) { + if (MAP == null) { + long size = Files.size(path); + if (size <= Integer.MAX_VALUE) { + try(FileChannel fc = FileChannel.open(path, StandardOpenOption.READ)) { + long length = fc.size(); + MappedByteBuffer buffer = fc.map(FileChannel.MapMode.READ_ONLY, 0L, length); + buffer.order(ByteOrder.LITTLE_ENDIAN); + return BufferData.wrap(buffer); + } + } + throw new IllegalStateException("Cannot map more than 2GB of data in locked up environment"); + } + try(FileChannel fc = FileChannel.open(path, StandardOpenOption.READ)) { long length = fc.size(); long address; try { @@ -38,13 +56,13 @@ public static ByteData map(Path path) throws IOException { } else { address = (long) MAP.invoke(fc, 0, 0L, length, false); } - } catch (InvocationTargetException | IllegalAccessException ex) { + } catch(InvocationTargetException | IllegalAccessException ex) { throw new IllegalStateException("Could not invoke map0", ex); } ByteData mappedFile = new UnsafeMappedFile(address, length, () -> { try { UNMAP.invoke(null, address, length); - } catch (IllegalAccessException | InvocationTargetException ex) { + } catch(IllegalAccessException | InvocationTargetException ex) { throw new InternalError(ex); } }); @@ -54,23 +72,38 @@ public static ByteData map(Path path) throws IOException { static { boolean oldMap = false; - try { - Class c = Class.forName("sun.nio.ch.FileChannelImpl"); - Method map; + Method map = null; + Method unmap = null; + get: + { + Class c; + try { + c = Class.forName("sun.nio.ch.FileChannelImpl"); + } catch(ClassNotFoundException ignored) { + break get; + } try { map = c.getDeclaredMethod("map0", int.class, long.class, long.class, boolean.class); - } catch (NoSuchMethodException ex) { - map = c.getDeclaredMethod("map0", int.class, long.class, long.class); - oldMap = true; + } catch(NoSuchMethodException ex) { + try { + map = c.getDeclaredMethod("map0", int.class, long.class, long.class); + oldMap = true; + } catch(NoSuchMethodException ignored) { + break get; + } + } + try { + map.setAccessible(true); + unmap = c.getDeclaredMethod("unmap0", long.class, long.class); + unmap.setAccessible(true); + } catch(Exception ex) { + // Locked up environment, probably threw InaccessibleObjectException + map = null; + unmap = null; } - map.setAccessible(true); - Method unmap = c.getDeclaredMethod("unmap0", long.class, long.class); - unmap.setAccessible(true); - MAP = map; - UNMAP = unmap; - OLD_MAP = oldMap; - } catch (ClassNotFoundException | NoSuchMethodException ex) { - throw new ExceptionInInitializerError(ex); } + MAP = map; + UNMAP = unmap; + OLD_MAP = oldMap; } } diff --git a/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java b/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java index d610826..53b98fb 100644 --- a/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java +++ b/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java @@ -15,6 +15,7 @@ final class UnsafeMappedFile implements ByteData { private static final boolean SWAP = ByteOrder.nativeOrder() == ByteOrder.BIG_ENDIAN; private static final Unsafe UNSAFE = UnsafeUtil.get(); + private volatile boolean closed; private final long address; private final long end; private final Runnable deallocator; @@ -101,13 +102,25 @@ public int hashCode() { return result; } + @Override + public void close() { + if (!closed) { + synchronized (this) { + if (closed) + return; + closed = true; + } + } + Runnable deallocator = this.deallocator; + if (deallocator != null) + deallocator.run(); + } + @SuppressWarnings("deprecation") @Override protected void finalize() throws Throwable { - Runnable deallocator = this.deallocator; try { - if (deallocator != null) - deallocator.run(); + close(); } finally { super.finalize(); } From b8fbd2f17df4a38c651d81f4f84704b531acf9fd Mon Sep 17 00:00:00 2001 From: xDark <19853368+xxDark@users.noreply.github.com> Date: Sat, 21 May 2022 17:31:23 +0300 Subject: [PATCH 2/5] set supported flag to false on exception --- src/main/java/software/coley/llzip/util/CleanerUtil.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/software/coley/llzip/util/CleanerUtil.java b/src/main/java/software/coley/llzip/util/CleanerUtil.java index 400e9fd..70dce79 100644 --- a/src/main/java/software/coley/llzip/util/CleanerUtil.java +++ b/src/main/java/software/coley/llzip/util/CleanerUtil.java @@ -79,6 +79,7 @@ public static void invokeCleaner(ByteBuffer buffer) { } catch(Exception ignored1) { invokeCleaner = null; getCleaner = null; + supported = false; } } catch(Exception ex) { invokeCleaner = null; From 59711d0279b731519dc1c8aa001c534036fa1050 Mon Sep 17 00:00:00 2001 From: xDark <19853368+xxDark@users.noreply.github.com> Date: Sat, 21 May 2022 19:13:31 +0300 Subject: [PATCH 3/5] Prevent access to byte data after close to prevent accidental memory corruption --- .../software/coley/llzip/util/BufferData.java | 12 +++++++++++ .../coley/llzip/util/UnsafeMappedFile.java | 20 +++++++++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/main/java/software/coley/llzip/util/BufferData.java b/src/main/java/software/coley/llzip/util/BufferData.java index e4141b3..b10555f 100644 --- a/src/main/java/software/coley/llzip/util/BufferData.java +++ b/src/main/java/software/coley/llzip/util/BufferData.java @@ -25,21 +25,25 @@ private BufferData(ByteBuffer buffer) { @Override public int getInt(long position) { + ensureOpen(); return buffer.getInt(validate(position)); } @Override public short getShort(long position) { + ensureOpen(); return buffer.getShort(validate(position)); } @Override public byte get(long position) { + ensureOpen(); return buffer.get(validate(position)); } @Override public void get(long position, byte[] b, int off, int len) { + ensureOpen(); ByteBuffer buffer = this.buffer; ((ByteBuffer) buffer.slice() .order(buffer.order()) @@ -49,6 +53,7 @@ public void get(long position, byte[] b, int off, int len) { @Override public void transferTo(OutputStream out, byte[] buf) throws IOException { + ensureOpen(); ByteBuffer buffer = this.buffer; int remaining = buffer.remaining(); if (buffer.hasArray()) { @@ -68,11 +73,13 @@ public void transferTo(OutputStream out, byte[] buf) throws IOException { @Override public ByteData slice(long startIndex, long endIndex) { + ensureOpen(); return new BufferData(ByteDataUtil.sliceExact(buffer, validate(startIndex), validate(endIndex)), null); } @Override public long length() { + ensureOpen(); return ByteDataUtil.length(buffer); } @@ -113,6 +120,11 @@ protected void finalize() throws Throwable { } } + private void ensureOpen() { + if (cleaned) + throw new IllegalStateException("Cannot access data after close"); + } + private static int validate(long v) { if (v < 0L || v > Integer.MAX_VALUE) { throw new IllegalArgumentException(Long.toString(v)); diff --git a/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java b/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java index 53b98fb..91394c5 100644 --- a/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java +++ b/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java @@ -15,7 +15,7 @@ final class UnsafeMappedFile implements ByteData { private static final boolean SWAP = ByteOrder.nativeOrder() == ByteOrder.BIG_ENDIAN; private static final Unsafe UNSAFE = UnsafeUtil.get(); - private volatile boolean closed; + private volatile boolean cleaned; private final long address; private final long end; private final Runnable deallocator; @@ -36,21 +36,25 @@ private UnsafeMappedFile(Object attachment, long address, long end) { @Override public int getInt(long position) { + ensureOpen(); return swap(UNSAFE.getInt(validate(position))); } @Override public short getShort(long position) { + ensureOpen(); return swap(UNSAFE.getShort(validate(position))); } @Override public byte get(long position) { + ensureOpen(); return UNSAFE.getByte(validate(position)); } @Override public void get(long position, byte[] b, int off, int len) { + ensureOpen(); long address = validate(position); if (address + len > end) throw new IllegalArgumentException(); @@ -59,6 +63,7 @@ public void get(long position, byte[] b, int off, int len) { @Override public void transferTo(OutputStream out, byte[] buf) throws IOException { + ensureOpen(); int copyThreshold = buf.length; long address = this.address; long remaining = end - address; @@ -73,6 +78,7 @@ public void transferTo(OutputStream out, byte[] buf) throws IOException { @Override public ByteData slice(long startIndex, long endIndex) { + ensureOpen(); if (startIndex > endIndex) throw new IllegalArgumentException(); return new UnsafeMappedFile(this, validate(startIndex), validate(endIndex)); @@ -80,6 +86,7 @@ public ByteData slice(long startIndex, long endIndex) { @Override public long length() { + ensureOpen(); return end - address; } @@ -104,11 +111,11 @@ public int hashCode() { @Override public void close() { - if (!closed) { + if (!cleaned) { synchronized (this) { - if (closed) + if (cleaned) return; - closed = true; + cleaned = true; } } Runnable deallocator = this.deallocator; @@ -116,6 +123,11 @@ public void close() { deallocator.run(); } + private void ensureOpen() { + if (cleaned) + throw new IllegalStateException("Cannot access data after close"); + } + @SuppressWarnings("deprecation") @Override protected void finalize() throws Throwable { From 0c346231d6ebaefcb195a47816debe2a2be99785 Mon Sep 17 00:00:00 2001 From: xDark <19853368+xxDark@users.noreply.github.com> Date: Sat, 21 May 2022 19:45:16 +0300 Subject: [PATCH 4/5] Move deallocator to the right place --- src/main/java/software/coley/llzip/util/BufferData.java | 8 ++++---- .../java/software/coley/llzip/util/UnsafeMappedFile.java | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/software/coley/llzip/util/BufferData.java b/src/main/java/software/coley/llzip/util/BufferData.java index b10555f..f4946aa 100644 --- a/src/main/java/software/coley/llzip/util/BufferData.java +++ b/src/main/java/software/coley/llzip/util/BufferData.java @@ -103,10 +103,10 @@ public void close() { if (cleaned) return; cleaned = true; - } - ByteBuffer buffer = this.buffer; - if (buffer.isDirect()) { - CleanerUtil.invokeCleaner(buffer); + ByteBuffer buffer = this.buffer; + if (buffer.isDirect()) { + CleanerUtil.invokeCleaner(buffer); + } } } } diff --git a/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java b/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java index 91394c5..748caae 100644 --- a/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java +++ b/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java @@ -116,11 +116,11 @@ public void close() { if (cleaned) return; cleaned = true; + Runnable deallocator = this.deallocator; + if (deallocator != null) + deallocator.run(); } } - Runnable deallocator = this.deallocator; - if (deallocator != null) - deallocator.run(); } private void ensureOpen() { From 71d73441d1cc0f5f1ba8b9668d741eb695838335 Mon Sep 17 00:00:00 2001 From: xDark <19853368+xxDark@users.noreply.github.com> Date: Sat, 21 May 2022 21:40:24 +0300 Subject: [PATCH 5/5] Not used --- src/main/java/software/coley/llzip/util/BufferData.java | 7 +------ .../java/software/coley/llzip/util/UnsafeMappedFile.java | 4 +++- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/main/java/software/coley/llzip/util/BufferData.java b/src/main/java/software/coley/llzip/util/BufferData.java index f4946aa..0f6454d 100644 --- a/src/main/java/software/coley/llzip/util/BufferData.java +++ b/src/main/java/software/coley/llzip/util/BufferData.java @@ -14,11 +14,6 @@ public final class BufferData implements ByteData { private final ByteBuffer buffer; private volatile boolean cleaned; - private BufferData(ByteBuffer buffer, Void slice) { - this.buffer = buffer; - cleaned = true; - } - private BufferData(ByteBuffer buffer) { this.buffer = buffer; } @@ -74,7 +69,7 @@ public void transferTo(OutputStream out, byte[] buf) throws IOException { @Override public ByteData slice(long startIndex, long endIndex) { ensureOpen(); - return new BufferData(ByteDataUtil.sliceExact(buffer, validate(startIndex), validate(endIndex)), null); + return new BufferData(ByteDataUtil.sliceExact(buffer, validate(startIndex), validate(endIndex))); } @Override diff --git a/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java b/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java index 748caae..7b91a80 100644 --- a/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java +++ b/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java @@ -19,7 +19,8 @@ final class UnsafeMappedFile implements ByteData { private final long address; private final long end; private final Runnable deallocator; - private Object attachment; + @SuppressWarnings("unused") + private final Object attachment; private UnsafeMappedFile(Object attachment, long address, long end) { this.attachment = attachment; @@ -32,6 +33,7 @@ private UnsafeMappedFile(Object attachment, long address, long end) { this.address = address; this.end = address + length; this.deallocator = deallocator; + attachment = null; } @Override