Re: [PATCH v2] x86: fix ktermios-termio conversion

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday 12 May 2009, Ingo Molnar wrote:
> Hm, that looks a bit ugly and it also adds 150 bytes of bloat to the 
> kernel:
> 
> drivers/char/tty_ioctl.o:
> 
>    text    data     bss     dec     hex filename
>    4704       0       0    4704    1260 tty_ioctl.o.before
>    4841       0       0    4841    12e9 tty_ioctl.o.after

It's easy to go back to a macro (or replace it by another inline
function if you prefer that stylistically) to make it look
nicer.

Regarding the bloat, I'm still looking for a better version.
The code I posted is basically what is in asm-generic/termios.h
traditionally, so at least I'm reasonably confident that it
is correct, while the x86 version currently fails to return
-EFAULT on incorrect pointers and misses c_line changes.

I now tried this version, which theoretically should be
fairly compact on most architectures:

static inline int set_low_termios_bits(unsigned int *termios,
			const short __user *termio)
{
	unsigned short tmp;
	int ret;

	ret = __get_user(tmp, termio);
	*termios = (0xffff0000 & *termios) | tmp;

	return ret;
}

/*
 * Translate a "termio" structure into a "termios". Ugh.
 */
static inline int user_termio_to_kernel_termios(struct ktermios *termios,
						const struct termio __user *termio)
{
	if (access_ok(VERIFY_READ, termio, sizeof (*termio)))
		return -EFAULT;

	return set_low_termios_bits(&termios->c_iflag, &termio->c_iflag) |
	       set_low_termios_bits(&termios->c_oflag, &termio->c_oflag) |
	       set_low_termios_bits(&termios->c_cflag, &termio->c_cflag) |
	       set_low_termios_bits(&termios->c_lflag, &termio->c_lflag) |
	       __get_user(termios->c_line, &termio->c_line) |
	       (__copy_from_user(termios->c_cc, termio->c_cc, NCC)
				? -EFAULT : 0);
}

/*
 * Translate a "termios" structure into a "termio". Ugh.
 */
static inline int kernel_termios_to_user_termio(struct termio __user *termio,
						struct ktermios *termios)
{
	if (access_ok(VERIFY_WRITE, termio, sizeof (*termio)))
		return -EFAULT;

	return __put_user(termios->c_iflag, &termio->c_iflag) |
	       __put_user(termios->c_oflag, &termio->c_oflag) |
	       __put_user(termios->c_cflag, &termio->c_cflag) |
	       __put_user(termios->c_lflag, &termio->c_lflag) |
	       __put_user(termios->c_line,  &termio->c_line) |
	       (__copy_to_user(termio->c_cc, termios->c_cc, NCC)
				? -EFAULT : 0);
}

Unfortunately, this is even more code on x86, where get_user/put_user
is out-of-line, while __get_user/__put_user is inline and produces
fixup code.
The best I could come up with is somewhat slower but also shorter:

static inline int set_low_termios_bits(unsigned int *termios,
			const short __user *termio)
{
	unsigned short tmp;
	int ret;

	ret = get_user(tmp, termio);
	*termios = (0xffff0000 & *termios) | tmp;

	return ret;
}

/*
 * Translate a "termio" structure into a "termios". Ugh.
 */
static inline int user_termio_to_kernel_termios(struct ktermios *termios,
						const struct termio __user *termio)
{
	return (set_low_termios_bits(&termios->c_iflag, &termio->c_iflag) ||
		set_low_termios_bits(&termios->c_oflag, &termio->c_oflag) ||
		set_low_termios_bits(&termios->c_cflag, &termio->c_cflag) ||
		set_low_termios_bits(&termios->c_lflag, &termio->c_lflag) ||
		get_user(termios->c_line, &termio->c_line) ||
		copy_from_user(termios->c_cc, termio->c_cc, NCC))
				? -EFAULT : 0;
}

/*
 * Translate a "termios" structure into a "termio". Ugh.
 */
static inline int kernel_termios_to_user_termio(struct termio __user *termio,
						struct ktermios *termios)
{
	return put_user(termios->c_iflag, &termio->c_iflag) ||
	       put_user(termios->c_oflag, &termio->c_oflag) ||
	       put_user(termios->c_cflag, &termio->c_cflag) ||
	       put_user(termios->c_lflag, &termio->c_lflag) ||
	       put_user(termios->c_line,  &termio->c_line) ||
	       (copy_to_user(termio->c_cc, termios->c_cc, NCC))
				? -EFAULT : 0;
}

If you think that looks better, I can send another patch. I have
not tested this code functionally though.

	Arnd <><