Commit graph

3960 commits

Author SHA1 Message Date
Treehugger Robot
e8d42e65c8 Merge changes I169b52cf,Ieb0e4e24
* changes:
  init: Add test for gentle_kill
  init: Add gentle_kill service property
2023-01-11 23:16:55 +00:00
Akilesh Kailash
717ee9120a Merge "init: Avoid killing all processes during reboot if update is in progress" 2023-01-11 20:52:13 +00:00
Treehugger Robot
7fcaa14e27 Merge "Add comment about out-of-bounds being on purpose" 2023-01-11 18:55:18 +00:00
Jooyung Han
6b88d1684c Parser::ParseConfigFile returns Result<void>
ParseApexConfigs() uses Parser::ParseConfigFile() to parse .rc files in
the target apex. ParseConfigFile() returning bool (with logging on
error) doesn't propagate the error message back to the callers
(including apexd or PackageManager).

We'd better to migrate other Parse*() methods of Parser class to return
Result<T>. But this change focuses on plumbing error progagation for
APEX configs.

Bug: 238820991
Test: atest CtsInitTestCases
Change-Id: Ifad97635dbb53a70053ec73a7a5b7e742466daf6
2023-01-11 17:20:16 +09:00
Akilesh Kailash
2c52d09078 init: Avoid killing all processes during reboot if update is in progress
Bug: 262321167
Test: Reboot device post OTA reboot
Change-Id: I5d13844e4e157d24a414dce668163bb1915bf65e
Signed-off-by: Akilesh Kailash <akailash@google.com>
2023-01-11 06:00:34 +00:00
Daniel Rosenberg
ed8178c8cc init: Add test for gentle_kill
Tests that a service marked as gentle_kill terminiated with SIGTERM

Bug: 249043036
Test: atest CtsInitTestCases:init#GentleKill
Change-Id: I169b52cfd74f07b4d2439761ea315117323b3fcb
2023-01-10 18:29:46 -08:00
Daniel Rosenberg
de76688e40 init: Add gentle_kill service property
If a service specifies gentle_kill, attempt to stop it will send SIGTERM
instead of SIGKILL. After 200ms, it will issue a SIGKILL.

Bug: 249043036
Test: atest CtsInitTestCases:init#GentleKill
      Added in next patch
Change-Id: Ieb0e4e24d31780aca1cf291f9d21d49cee181cf2
2023-01-10 18:29:46 -08:00
Florian Mayer
04fb2c838d Add comment about out-of-bounds being on purpose
Change-Id: I97e334f0555cce80eeab6567905054d807af3c91
2023-01-09 21:46:10 +00:00
Carlos Galo
1447120f78 Set memlock rlimit to 64KB
Defaulting Android to limit memlock to 64KB. This will help preventing
pages from being swapped until the app is killed it's memory will stay
resident. CTS test is enforced only in U+ devies.

Bug: 201797650
Test: Added new test to verify we are memlock at or under 64KB
Change-Id: I5a9e9da12f6df5a056ee47d0593c13e9c779e054
2023-01-05 16:29:59 +00:00
Bart Van Assche
f85317fb43 Make an error message more informative
From
https://android-build.googleplex.com/builds/tests/view?testResultId=TR66328435937757440&invocationId=I00700010119503421:

system/core/init/init_test.cpp:219: Failure
Failed
Value of: service-&gt;Start()
  Actual: createProcessGroup(0, 15611) failed for service 'console'
  Expected: is ok

The above error message does not contain enough information to
root-cause the test failure. Hence this CL that makes an error message
more informative.

Bug: 262090304
Change-Id: I09929b2f2aabf1eec4d90ec93234a9e968888da4
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-12-31 23:41:29 +00:00
Nikita Ioffe
24d1c238ce Revert "host_init_verifier: add check for root services and linux capabilities"
This reverts commit f1e3bfff40.

Reason for revert: Breaks builds

Bug: 263874232
Change-Id: Iabe5cd01114bf4a3731ace4345da8009e6b86f74
2022-12-28 16:18:20 +00:00
Nikita Ioffe
f1e3bfff40 host_init_verifier: add check for root services and linux capabilities
If a service that runs under root doesn't have the capabilities field in
it's definition, then it will inherit all the capabilities that init
has.

