2009-03-17 12:20:12

by Roel Kluin

[permalink] [raw]
Subject: [PATCH] hvc_console: prevent wrapping in hvc_console_print()

This was found by code analysis, is it needed?
------------------------------>8-------------8<---------------------------------
If we subtract too much on unsigned i it wraps.

Signed-off-by: Roel Kluin <[email protected]>
---
diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
index 94e7e3c..d06313c 100644
--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c
@@ -161,10 +161,10 @@ static void hvc_console_print(struct console *co, const char *b,
}
} else {
r = cons_ops[index]->put_chars(vtermnos[index], c, i);
- if (r <= 0) {
+ if (r <= 0 || r > i) {
/* throw away chars on error */
i = 0;
- } else if (r > 0) {
+ } else {
i -= r;
if (i > 0)
memmove(c, c+r, i);


2009-03-30 20:38:10

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] hvc_console: prevent wrapping in hvc_console_print()

On Tue, 17 Mar 2009 13:19:55 +0100
Roel Kluin <[email protected]> wrote:

> This was found by code analysis, is it needed?
> ------------------------------>8-------------8<---------------------------------
> If we subtract too much on unsigned i it wraps.
>
> Signed-off-by: Roel Kluin <[email protected]>
> ---
> diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
> index 94e7e3c..d06313c 100644
> --- a/drivers/char/hvc_console.c
> +++ b/drivers/char/hvc_console.c
> @@ -161,10 +161,10 @@ static void hvc_console_print(struct console *co, const char *b,
> }
> } else {
> r = cons_ops[index]->put_chars(vtermnos[index], c, i);
> - if (r <= 0) {
> + if (r <= 0 || r > i) {
> /* throw away chars on error */
> i = 0;
> - } else if (r > 0) {
> + } else {
> i -= r;
> if (i > 0)
> memmove(c, c+r, i);

I expect that ->put_chars() will either return a -ve errno or will
return the number of chars which were written, which will be less than
or equal to `i'.

Or maybe I miss your point.

Guys, could we have a MAINTAINERS entry for this driver please?

2009-03-30 20:56:22

by Josh Boyer

[permalink] [raw]
Subject: Re: [PATCH] hvc_console: prevent wrapping in hvc_console_print()

On Mon, Mar 30, 2009 at 01:35:31PM -0700, Andrew Morton wrote:
>Guys, could we have a MAINTAINERS entry for this driver please?

To my knowledge, there isn't one. Would be Ben now by default I
guess. The previous developers have disclaimed ownership for a
while from what I remember.

josh

2009-03-30 21:22:29

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] hvc_console: prevent wrapping in hvc_console_print()

On Mon, 30 Mar 2009 16:55:53 -0400
Josh Boyer <[email protected]> wrote:

> On Mon, Mar 30, 2009 at 01:35:31PM -0700, Andrew Morton wrote:
> >Guys, could we have a MAINTAINERS entry for this driver please?
>
> To my knowledge, there isn't one. Would be Ben now by default I
> guess. The previous developers have disclaimed ownership for a
> while from what I remember.
>

A MAINAINERS record doesn't have to identify an individual. It can at
least point people at the appropriate mailing list(s).

2009-03-31 13:30:06

by Josh Boyer

[permalink] [raw]
Subject: [PATCH] Add hvc_console to MAINTAINERS

Add a MAINTAINERS entry for the hypervisor virtual console support
used on IBM POWER servers.

Signed-off-by: Josh Boyer <[email protected]>
---

diff --git a/MAINTAINERS b/MAINTAINERS
index c5f4e9d..387ad45 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2176,6 +2176,11 @@ W: http://www.ia64-linux.org/
T: git kernel.org:/pub/scm/linux/kernel/git/aegl/linux-2.6.git
S: Maintained

+IBM HYPERVISOR VIRTUAL CONSOLE
+P: Several
+L: [email protected]
+S: Maintained
+
IBM MCA SCSI SUBSYSTEM DRIVER
P: Michael Lang
M: [email protected]

2009-03-31 21:12:47

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] Add hvc_console to MAINTAINERS

On Tue, 2009-03-31 at 09:29 -0400, Josh Boyer wrote:
> Add a MAINTAINERS entry for the hypervisor virtual console support
> used on IBM POWER servers.
>
> Signed-off-by: Josh Boyer <[email protected]>
> ---
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c5f4e9d..387ad45 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2176,6 +2176,11 @@ W: http://www.ia64-linux.org/
> T: git kernel.org:/pub/scm/linux/kernel/git/aegl/linux-2.6.git
> S: Maintained
>
> +IBM HYPERVISOR VIRTUAL CONSOLE
> +P: Several
> +L: [email protected]
> +S: Maintained
> +
> IBM MCA SCSI SUBSYSTEM DRIVER
> P: Michael Lang
> M: [email protected]

I think this sort of entry should not be acceptable.
Maintainers should have individual names and contact addresses.

An exploder entry should be OK though.
For instance, the DVB subsystem:

DVB SUBSYSTEM AND DRIVERS
P: LinuxTV.org Project
M: [email protected]

A list entry could be something like:

P: Linux PCMCIA Team
L: [email protected]

2009-03-31 21:26:35

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Add hvc_console to MAINTAINERS

On Tue, 31 Mar 2009 14:10:23 -0700
Joe Perches <[email protected]> wrote:

> On Tue, 2009-03-31 at 09:29 -0400, Josh Boyer wrote:
> > Add a MAINTAINERS entry for the hypervisor virtual console support
> > used on IBM POWER servers.
> >
> > Signed-off-by: Josh Boyer <[email protected]>
> > ---
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c5f4e9d..387ad45 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2176,6 +2176,11 @@ W: http://www.ia64-linux.org/
> > T: git kernel.org:/pub/scm/linux/kernel/git/aegl/linux-2.6.git
> > S: Maintained
> >
> > +IBM HYPERVISOR VIRTUAL CONSOLE
> > +P: Several
> > +L: [email protected]
> > +S: Maintained
> > +
> > IBM MCA SCSI SUBSYSTEM DRIVER
> > P: Michael Lang
> > M: [email protected]
>
> I think this sort of entry should not be acceptable.
> Maintainers should have individual names and contact addresses.

There isn't a maintainer.

But people send patches to the wrong list, so the ppc guys (who use
this driver) don't get to see them.

That's a practical, real-world problem. We should have a fix for it.

> An exploder entry should be OK though.
> For instance, the DVB subsystem:
>
> DVB SUBSYSTEM AND DRIVERS
> P: LinuxTV.org Project
> M: [email protected]
>
> A list entry could be something like:
>
> P: Linux PCMCIA Team
> L: [email protected]
>

I'm unclear on what the difference is here..

2009-03-31 21:42:07

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] Add hvc_console to MAINTAINERS

On Tue, 2009-03-31 at 14:22 -0700, Andrew Morton wrote:
> On Tue, 31 Mar 2009 14:10:23 -0700
> Joe Perches <[email protected]> wrote:
> > On Tue, 2009-03-31 at 09:29 -0400, Josh Boyer wrote:
> > > Add a MAINTAINERS entry for the hypervisor virtual console support
> > > used on IBM POWER servers.
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index c5f4e9d..387ad45 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -2176,6 +2176,11 @@ W: http://www.ia64-linux.org/
> > > T: git kernel.org:/pub/scm/linux/kernel/git/aegl/linux-2.6.git
> > > S: Maintained
> > >
> > > +IBM HYPERVISOR VIRTUAL CONSOLE
> > > +P: Several
> > > +L: [email protected]
> > > +S: Maintained
> > > +
> > > IBM MCA SCSI SUBSYSTEM DRIVER
> > > P: Michael Lang
> > > M: [email protected]
> >
> > I think this sort of entry should not be acceptable.
> > Maintainers should have individual names and contact addresses.
>
> There isn't a maintainer.
>
> But people send patches to the wrong list, so the ppc guys (who use
> this driver) don't get to see them.
>
> That's a practical, real-world problem. We should have a fix for it.
>
> > An exploder entry should be OK though.
> > For instance, the DVB subsystem:
> >
> > DVB SUBSYSTEM AND DRIVERS
> > P: LinuxTV.org Project
> > M: [email protected]
> >
> > A list entry could be something like:
> >
> > P: Linux PCMCIA Team
> > L: [email protected]
> >
>
> I'm unclear on what the difference is here..

I have no problem with the "L: list" entry.
I think that's good. Along with some file
matching pattern as well of course...

I think the "P: Several" entry is not good.

"Several" doesn't give any idea as to who
actually does the work.

If the "P:" entry is there at all, it should
be the list name.

Maybe "P: Linux PPC Development Team" or
something similar.

2009-03-31 23:44:25

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH] Add hvc_console to MAINTAINERS

On Tue, 2009-03-31 at 09:29 -0400, Josh Boyer wrote:
> Add a MAINTAINERS entry for the hypervisor virtual console support
> used on IBM POWER servers.
>
> Signed-off-by: Josh Boyer <[email protected]>
> ---
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c5f4e9d..387ad45 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2176,6 +2176,11 @@ W: http://www.ia64-linux.org/
> T: git kernel.org:/pub/scm/linux/kernel/git/aegl/linux-2.6.git
> S: Maintained
>
> +IBM HYPERVISOR VIRTUAL CONSOLE
> +P: Several
> +L: [email protected]
> +S: Maintained

I don't think it's the "IBM" Hypervisor virtual console driver anymore.
There are backends for Xen, Beat (Toshiba) and virtio.

cheers

--
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person


Attachments:
signature.asc (197.00 B)
This is a digitally signed message part

2009-03-31 23:51:54

by Josh Boyer

[permalink] [raw]
Subject: Re: [PATCH] Add hvc_console to MAINTAINERS

On Wed, Apr 01, 2009 at 10:44:00AM +1100, Michael Ellerman wrote:
>On Tue, 2009-03-31 at 09:29 -0400, Josh Boyer wrote:
>> Add a MAINTAINERS entry for the hypervisor virtual console support
>> used on IBM POWER servers.
>>
>> Signed-off-by: Josh Boyer <[email protected]>
>> ---
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index c5f4e9d..387ad45 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2176,6 +2176,11 @@ W: http://www.ia64-linux.org/
>> T: git kernel.org:/pub/scm/linux/kernel/git/aegl/linux-2.6.git
>> S: Maintained
>>
>> +IBM HYPERVISOR VIRTUAL CONSOLE
>> +P: Several
>> +L: [email protected]
>> +S: Maintained
>
>I don't think it's the "IBM" Hypervisor virtual console driver anymore.
>There are backends for Xen, Beat (Toshiba) and virtio.

Was just going by the copyright in the files, and Documentation/powerpc/hvc*

Also, if there are other backends, then is the list specified really
correct? Perhaps it should just be linux-kernel.

Guess that's the problem with trying to add a maintainers entry for something
that doesn't really have a specific maintainer.

josh

2009-03-31 23:53:49

by Josh Boyer

[permalink] [raw]
Subject: Re: [PATCH] Add hvc_console to MAINTAINERS

On Tue, Mar 31, 2009 at 02:40:58PM -0700, Joe Perches wrote:
>On Tue, 2009-03-31 at 14:22 -0700, Andrew Morton wrote:
>> On Tue, 31 Mar 2009 14:10:23 -0700
>> Joe Perches <[email protected]> wrote:
>> > On Tue, 2009-03-31 at 09:29 -0400, Josh Boyer wrote:
>> > > Add a MAINTAINERS entry for the hypervisor virtual console support
>> > > used on IBM POWER servers.
>> > > diff --git a/MAINTAINERS b/MAINTAINERS
>> > > index c5f4e9d..387ad45 100644
>> > > --- a/MAINTAINERS
>> > > +++ b/MAINTAINERS
>> > > @@ -2176,6 +2176,11 @@ W: http://www.ia64-linux.org/
>> > > T: git kernel.org:/pub/scm/linux/kernel/git/aegl/linux-2.6.git
>> > > S: Maintained
>> > >
>> > > +IBM HYPERVISOR VIRTUAL CONSOLE
>> > > +P: Several
>> > > +L: [email protected]
>> > > +S: Maintained
>> > > +
>> > > IBM MCA SCSI SUBSYSTEM DRIVER
>> > > P: Michael Lang
>> > > M: [email protected]
>> >
>> > I think this sort of entry should not be acceptable.
>> > Maintainers should have individual names and contact addresses.
>>
>> There isn't a maintainer.
>>
>> But people send patches to the wrong list, so the ppc guys (who use
>> this driver) don't get to see them.
>>
>> That's a practical, real-world problem. We should have a fix for it.
>>
>> > An exploder entry should be OK though.
>> > For instance, the DVB subsystem:
>> >
>> > DVB SUBSYSTEM AND DRIVERS
>> > P: LinuxTV.org Project
>> > M: [email protected]
>> >
>> > A list entry could be something like:
>> >
>> > P: Linux PCMCIA Team
>> > L: [email protected]
>> >
>>
>> I'm unclear on what the difference is here..
>
>I have no problem with the "L: list" entry.
>I think that's good. Along with some file
>matching pattern as well of course...
>
>I think the "P: Several" entry is not good.
>
>"Several" doesn't give any idea as to who
>actually does the work.

I copied that from something else in MAINTAINERS. I believe it
was the KERNEL JANITORS entry. I guess I should have known better
than to copy from that...

>If the "P:" entry is there at all, it should
>be the list name.
>
>Maybe "P: Linux PPC Development Team" or
>something similar.

Except that isn't right for the reasons Michael specified.

josh

2009-04-01 00:04:26

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH] Add hvc_console to MAINTAINERS

On Tue, 2009-03-31 at 19:50 -0400, Josh Boyer wrote:
> On Wed, Apr 01, 2009 at 10:44:00AM +1100, Michael Ellerman wrote:
> >On Tue, 2009-03-31 at 09:29 -0400, Josh Boyer wrote:
> >> Add a MAINTAINERS entry for the hypervisor virtual console support
> >> used on IBM POWER servers.
> >>
> >> Signed-off-by: Josh Boyer <[email protected]>
> >> ---
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index c5f4e9d..387ad45 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -2176,6 +2176,11 @@ W: http://www.ia64-linux.org/
> >> T: git kernel.org:/pub/scm/linux/kernel/git/aegl/linux-2.6.git
> >> S: Maintained
> >>
> >> +IBM HYPERVISOR VIRTUAL CONSOLE
> >> +P: Several
> >> +L: [email protected]
> >> +S: Maintained
> >
> >I don't think it's the "IBM" Hypervisor virtual console driver anymore.
> >There are backends for Xen, Beat (Toshiba) and virtio.
>
> Was just going by the copyright in the files, and Documentation/powerpc/hvc*
>
> Also, if there are other backends, then is the list specified really
> correct? Perhaps it should just be linux-kernel.

I would say /also/ linux-kernel.

I think there's still more folks on linux-ppc who have looked at the
driver than elsewhere, but cross-posting to lkml is probably good as
well.

cheers

--
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person


Attachments:
signature.asc (197.00 B)
This is a digitally signed message part

2009-04-01 00:21:48

by Michael Ellerman

[permalink] [raw]
Subject: Add hvc_console to MAINTAINERS

Add a MAINTAINERS entry for the hypervisor virtual console driver.

Signed-off-by: Michael Ellerman <[email protected]>
---
MAINTAINERS | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

How about this?

diff --git a/MAINTAINERS b/MAINTAINERS
index 1c2ca1d..854437a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1918,6 +1918,12 @@ L: [email protected]
W: http://www.kernel.org/pub/linux/kernel/people/fseidel/hdaps/
S: Maintained

+HYPERVISOR VIRTUAL CONSOLE DRIVER
+L: [email protected]
+L: [email protected]
+S: Odd Fixes
+F: drivers/char/hvc_*
+
GSPCA FINEPIX SUBDRIVER
P: Frank Zago
M: [email protected]
--
1.6.1.2


2009-04-01 00:34:20

by Josh Boyer

[permalink] [raw]
Subject: Re: Add hvc_console to MAINTAINERS

On Wed, Apr 01, 2009 at 11:20:50AM +1100, Michael Ellerman wrote:
>Add a MAINTAINERS entry for the hypervisor virtual console driver.
>
>Signed-off-by: Michael Ellerman <[email protected]>

Acked-by: Josh Boyer <[email protected]>

>---
> MAINTAINERS | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
>How about this?
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 1c2ca1d..854437a 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -1918,6 +1918,12 @@ L: [email protected]
> W: http://www.kernel.org/pub/linux/kernel/people/fseidel/hdaps/
> S: Maintained
>
>+HYPERVISOR VIRTUAL CONSOLE DRIVER
>+L: [email protected]
>+L: [email protected]
>+S: Odd Fixes
>+F: drivers/char/hvc_*
>+
> GSPCA FINEPIX SUBDRIVER
> P: Frank Zago
> M: [email protected]
>--
>1.6.1.2
>
>
>