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

Commit d3275de4 authored by Kohsuke Yatoh's avatar Kohsuke Yatoh
Browse files

Close FDs even when throwing SecurityException. 

Bug: 187879195
Test: atest UpdatableSystemFontTest
Change-Id: I34d04f152210fe2f89f4d96e920978dde21d06c8
parent 7c41518f

services/core/java/com/android/server/graphics/fonts/FontManagerService.java

+30 −18
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.server.graphics.fonts; 
import android.Manifest;

import android.annotation.NonNull;

import android.annotation.Nullable;

import android.annotation.RequiresPermission;

import android.content.Context;

import android.graphics.Typeface;

import android.graphics.fonts.FontFamily;
@@ -62,6 +63,7 @@ public final class FontManagerService extends IFontManager.Stub { 


    private static final String FONT_FILES_DIR = "/data/fonts/files";



    @RequiresPermission(Manifest.permission.UPDATE_FONTS)

    @Override

    public FontConfig getFontConfig() {

        getContext().enforceCallingPermission(Manifest.permission.UPDATE_FONTS,
@@ -69,8 +71,10 @@ public final class FontManagerService extends IFontManager.Stub { 
        return getSystemFontConfig();

    }



    @RequiresPermission(Manifest.permission.UPDATE_FONTS)

    @Override

    public int updateFontFamily(@NonNull List<FontUpdateRequest> requests, int baseVersion) {

        try {

            Preconditions.checkArgumentNonnegative(baseVersion);

            Objects.requireNonNull(requests);

            getContext().enforceCallingPermission(Manifest.permission.UPDATE_FONTS,
@@ -81,10 +85,20 @@ public final class FontManagerService extends IFontManager.Stub { 
            } catch (SystemFontException e) {

                Slog.e(TAG, "Failed to update font family", e);

                return e.getErrorCode();

            }

        } finally {

            closeFileDescriptors(requests);

        }

    }



    private static void closeFileDescriptors(@Nullable List<FontUpdateRequest> requests) {

        // Make sure we close every passed FD, even if 'requests' is constructed incorrectly and

        // some fields are null.

        if (requests == null) return;

        for (FontUpdateRequest request : requests) {

            if (request == null) continue;

            ParcelFileDescriptor fd = request.getFd();

                if (fd != null) {

            if (fd == null) continue;

            try {

                fd.close();

            } catch (IOException e) {
@@ -92,8 +106,6 @@ public final class FontManagerService extends IFontManager.Stub { 
            }

        }

    }

        }

    }



    /* package */ static class SystemFontException extends AndroidException {

        private final int mErrorCode;

tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java

+26 −7
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import static android.os.ParcelFileDescriptor.MODE_READ_ONLY; 


import static com.google.common.truth.Truth.assertThat;



import static org.junit.Assert.assertThrows;

import static org.junit.Assume.assumeTrue;



import static java.util.concurrent.TimeUnit.SECONDS;
@@ -247,7 +248,6 @@ public class UpdatableSystemFontTest { 
        assertThat(fontPathAfterReboot).isEqualTo(fontPath);

    }





    @Test

    public void fdLeakTest() throws Exception {

        long originalOpenFontCount =
@@ -273,6 +273,20 @@ public class UpdatableSystemFontTest { 
        }

    }



    @Test

    public void fdLeakTest_withoutPermission() throws Exception {

        Pattern patternEmojiVPlus1 =

                Pattern.compile(Pattern.quote(TEST_NOTO_COLOR_EMOJI_VPLUS1_TTF));

        byte[] signature = Files.readAllBytes(Paths.get(TEST_NOTO_COLOR_EMOJI_VPLUS1_TTF_FSV_SIG));

        try (ParcelFileDescriptor fd = ParcelFileDescriptor.open(

                new File(TEST_NOTO_COLOR_EMOJI_VPLUS1_TTF), MODE_READ_ONLY)) {

            assertThrows(SecurityException.class,

                    () -> updateFontFileWithoutPermission(fd, signature, 0));

        }

        List<String> openFiles = getOpenFiles("system_server");

        assertThat(countMatch(openFiles, patternEmojiVPlus1)).isEqualTo(0);

    }



    private static String insertCert(String certPath) throws Exception {

        Pair<String, String> result;

        try (InputStream is = new FileInputStream(certPath)) {
@@ -290,14 +304,19 @@ public class UpdatableSystemFontTest { 
        try (ParcelFileDescriptor fd =

                ParcelFileDescriptor.open(new File(fontPath), MODE_READ_ONLY)) {

            return SystemUtil.runWithShellPermissionIdentity(() -> {

                FontConfig fontConfig = mFontManager.getFontConfig();

                int configVersion = mFontManager.getFontConfig().getConfigVersion();

                return updateFontFileWithoutPermission(fd, signature, configVersion);

            });

        }

    }



    private int updateFontFileWithoutPermission(ParcelFileDescriptor fd, byte[] signature,

            int configVersion) {

        return mFontManager.updateFontFamily(

                new FontFamilyUpdateRequest.Builder()

                        .addFontFileUpdateRequest(new FontFileUpdateRequest(fd, signature))

                        .build(),

                        fontConfig.getConfigVersion());

            });

        }

                configVersion);

    }



    private String getFontPath(String psName) {