2007-09-12 02:04:50

by Michael Neuling

[permalink] [raw]
Subject: [PATCH] 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(+)

Index: linux-2.6-ozlabs/include/asm-powerpc/termios.h
===================================================================
--- linux-2.6-ozlabs.orig/include/asm-powerpc/termios.h
+++ 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 */


2007-09-12 02:18:18

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions



On Wed, 12 Sep 2007, Michael Neuling wrote:
>
> 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

2007-09-12 02:33:34

by Tony Breeds

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions

On Tue, Sep 11, 2007 at 07:17:42PM -0700, Linus Torvalds wrote:

> 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?

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(-)

diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
index 4a8969c..3ee73cf 100644
--- a/drivers/char/tty_ioctl.c
+++ b/drivers/char/tty_ioctl.c
@@ -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!

2007-09-12 02:34:20

by Michael Neuling

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions

> On Wed, 12 Sep 2007, Michael Neuling wrote:
> >
> > 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 in
clude | 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?

Not, I actually compiled it.

> 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


2007-09-12 05:16:59

by Geoff Levand

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions

Michael Neuling wrote:
> 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

2007-09-12 10:20:45

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions

On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:
> 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.

2007-09-12 11:01:54

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions

On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <[email protected]> wrote:

> On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:
> > 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?

diff -puN drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions drivers/char/tty_ioctl.c
--- a/drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions
+++ a/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:
{
_

2007-09-12 11:16:31

by Michael Neuling

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions

In message <[email protected]> you wrote:
> On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <[email protected]> wrote:
>
> > On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:
> > > 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

>
> diff -puN drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions
drivers/char/tty_ioctl.c
> --- a/drivers/char/tty_ioctl.c~powerpc-add-new-required-termio-functions
> +++ a/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 __us
er *)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_lo
cked, (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:
> {
> _
>

2007-09-12 11:34:34

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions

On Wed, Sep 12, 2007 at 04:01:09AM -0700, Andrew Morton wrote:
> On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <[email protected]> wrote:
>
> > On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:
> > > 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?


2007-09-12 11:53:32

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions

On Wed, Sep 12, 2007 at 12:34:09PM +0100, Christoph Hellwig wrote:
> On Wed, Sep 12, 2007 at 04:01:09AM -0700, Andrew Morton wrote:
> > On Wed, 12 Sep 2007 12:20:32 +0200 Heiko Carstens <[email protected]> wrote:
> >
> > > On Wed, Sep 12, 2007 at 12:04:39PM +1000, Michael Neuling wrote:
> > > > 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.

> 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?

2007-09-12 13:49:29

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions

On Wed, Sep 12, 2007 at 01:52:57PM +0200, Heiko Carstens wrote:
> > 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..

2007-09-14 14:22:39

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH] powerpc: add new required termio functions

On Wed, Sep 12, 2007 at 02:49:10PM +0100, Christoph Hellwig wrote:
> On Wed, Sep 12, 2007 at 01:52:57PM +0200, Heiko Carstens wrote:
> > > 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


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