This change adds a linter to detect such services and ask developers to
explicitly specify capabilities that their service needs. If service
doesn't require any capabilities then empty capabilities fields should
be added in the service definition.

The actual access control list on what capabilities a process can use is
controlled by the SELinux, so inheriting all the init capabilities is
not a security issue here. However, asking services to explicitly
specify the capabilities they need is a good defense-in-depth mechanism.

So far this linter only checks the services on /system partition.

All currently offending services are added to the exempt list. I will
work on fixing some of them in the follow-up changes.

Bug: 249796710
Test: m dist
Change-Id: I2db06af165ae320a9c5086756067dceef20cd28d
2022-12-22 22:08:49 +00:00
Jiyong Park
3e951a7758 Merge "Skip StartConsole test on user builds." 2022-12-20 07:08:23 +00:00
Jiyong Park
5b7a51a59a Skip StartConsole test on user builds.
It runs a service with root privilege which can't be done on user
builds. Until the issue is resolved, skip the test on user builds.

Bug: 262090304
Test: N/A
Change-Id: I690ffbd7fdaef688a0c862e0c653e9b21e281ece
2022-12-20 10:46:31 +09:00
Treehugger Robot
fd98382506 Merge "Run the console test with /dev/null" 2022-12-16 01:54:14 +00:00
Nikita Ioffe
cc0e90a964 Update docs around capabilities and root processes
Current documentation is misleading: if a service runs as root and
doesn't specify any capabilities, then it will start with all the
capabilities (note that whether it can use them is controlled by
selinux).

Test: n/a
Bug: 249796710
Change-Id: I0d6a884127c6a6c5b651c1222fcf48322065daae
2022-12-15 18:04:21 +00:00
Jiyong Park
ae412804ca Run the console test with /dev/null
The availability of /dev/console varies across different devices and
even across different build variants (ex: userdebug/user). Instead of
relying on the device-specific condition, use the /dev/null as the
console device for the test.

Bug: 262090304
Test: atest CtsInitTestCases on raven-user
Change-Id: I3b12d66e94609328dfdd6f640d1adb88a773fa38
2022-12-15 16:32:38 +09:00
Jiyong Park
717ece7397 Merge "Clarify the behavior about event and property triggers" 2022-12-14 15:16:31 +00:00
Steven Moreland
bb1ee3c689 Merge "ignore error -> log" 2022-12-14 01:29:20 +00:00
Steven Moreland
507209ba55 ignore error -> log
Current code ignores an error, which is a code
rot risk.

Bug: 261700511
Change-Id: I04ca7046dc42d761ecaaf56f6100c96cc8298ec5
Test: N/A
2022-12-13 22:43:58 +00:00
David Anderson
95cfb31bd2 Merge "Support sysfs changes in the Linux 5.15 kernel." 2022-12-12 18:38:04 +00:00
Treehugger Robot
5c3e24816d Merge "Kill services even when cgroups is disabled" 2022-12-12 01:20:45 +00:00
Inseob Kim
a049a9928b Kill services even when cgroups is disabled
process_cgroup_empty_ is used to indicate that a service is already
killed or not. If cgroup support lacks, services cannot be killed
because process_cgroup_empty_ is always true.

This change fixes it by not assigning process_cgroup_empty_ as true.
Instead, make KillProcessGroup send signals even when cgroup is
disabled. Also DoKillProcessGroupOnce() is updated so it returns a number of killed processes, excluding already dead processes. This behavior agrees with its name (DoKillProcessOnce), and it prevents regression upon missing cgroups, because kill(-pgid) will always
"succeed" so KillProcessGroup will loop even when all processes are
already dead.

Bug: 257264124
Test: boot microdroid, see services are terminated
Change-Id: I19abf19ff1b70c666cd6f12d0a12956765174aaa
2022-12-12 01:19:26 +00:00
David Anderson
d6bf86b8cf Support sysfs changes in the Linux 5.15 kernel.
DM_DEV_CREATE no longer creates sysfs nodes. Note this in ueventd and
add some helper APIs to libdm, so devices can be created with a
placeholder table.

This also fixes a bug in dmctl where the detailed info on suspended
devices was wrong.

