Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3880f299 authored by Yurii Zubrytskyi's avatar Yurii Zubrytskyi
Browse files

[hardening] Use read() instead of mmap() for incfs files

Digest calculation is currently optimized for perfromance, and
it uses fine-tuned mmap() to access the APK. As IncFS-based APKs
aren't supposed to be fully digested, this code will probably
never run for those. But just in case, it's better to switch
over to a slightly slower read()-based implementation that
doesn't have any chance of crashing the whole process via a
SIGBUS signal

Bug: 175632872
Test: atest framework/base/core
Change-Id: Id3dc4cd5090679120d95da40c0dfdde0d72b6bb0
parent 80d0f4c6
Loading
Loading
Loading
Loading
+9 −0
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import android.util.SparseArray;
import com.android.internal.annotations.GuardedBy;

import java.io.File;
import java.io.FileDescriptor;
import java.io.IOException;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
@@ -262,6 +263,13 @@ public final class IncrementalManager {
        return nativeIsIncrementalPath(path);
    }

    /**
     * Checks if an fd corresponds to a file on a mounted Incremental File System.
     */
    public static boolean isIncrementalFileFd(@NonNull FileDescriptor fd) {
        return nativeIsIncrementalFd(fd.getInt$());
    }

    /**
     * Returns raw signature for file if it's on Incremental File System.
     * Unsafe, use only if you are sure what you are doing.
@@ -423,5 +431,6 @@ public final class IncrementalManager {
    private static native boolean nativeIsEnabled();
    private static native boolean nativeIsV2Available();
    private static native boolean nativeIsIncrementalPath(@NonNull String path);
    private static native boolean nativeIsIncrementalFd(@NonNull int fd);
    private static native byte[] nativeUnsafeGetFileSignature(@NonNull String path);
}
+2 −3
Original line number Diff line number Diff line
@@ -200,10 +200,9 @@ public final class ApkSigningBlockUtils {
        // physical memory.

        DataSource beforeApkSigningBlock =
                new MemoryMappedFileDataSource(apkFileDescriptor, 0,
                        signatureInfo.apkSigningBlockOffset);
                DataSource.create(apkFileDescriptor, 0, signatureInfo.apkSigningBlockOffset);
        DataSource centralDir =
                new MemoryMappedFileDataSource(
                DataSource.create(
                        apkFileDescriptor, signatureInfo.centralDirOffset,
                        signatureInfo.eocdOffset - signatureInfo.centralDirOffset);

+22 −0
Original line number Diff line number Diff line
@@ -16,6 +16,10 @@

package android.util.apk;

import android.annotation.NonNull;
import android.os.incremental.IncrementalManager;

import java.io.FileDescriptor;
import java.io.IOException;
import java.security.DigestException;

@@ -35,4 +39,22 @@ interface DataSource {
     */
    void feedIntoDataDigester(DataDigester md, long offset, int size)
            throws IOException, DigestException;

    /**
     * Creates a DataSource that can handle the passed fd in the most efficient and safe manner.
     * @param fd file descriptor to read from
     * @param pos starting offset
     * @param size size of the region
     * @return created DataSource object
     */
    static @NonNull DataSource create(@NonNull FileDescriptor fd, long pos, long size) {
        if (IncrementalManager.isIncrementalFileFd(fd)) {
            // IncFS-based files may have missing pages, and reading those via mmap() results
            // in a SIGBUS signal. Java doesn't have a good way of catching it, ending up killing
            // the process by default. Going back to read() is the safest option for these files.
            return new ReadFileDataSource(fd, pos, size);
        } else {
            return new MemoryMappedFileDataSource(fd, pos, size);
        }
    }
}
+1 −0
Original line number Diff line number Diff line
@@ -40,6 +40,7 @@ class MemoryMappedFileDataSource implements DataSource {
    /**
     * Constructs a new {@code MemoryMappedFileDataSource} for the specified region of the file.
     *
     * @param fd file descriptor to read from.
     * @param position start position of the region in the file.
     * @param size size (in bytes) of the region.
     */
+73 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2021 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package android.util.apk;

import android.system.ErrnoException;
import android.system.Os;

import java.io.FileDescriptor;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.security.DigestException;

/**
 * {@link DataSource} which provides data from a file descriptor by reading the sections
 * of the file via raw read() syscall. This is slower than memory-mapping but safer.
 */
class ReadFileDataSource implements DataSource {
    private final FileDescriptor mFd;
    private final long mFilePosition;
    private final long mSize;

    private static final int CHUNK_SIZE = 1024 * 1024;

    /**
     * Constructs a new {@code ReadFileDataSource} for the specified region of the file.
     *
     * @param fd file descriptor to read from.
     * @param position start position of the region in the file.
     * @param size size (in bytes) of the region.
     */
    ReadFileDataSource(FileDescriptor fd, long position, long size) {
        mFd = fd;
        mFilePosition = position;
        mSize = size;
    }

    @Override
    public long size() {
        return mSize;
    }

    @Override
    public void feedIntoDataDigester(DataDigester md, long offset, int size)
            throws IOException, DigestException {
        try {
            final byte[] buffer = new byte[Math.min(size, CHUNK_SIZE)];
            final long start = mFilePosition + offset;
            final long end = start + size;
            for (long pos = start, curSize = Math.min(size, CHUNK_SIZE);
                    pos < end; curSize = Math.min(end - pos, CHUNK_SIZE)) {
                final int readSize = Os.pread(mFd, buffer, 0, (int) curSize, pos);
                md.consume(ByteBuffer.wrap(buffer, 0, readSize));
                pos += readSize;
            }
        } catch (ErrnoException e) {
            throw new IOException(e);
        }
    }
}
Loading