From 2e8c044b2cb88473b4f672991371dab18204a7c2 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Sun, 8 May 2022 00:39:35 +0000 Subject: [PATCH 1/2] Update sdk_version check for jni_libs of updatable apps With aosp/1640364, all variants of a cc_* module use min_sdk_version as the version part of the clang triple. Therefore, checking min_sdk_version of jni_libs should be sufficient to ensure that there is no unintended access to symbols in newer Android versions Test: go test ./java Test: TH Bug: 155209650 Bug: 209409604 Change-Id: I6c064f8a6ea12c8aa40165a9063380306a180c9b --- java/app.go | 10 +++------- java/app_test.go | 10 +++++++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/java/app.go b/java/app.go index 2b52eab15..036b4b544 100755 --- a/java/app.go +++ b/java/app.go @@ -299,10 +299,6 @@ func (a *AndroidApp) checkAppSdkVersions(ctx android.ModuleContext) { // If an updatable APK sets min_sdk_version, min_sdk_vesion of JNI libs should match with it. // This check is enforced for "updatable" APKs (including APK-in-APEX). -// b/155209650: until min_sdk_version is properly supported, use sdk_version instead. -// because, sdk_version is overridden by min_sdk_version (if set as smaller) -// and sdkLinkType is checked with dependencies so we can be sure that the whole dependency tree -// will meet the requirements. func (a *AndroidApp) checkJniLibsSdkVersion(ctx android.ModuleContext, minSdkVersion android.ApiLevel) { // It's enough to check direct JNI deps' sdk_version because all transitive deps from JNI deps are checked in cc.checkLinkType() ctx.VisitDirectDeps(func(m android.Module) { @@ -313,10 +309,10 @@ func (a *AndroidApp) checkJniLibsSdkVersion(ctx android.ModuleContext, minSdkVer // The domain of cc.sdk_version is "current" and // We can rely on android.SdkSpec to convert it to so that "current" is // handled properly regardless of sdk finalization. - jniSdkVersion, err := android.SdkSpecFrom(ctx, dep.SdkVersion()).EffectiveVersion(ctx) + jniSdkVersion, err := android.SdkSpecFrom(ctx, dep.MinSdkVersion()).EffectiveVersion(ctx) if err != nil || minSdkVersion.LessThan(jniSdkVersion) { - ctx.OtherModuleErrorf(dep, "sdk_version(%v) is higher than min_sdk_version(%v) of the containing android_app(%v)", - dep.SdkVersion(), minSdkVersion, ctx.ModuleName()) + ctx.OtherModuleErrorf(dep, "min_sdk_version(%v) is higher than min_sdk_version(%v) of the containing android_app(%v)", + dep.MinSdkVersion(), minSdkVersion, ctx.ModuleName()) return } diff --git a/java/app_test.go b/java/app_test.go index 6a4508cd6..991359915 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -427,7 +427,8 @@ func TestUpdatableApps_JniLibShouldBeBuiltAgainstMinSdkVersion(t *testing.T) { name: "libjni", stl: "none", system_shared_libs: [], - sdk_version: "29", + sdk_version: "current", + min_sdk_version: "29", } ` fs := map[string][]byte{ @@ -481,12 +482,13 @@ func TestUpdatableApps_ErrorIfJniLibDoesntSupportMinSdkVersion(t *testing.T) { name: "libjni", stl: "none", sdk_version: "current", + min_sdk_version: "current", } ` - testJavaError(t, `"libjni" .*: sdk_version\(current\) is higher than min_sdk_version\(29\)`, bp) + testJavaError(t, `"libjni" .*: min_sdk_version\(current\) is higher than min_sdk_version\(29\)`, bp) } -func TestUpdatableApps_ErrorIfDepSdkVersionIsHigher(t *testing.T) { +func TestUpdatableApps_ErrorIfDepMinSdkVersionIsHigher(t *testing.T) { bp := cc.GatherRequiredDepsForTest(android.Android) + ` android_app { name: "foo", @@ -503,6 +505,7 @@ func TestUpdatableApps_ErrorIfDepSdkVersionIsHigher(t *testing.T) { shared_libs: ["libbar"], system_shared_libs: [], sdk_version: "27", + min_sdk_version: "27", } cc_library { @@ -510,6 +513,7 @@ func TestUpdatableApps_ErrorIfDepSdkVersionIsHigher(t *testing.T) { stl: "none", system_shared_libs: [], sdk_version: "current", + min_sdk_version: "current", } ` testJavaError(t, `"libjni" .*: links "libbar" built against newer API version "current"`, bp) From 42e89508eeb7d24bac5eb71b48aa89fbf5ed8aee Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Fri, 6 May 2022 22:12:55 +0000 Subject: [PATCH 2/2] Enforce updatable=true on apps of updatable apexes - Update apex_info (a topdown mutator) so that it sets updatable=true on apps of updatable apexes - Write a unit test that tests different combinations of updatable/non-updatable apks-in-apexes - Update an existing unit test that asserts a different error Test: go test ./java Test: m nothing (in internal) Bug: 209409604 Change-Id: Ie8881b857afcec44addf27fc360c5b8abf726bd2 --- apex/apex.go | 19 +++++++++++++- apex/apex_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++ java/app.go | 4 +++ 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/apex/apex.go b/apex/apex.go index a7b0a4fe2..4bcba7e43 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -994,6 +994,7 @@ type ApexInfoMutator interface { // apexInfoMutator delegates the work of identifying which modules need an ApexInfo and apex // specific variant to modules that support the ApexInfoMutator. +// It also propagates updatable=true to apps of updatable apexes func apexInfoMutator(mctx android.TopDownMutatorContext) { if !mctx.Module().Enabled() { return @@ -1001,8 +1002,8 @@ func apexInfoMutator(mctx android.TopDownMutatorContext) { if a, ok := mctx.Module().(ApexInfoMutator); ok { a.ApexInfoMutator(mctx) - return } + enforceAppUpdatability(mctx) } // apexStrictUpdatibilityLintMutator propagates strict_updatability_linting to transitive deps of a mainline module @@ -1033,6 +1034,22 @@ func apexStrictUpdatibilityLintMutator(mctx android.TopDownMutatorContext) { } } +// enforceAppUpdatability propagates updatable=true to apps of updatable apexes +func enforceAppUpdatability(mctx android.TopDownMutatorContext) { + if !mctx.Module().Enabled() { + return + } + if apex, ok := mctx.Module().(*apexBundle); ok && apex.Updatable() { + // checking direct deps is sufficient since apex->apk is a direct edge, even when inherited via apex_defaults + mctx.VisitDirectDeps(func(module android.Module) { + // ignore android_test_app + if app, ok := module.(*java.AndroidApp); ok { + app.SetUpdatable(true) + } + }) + } +} + // TODO: b/215736885 Whittle the denylist // Transitive deps of certain mainline modules baseline NewApi errors // Skip these mainline modules for now diff --git a/apex/apex_test.go b/apex/apex_test.go index 77cbb58c3..bb3556c3c 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -2393,6 +2393,7 @@ func TestApexMinSdkVersion_ErrorIfDepIsNewer_Java(t *testing.T) { key: "myapex.key", apps: ["AppFoo"], min_sdk_version: "29", + updatable: false, } apex_key { @@ -9324,6 +9325,69 @@ func TestApexStrictUpdtabilityLintBcpFragmentDeps(t *testing.T) { } } +// updatable apexes should propagate updatable=true to its apps +func TestUpdatableApexEnforcesAppUpdatability(t *testing.T) { + bp := ` + apex { + name: "myapex", + key: "myapex.key", + updatable: %v, + apps: [ + "myapp", + ], + min_sdk_version: "30", + } + apex_key { + name: "myapex.key", + } + android_app { + name: "myapp", + updatable: %v, + apex_available: [ + "myapex", + ], + sdk_version: "current", + min_sdk_version: "30", + } + ` + testCases := []struct { + name string + apex_is_updatable_bp bool + app_is_updatable_bp bool + app_is_updatable_expected bool + }{ + { + name: "Non-updatable apex respects updatable property of non-updatable app", + apex_is_updatable_bp: false, + app_is_updatable_bp: false, + app_is_updatable_expected: false, + }, + { + name: "Non-updatable apex respects updatable property of updatable app", + apex_is_updatable_bp: false, + app_is_updatable_bp: true, + app_is_updatable_expected: true, + }, + { + name: "Updatable apex respects updatable property of updatable app", + apex_is_updatable_bp: true, + app_is_updatable_bp: true, + app_is_updatable_expected: true, + }, + { + name: "Updatable apex sets updatable=true on non-updatable app", + apex_is_updatable_bp: true, + app_is_updatable_bp: false, + app_is_updatable_expected: true, + }, + } + for _, testCase := range testCases { + result := testApex(t, fmt.Sprintf(bp, testCase.apex_is_updatable_bp, testCase.app_is_updatable_bp)) + myapp := result.ModuleForTests("myapp", "android_common").Module().(*java.AndroidApp) + android.AssertBoolEquals(t, testCase.name, testCase.app_is_updatable_expected, myapp.Updatable()) + } +} + func TestMain(m *testing.M) { os.Exit(m.Run()) } diff --git a/java/app.go b/java/app.go index 036b4b544..3b512f1d3 100755 --- a/java/app.go +++ b/java/app.go @@ -838,6 +838,10 @@ func (a *AndroidApp) Updatable() bool { return Bool(a.appProperties.Updatable) } +func (a *AndroidApp) SetUpdatable(val bool) { + a.appProperties.Updatable = &val +} + func (a *AndroidApp) getCertString(ctx android.BaseModuleContext) string { certificate, overridden := ctx.DeviceConfig().OverrideCertificateFor(ctx.ModuleName()) if overridden {