Bug: 259328366
Test: dmctl with "uevents" tool
Change-Id: I822f8010e48d32841aa0ee508822f76d03a3dd85
2022-12-08 16:22:47 -08:00
Chih-hung Hsieh
48cd41324b Merge "Fix potential memory leaks" 2022-12-07 02:36:27 +00:00
Bart Van Assche
d3484c84bf Merge "init: Enable ANDROID_BASE_UNIQUE_FD_DISABLE_IMPLICIT_CONVERSION" 2022-12-06 23:05:30 +00:00
Bart Van Assche
9dbf8c32e9 Merge "init: Prevent copying of Service objects" 2022-12-06 17:32:23 +00:00
Bart Van Assche
b3f9f6f4c6 Merge "init/Android.bp: Sort cflags alphabetically" 2022-12-06 17:29:41 +00:00
Treehugger Robot
70b22e1c5b Merge "init: Make an error message more informative" 2022-12-06 03:04:16 +00:00
Treehugger Robot
ab3bc215e8 Merge "init: Convert a single-element array into a scalar" 2022-12-06 01:48:58 +00:00
Chih-Hung Hsieh
784e63c9a2 Fix potential memory leaks
Bug: 259995529
Test: make tidy-system_subset
Change-Id: I604a308caf498a854b916dc86a8e274148c21ab0
2022-12-05 23:52:58 +00:00
Bart Van Assche
0bb4757d03 init: Prevent copying of Service objects
Service objects have external state (the child process) and hence must
not be duplicated. Disable the copy constructor and the assignment
operator to prevent that these objects get duplicated accidentally.

Bug: 213617178
Change-Id: Ia5391154b94eca7f12be69eabcdf3f173fc06452
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-12-05 11:41:26 -08:00
Bart Van Assche
42764c4e3e init: Make an error message more informative
Make it easier to diagnose service failures.

Bug: 213617178
Change-Id: I27135cb32b6a98b2fe24ab2324dffbf5b591fdd5
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-12-05 11:41:10 -08:00
Bart Van Assche
c41a4826d1 init: Do not invoke the ServiceList destructor when exiting
From the Google C++ style guide: "Objects with static storage duration
are forbidden unless they are trivially destructible." Hence this CL.

Bug: 213617178
Change-Id: I4c9a51618ee1eb14ed439295a5fc0101b940a63d
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-12-05 10:45:49 -08:00
Bart Van Assche
aee2ec8f1e init: Enable ANDROID_BASE_UNIQUE_FD_DISABLE_IMPLICIT_CONVERSION
From the unique_fd.h header file: "unique_fd's operator int is
dangerous, but we have way too much code that depends on it, so make
this opt-in at first."

From the Google C++ style guide: "Do not define implicit conversions."
See also go/cstyle#Implicit_Conversions.

Hence this CL that disables unique_fd::operator int().

Change-Id: I28d94755d5408f63e5819da8d1cbc285057f867f
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-12-05 09:40:06 -08:00
Bart Van Assche
91ba2197ef init/Android.bp: Sort cflags alphabetically
Sort the compiler flags alphabetically before adding a new macro
definition.

Change-Id: If919333302817406a890c9622396ec96920adf29
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-12-05 09:40:05 -08:00
Bart Van Assche
fdc0f89a4a init: Convert a single-element array into a scalar
Simplify PollIn() by converting a single-element array into a scalar. No
functionality is changed.

Change-Id: I3ef36b9c0daafeed3a92f90c7a7c4fe0654dd586
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-12-05 09:40:05 -08:00
Bart Van Assche
fa4926a9de Merge "Revert "init: Add more diagnostics for signalfd hangs."" 2022-12-01 17:50:27 +00:00
Jiyong Park
f477319692 Clarify the behavior about event and property triggers
See the comment threads at [1] for the rationale behind the behavior.

[1] https://android-review.git.corp.google.com/c/platform/system/core/+/161601/16/init/action.cpp#232

Bug: N/A
Test: N/A

Change-Id: I8b271073ec14ad9fa9add5df60577b34d1df3241
2022-12-01 10:38:48 +09:00
Bart Van Assche
3b21d95a0e init: Fix and re-enable the init#StartConsole test
Skip the test if /dev/console does not exist. Fix the console service
security label. Fix the getsid() test.

