Fixing Line Editing on Android 8.0+

Recently I’ve been working on a toy that launches an REPL in adb shell, which would be more comfortable to use if it has the line editing functionality similar to bash. After some research, I found that actually there are a number of line editing libraries available, so it should be a nice and easy addition. However, after some integration work, I was surprised to find that all these libraries (e.g. jline3, linenoise (which was also once imported into AOSP)) only worked on older versions of Android, that is, until Android 8.0.

Identifying the issue

So I created a small sample app with linenoise and debugged it to pinpoint the issue. Strangely, I found that it was the following line that failed:

/* put terminal in raw mode after flushing */
if (tcsetattr(fd,TCSAFLUSH,&raw) < 0) goto fatal;

Which returned -1 with errno set to EACCES (13). But why would this terminal configuration fail anyway?

So I took a look at man tcsetattr:

tcsetattr() sets the parameters associated with the terminal (unless support is required from the underlying hardware that is not available) from the termios structure referred to by termios_p. optional_actions specifies when the changes take effect:

TCSANOW

the change occurs immediately.

TCSADRAIN

the change occurs after all output written to fd has been transmitted. This function should be used when changing parameters that affect output.

TCSAFLUSH

the change occurs after all output written to the object referred by fd has been transmitted, and all input that has been received but not read will be discarded before the change is made.

The manual page didn’t contain any information for specific error codes, so it wasn’t very helpful for understanding why we’ve got EACCES. Maybe we’ve lost some kind of access to the TTY device since Android 8.0+? However, the default Android shell mksh (used by adb shell) still has working line editing, and how that was done became another mystery to me.

So I began searching for tcsetattr Android, and found that the Termux people also noticed the same issue years ago — the EACCES was actually a SELinux denial. And as a workaround, they were redefining TCSAFLUSH to be TCSANOW, but that isn’t functionally equivalent and may be subject to subtle breakages. So I decided to find out what was actually going wrong in SELinux.

My next step was to figure out why line editing is still working for mksh — is there any similar patch in AOSP, and how did AOSP do it? After some code search, I found out that mksh was actually using TCSADRAIN instead of TCSAFLUSH since a long time ago, so it seems both TCSANOW and TCSADRAIN are working, and it’s just TCSAFLUSH became broken on Android 8.0+. However, TCSAFLUSH still sounds more robust and is still being used in a number of places in Android, notably toybox‘s stty implementation (which broke jline3). So I think it’s still worth fixing TCSAFLUSH.

Debugging the SELinux denial

To debug the SELinux denial, I looked at the SELinux logcat message on an Android 11 emulator:

type=1400 audit(0.0:763): avc: denied { ioctl } for path="/dev/pts/0" dev="devpts" ino=3 ioctlcmd=0x5404 scontext=u:r:shell:s0 tcontext=u:object_r:devpts:s0 tclass=chr_file permissive=0

It is clear that tcsetattr is actually a wrapper over an ioctl operation on the TTY device, and Android do have SELinux rules over ioctl. So I looked for devpts and ioctl inside system/sepolicy, and found the macro unpriv_tty_ioctls in ioctl_macros:

# commonly used TTY ioctls
# merge with unpriv_unix_sock_ioctls?
define(`unpriv_tty_ioctls', `{
  TIOCOUTQ FIOCLEX FIONCLEX TCGETS TCSETS TIOCGWINSZ TIOCSWINSZ TIOCSCTTY
  TCSETSW TCFLSH TIOCSPGRP TIOCGPGRP
}')

Which is used in domain.te:

# Restrict PTYs to only allowed ioctls.
# Note that granting this allowlist to domain does
# not grant the wider ioctl permission. That must be granted
# separately.
allowxperm domain devpts:chr_file ioctl unpriv_tty_ioctls;

So I took a look at man tty_ioctl:

TCSETS

const struct termios *argp

Equivalent to tcsetattr(fd, TCSANOW, argp).

Set the current serial port settings.

TCSETSW

const struct termios *argp

Equivalent to tcsetattr(fd, TCSADRAIN, argp).

Allow the output buffer to drain, and set the current serial port settings.

TCSETSF

const struct termios *argp

Equivalent to tcsetattr(fd, TCSAFLUSH, argp).

Allow the output buffer to drain, discard pending input, and set the current serial port settings.

Notably, the macro unpriv_tty_ioctls included TCSETS and TCSETSW, but not TCSETSF. So it seems it’s indeed the missing TCSETSF that caused this SELinux denial.

Fixing the SELinux policy

Now that we have the root cause, can we fix it by simply adding TCSETSF to the macro unpriv_tty_ioctls, or was there any other concerns about it so that it was deliberately left out?

So I looked into git history of the file. Actually, the variant TCSETSW was the initial one added into unpriv_tty_ioctls in aosp/306278, and the base TCSETS was added later by aosp/310920. Looking at the initial change aosp/306278, the primary reason to restrict ioctl for TTY devices was to mitigate the security exploit of TIOCSTI, so it indeed seems that adding TCSETSF as another variant of TCSETS and TCSETSW would be fine, and should fix all the code relying on it.

Testing and submitting the fix

Then I started testing my fix locally. As I’m working on Android already, I naturally starting tested on an eng build, however soon I was surprised to find that my sample app runs perfectly even without my fix. Was it somehow patched recently? However since I’ve been looking at the latest version of the source code anyway, was my entire theory going in the wrong direction? I became deeply confused, until I suddenly realized the special thing about eng build — adb shell has root access.

So I immediately checked the SELinux context of the shell with ps -AZ | grep sh, and the output was u:r:su:s0, so actually this wouldn’t be the usual SELinux context on consumer devices because the shell process can’t be in the su domain. The su domain has a lot of privileges, and no wonder my sample app was working fine inside it. I became relieved that I’m not dealing with something crazy, and began testing on a user build instead. This time the sample app failed without the fix, and worked when my fix was applied, so this was finally confirmed to be the correct fix. Yay!

The remaining work was uploading the patch as aosp/1491378, getting it persubmit verified and code reviewed, and finally submitting the change. This concluded my small weekend journey into SELinux, and the TCSAFLUSH issue will become fixed with the Android S release. However since there’s no way to backport the fix, we still have to stick with TCSADRAIN for Android 8.0–11 compatibility for some years.