powerpc: add new required termio functions

PATCH In review Michael Neuling 8 participants 12 messages Sep 12, 2007 – Sep 14, 2007 Char and misc drivers

Share & tools

Michael Neuling

author

2007-09-12 02:04 UTC #0 lore raw

PATCH 1/1 powerpc: add new required termio functions

The "tty: termios locking functions break with new termios type" patch (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile. This adds the required API to asm-powerpc.

Signed-off-by Michael Neuling <[email protected]>

--
This needs to go up for 2.6.23.

Should we really put these definitions in asm-generic/termios.h as I'm guessing other architectures are broken too?

coopers@include/ % git grep kernel_termios_to_user_termios_1 asm-arm/termios.h:#define kernel_termios_to_user_termios_1(u, k) asm-cris/termios.h:#define kernel_termios_to_user_termios_1(u, k) asm-h8300/termios.h:#define kernel_termios_to_user_termios_1(u, k) asm-i386/termios.h:#define kernel_termios_to_user_termios_1(u, k) asm-ia64/termios.h:#define kernel_termios_to_user_termios_1(u, k) asm-m32r/termios.h:#define kernel_termios_to_user_termios_1(u, k) asm-m68k/termios.h:#define kernel_termios_to_user_termios_1(u, k) asm-mips/termios.h:#define kernel_termios_to_user_termios_1(u, k) asm-v850/termios.h:#define kernel_termios_to_user_termios_1(u, k) asm-x86_64/termios.h:#define kernel_termios_to_user_termios_1(u, k)

 include/asm-powerpc/termios.h |    3 +++
 1 file changed, 3 insertions(+)

include/asm-powerpc/termios.h +3 -0

Index: linux-2.6-ozlabs/include/asm-powerpc/termios.h
===================================================================
@@ -80,6 +80,9 @@ struct termio {
 
 #include <asm-generic/termios.h>
 
+#define user_termios_to_kernel_termios_1(k, u) copy_from_user(k, u, sizeof(struct termios))
+#define kernel_termios_to_user_termios_1(u, k) copy_to_user(u, k, sizeof(struct termios))
+
 #endif	/* __KERNEL__ */
 
 #endif	/* _ASM_POWERPC_TERMIOS_H */

Linus Torvalds

2007-09-12 02:17 UTC #1 lore raw

Quoted 3 lines · Michael Neuling

The "tty: termios locking functions break with new termios type" patch (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.

Really?

It shouldn't. The use of kernel_termios_to_user_termios_1() is conditional on the architecture having a define for TCGETS2, and I think they match up. I see:

	[torvalds@woody linux]$ git grep -l kernel_termios_to_user_termios_1 include | wc -l
	10
	[torvalds@woody linux]$ git grep -l TCGETS2 include | wc -l
	10

and in neither case is ppc in that list of architecures.

So maybe you just read the patch without actually testing whether it actually broke powerpc?

Or is something subtler going on?

Linus

Tony Breeds

2007-09-12 02:33 UTC #2 lore raw

Quoted 11 lines · Linus Torvalds

Really?
It shouldn't. The use of kernel_termios_to_user_termios_1() is conditional on the architecture having a define for TCGETS2, and I think they match up. I see:
	[torvalds@woody linux]$ git grep -l kernel_termios_to_user_termios_1 include | wc -l
	10
	[torvalds@woody linux]$ git grep -l TCGETS2 include | wc -l
	10

Quoted 6 lines

and in neither case is ppc in that list of architecures.
So maybe you just read the patch without actually testing whether it actually broke powerpc?
Or is something subtler going on?

As far as I can see TIOCSLCKTRMIOS and TIOCGLCKTRMIOS aren't protected by TCGETS2 guards. Do they need to be ... Perhaps

From: Tony Breeds <[email protected]>

Add Guards around TIOCSLCKTRMIOS and TIOCGLCKTRMIOS.

Signed-off-by Tony Breeds <[email protected]>

---

 drivers/char/tty_ioctl.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

drivers/char/tty_ioctl.c +14 -0

index 4a8969c..3ee73cf 100644
@@ -795,6 +795,19 @@ int n_tty_ioctl(struct tty_struct * tty, struct file * file,
 			if (L_ICANON(tty))
 				retval = inq_canon(tty);
 			return put_user(retval, (unsigned int __user *) arg);
+#ifndef TCGETS2
+		case TIOCGLCKTRMIOS:
+			if (kernel_termios_to_user_termios((struct termios __user *)arg, real_tty->termios_locked))
+				return -EFAULT;
+			return 0;
+
+		case TIOCSLCKTRMIOS:
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
+			if (user_termios_to_kernel_termios(real_tty->termios_locked, (struct termios __user *) arg))
+				return -EFAULT;
+			return 0;
+#else
 		case TIOCGLCKTRMIOS:
 			if (kernel_termios_to_user_termios_1((struct termios __user *)arg, real_tty->termios_locked))
 				return -EFAULT;

@@ -806,6 +819,7 @@ int n_tty_ioctl(struct tty_struct * tty, struct file * file,
 			if (user_termios_to_kernel_termios_1(real_tty->termios_locked, (struct termios __user *) arg))
 				return -EFAULT;
 			return 0;
+#endif
 
 		case TIOCPKT:
 		{

Yours Tony

linux.conf.au http://linux.conf.au/ || http://lca2008.linux.org.au/
Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

Michael Neuling

author

2007-09-12 02:34 UTC #3 lore raw

In reply to Linus Torvalds

Quoted 5 lines · Michael Neuling

Quoted 3 lines
The "tty: termios locking functions break with new termios type" patch (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile.

Quoted 7 lines

Really?
It shouldn't. The use of kernel_termios_to_user_termios_1() is conditional on the architecture having a define for TCGETS2, and I think they match up. I see:
[torvalds@woody linux]$ git grep -l kernel_termios_to_user_termios_1 in

clude | wc -l

Quoted 8 lines

	10
	[torvalds@woody linux]$ git grep -l TCGETS2 include | wc -l
	10
and in neither case is ppc in that list of architecures.
So maybe you just read the patch without actually testing whether it actually broke powerpc?

Not, I actually compiled it.

Quoted 1 line

Or is something subtler going on?

Looks like those new calls are not protected by the TCGETS2 define. Adding those ifdefs seems like the correct fix.

Mikey

Geoff Levand

2007-09-12 02:19 UTC #4 lore raw

In reply to Michael Neuling

Quoted 10 lines · Michael Neuling

The "tty: termios locking functions break with new termios type" patch (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile. This adds the required API to asm-powerpc.
Signed-off-by Michael Neuling <[email protected]>
--
This needs to go up for 2.6.23.
Should we really put these definitions in asm-generic/termios.h as I'm guessing other architectures are broken too?

I think it would be better to do so, as that is where we pickup the defs for the original kernel_termios_to_user_termios and user_termios_to_kernel_termios.

-Geoff

Heiko Carstens

2007-09-12 10:20 UTC #5 lore raw

In reply to Michael Neuling

Quoted 4 lines · Michael Neuling

The "tty: termios locking functions break with new termios type" patch (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile. [...]
I'm guessing other architectures are broken too?

FWIW, the above quoted patch breaks s390 as well.

Andrew Morton

2007-09-12 11:01 UTC #6 lore raw

Quoted 7 lines · On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <[email protected]>…

Quoted 4 lines · Michael Neuling
The "tty: termios locking functions break with new termios type" patch (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile. [...]
I'm guessing other architectures are broken too?
FWIW, the above quoted patch breaks s390 as well.

Does this fix it?

drivers/char/tty_ioctl.c renamed from drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions +14 -0

diff -puN drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions drivers/char/tty_ioctl.c
@@ -795,6 +795,19 @@ int n_tty_ioctl(struct tty_struct * tty,
 			if (L_ICANON(tty))
 				retval = inq_canon(tty);
 			return put_user(retval, (unsigned int __user *) arg);
+#ifndef TCGETS2
+		case TIOCGLCKTRMIOS:
+			if (kernel_termios_to_user_termios((struct termios __user *)arg, real_tty->termios_locked))
+				return -EFAULT;
+			return 0;
+
+		case TIOCSLCKTRMIOS:
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
+			if (user_termios_to_kernel_termios(real_tty->termios_locked, (struct termios __user *) arg))
+				return -EFAULT;
+			return 0;
+#else
 		case TIOCGLCKTRMIOS:
 			if (kernel_termios_to_user_termios_1((struct termios __user *)arg, real_tty->termios_locked))
 				return -EFAULT;

@@ -806,6 +819,7 @@ int n_tty_ioctl(struct tty_struct * tty,
 			if (user_termios_to_kernel_termios_1(real_tty->termios_locked, (struct termios __user *) arg))
 				return -EFAULT;
 			return 0;
+#endif
 
 		case TIOCPKT:
 		{

Michael Neuling

author

2007-09-12 11:16 UTC #7 lore raw

Quoted 11 lines · In message <[email protected]> you wrote:

Quoted 7 lines · On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <[email protected]>…
Quoted 4 lines · Michael Neuling
The "tty: termios locking functions break with new termios type" patch (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile. [...]
I'm guessing other architectures are broken too?
FWIW, the above quoted patch breaks s390 as well.
Does this fix it?

FYI it does fix powerpc, but I suspect you were asking about s390 here.

Mikey

Quoted 2 lines

diff -puN drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions

drivers/char/tty_ioctl.c

drivers/char/tty_ioctl.c int n_tty_ioctl(struct tty_struct * tty, · line 800

Quoted 9 lines

drivers/char/tty_ioctl.c renamed from drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions +3 -0

@@ -795,6 +795,19 @@ int n_tty_ioctl(struct tty_struct * tty,
 			if (L_ICANON(tty))
 				retval = inq_canon(tty);
 			return put_user(retval, (unsigned int __user *) arg);
+#ifndef TCGETS2
+		case TIOCGLCKTRMIOS:
+			if (kernel_termios_to_user_termios((struct termios __us

er *)arg, real_tty->termios_locked))

drivers/char/tty_ioctl.c int n_tty_ioctl(struct tty_struct * tty,

Quoted 7 lines

drivers/char/tty_ioctl.c renamed from drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions +7 -0
+				return -EFAULT;
+			return 0;
+
+		case TIOCSLCKTRMIOS:
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
+			if (user_termios_to_kernel_termios(real_tty->termios_lo

cked, (struct termios __user *) arg))

drivers/char/tty_ioctl.c int n_tty_ioctl(struct tty_struct * tty,

Quoted 5 lines

drivers/char/tty_ioctl.c renamed from drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions +3 -0
+				return -EFAULT;
+			return 0;
+#else
 		case TIOCGLCKTRMIOS:
 			if (kernel_termios_to_user_termios_1((struct termios __

user *)arg, real_tty->termios_locked))

drivers/char/tty_ioctl.c int n_tty_ioctl(struct tty_struct * tty, · line 819

Quoted 3 lines

drivers/char/tty_ioctl.c renamed from drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions +0 -0
 				return -EFAULT;
@@ -806,6 +819,7 @@ int n_tty_ioctl(struct tty_struct * tty,
 			if (user_termios_to_kernel_termios_1(real_tty->termios_

locked, (struct termios __user *) arg))

Quoted 8 lines

drivers/char/tty_ioctl.c renamed from drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions +1 -0
 				return -EFAULT;
 			return 0;
+#endif
 
 		case TIOCPKT:
 		{
_

Christoph Hellwig

2007-09-12 11:34 UTC #8 lore raw

In reply to Andrew Morton

Quoted 11 lines · Andrew Morton

Quoted 7 lines · On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <[email protected]>…
Quoted 4 lines · Michael Neuling
The "tty: termios locking functions break with new termios type" patch (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile. [...]
I'm guessing other architectures are broken too?
FWIW, the above quoted patch breaks s390 as well.
Does this fix it?

I might be missing something, but the the right fix is probably to apply the arch patches from Alan to powerpc and s390. We don't want to be left over without all the nice termios features on these platforms, do we?

Heiko Carstens

2007-09-12 11:53 UTC #9 lore raw

Quoted 4 lines · Christoph Hellwig

Quoted 3 lines · Andrew Morton
Quoted 1 line · On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <[email protected]>…
On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:

Quoted 8 lines

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
The "tty: termios locking functions break with new termios type" patch (f629307c857c030d5a3dd777fee37c8bb395e171) breaks the powerpc compile. [...]
I'm guessing other architectures are broken too?
FWIW, the above quoted patch breaks s390 as well.
Does this fix it?

Yes, it does.

Quoted 4 lines

I might be missing something, but the the right fix is probably to apply the arch patches from Alan to powerpc and s390. We don't want to be left over without all the nice termios features on these platforms, do we?

But not in rc6 timeframe, I would guess?

Christoph Hellwig

2007-09-12 13:49 UTC #10 lore raw

Quoted 6 lines · Heiko Carstens

Quoted 4 lines
I might be missing something, but the the right fix is probably to apply the arch patches from Alan to powerpc and s390. We don't want to be left over without all the nice termios features on these platforms, do we?
But not in rc6 timeframe, I would guess?

Ask Alan..

Alan Cox

2007-09-14 14:22 UTC #11 lore raw

Quoted 9 lines · Christoph Hellwig

Quoted 6 lines · Heiko Carstens
Quoted 4 lines
I might be missing something, but the the right fix is probably to apply the arch patches from Alan to powerpc and s390. We don't want to be left over without all the nice termios features on these platforms, do we?
But not in rc6 timeframe, I would guess?
Ask Alan..

Well i've been chasing people for many months on the 390 side so if they missed a release tough ...

Alan

Signature · 3 lines

--
		"A wiki is where ideas go to die"
				-- Becky Hogge