Bug: 260104465
Test: atest 'CtsInitTestCases:init#StartConsole' on a P2023 development board
Change-Id: If2533a3f205f922a9b04a748cb558a09c5925986
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-11-30 13:25:47 -08:00
Bart Van Assche
29d8a42d14 Revert "init: Add more diagnostics for signalfd hangs."
Revert commit 14f9c15e05 ("init: Add more diagnostics for signalfd
hangs") because:
* That commit was intented to help with root-causing b/223076262.
* The root cause of b/223076262 has been fixed (not blocking SIGCHLD
  in all threads in the init process).

Test: Treehugger
Change-Id: I586663ec0588e74a9d58512f7f31155398cf4f52
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-11-30 09:17:16 -08:00
Bart Van Assche
027b275b57 init: Disable test init#StartConsole
Bug: 260104465
Change-Id: I43a19b2451bad955d101d9ebdc3c98419d83597d
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-11-22 16:38:43 -08:00
Bart Van Assche
01e6669c66 init: Fix a race condition in KillProcessGroup()
Multiple tests in CtsInitTestCases, e.g. RebootTest#StopServicesSIGKILL,
can trigger the following race condition:
* A service is started. This involves calling fork() and also to call
  RunService() in the child process. RunService() calls setpgid().
* Service::Stop() is called and calls KillProcessGroup().
  KillProcessGroup() calls kill(-pgid, SIGKILL) before the child process
  has called setpgid(). pgid is the process ID of the child process. The
  kill() call fails because setpgid() has not yet been called.

Fix this race condition by adding a setpgid() call in the parent process
and by waiting from the parent until the child has called setsid() if a
console is attached.

Bug: 213617178
Change-Id: Ieb9e6908df725447e3695ed66bb8bd30e4e38aa9
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-11-21 11:42:44 -08:00
Bart Van Assche
987391656f init: Introduce RequiresConsole()
Prepare for adding more code in the parent process that depends on
whether or not a console is required.

Bug: 213617178
Change-Id: I066ede32fcd4ce09d06be23158f3c1970064a697
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-11-18 09:49:43 -08:00
Bart Van Assche
c8f34254b8 init: Introduce symbolic names for certain constants
Make the code easier to read by introducing symbolic names for the
constants used by Service::Start() for communication between the parent
and child processes.

Bug: 213617178
Change-Id: I3e735e149682fa9df2ed57f75eb5a67d7c68bd92
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-11-18 09:49:01 -08:00
Bart Van Assche
d394f74365 init: Reduce the number of system calls
Change setpgid(0, getpid()) into setpgid(0, 0). This patch removes one
system call but does not change the behavior of the code. From the
setpgid() man page: "If pgid is zero, then the PGID of the process
specified by pid is made the same as its process ID."

Bug: 213617178
Change-Id: I7031d9eb3711f526751da495c07a8927f9386d97
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-11-18 09:45:33 -08:00
Bart Van Assche
41787239ec Revert "init: Rename 'cgroups_activated' into 'fifo'"
Revert commit 9c61dad67e in preparation of
introducing a second interprocess communication channel.

Bug: 213617178
Change-Id: I2959a3902a1b994cca2ac99855be1fc60d63bcbb
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-11-18 09:42:14 -08:00
Bart Van Assche
bfcf4374ed init: Document which class Service members are not modified
It is nontrivial to derive from the implementation of class Service
which members are not modified. Hence this CL that documents this by
declaring these members 'const'.

Change-Id: I27b907a1c7044376d5c5393a29050c66cbdab7bf
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-11-18 09:42:14 -08:00
Bart Van Assche
5d18891e60 init: Add a unit test for the "start console" action
Treehugger starts Cuttlefish with the console disabled. Add a test that
enables the console. The purpose of this test is to trigger the code paths
in Service::Start() that are unique to processes associated with a
console.

Bug: 213617178
Change-Id: I834632ce6ec5c237c9c2c3f5b1aa7bc98c3ef260
Signed-off-by: Bart Van Assche <bvanassche@google.com>
2022-11-18 09:42:05 -08:00
Bart Van Assche
f26e59ebba Revert "init: Fix a race condition in KillProcessGroup()"
This reverts commit 15e5ecdcd7.

Reason for revert: breaks console support.
Bug: 213617178
Bug: 258754901
Change-Id: Iffe213e2cd295461a427621f2b84933f1bebd39f
2022-11-15 00:55:45 +00:00