[PATCH] hvc_console: prevent wrapping in hvc_console_print()

[PATCH] hvc_console: prevent wrapping in hvc_console_print()

[Roel Kluin]

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 <roel.kluin@gmail.com>
---
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);

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

[Andrew Morton]

On Tue, 17 Mar 2009 13:19:55 +0100
Roel Kluin <roel.kluin@gmail.com> 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 <roel.kluin@gmail.com>
> ---
> 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?

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

[Josh Boyer]

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

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

[Andrew Morton]

On Mon, 30 Mar 2009 16:55:53 -0400
Josh Boyer <jwboyer@linux.vnet.ibm.com> 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).

[PATCH] Add hvc_console to MAINTAINERS

[Josh Boyer]

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

Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
---

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:     linuxppc-dev@ozlabs.org
+S:     Maintained
+
 IBM MCA SCSI SUBSYSTEM DRIVER
 P:     Michael Lang
 M:     langa2@kph.uni-mainz.de

Re: [PATCH] Add hvc_console to MAINTAINERS

[Joe Perches]

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 <jwboyer@linux.vnet.ibm.com>
> ---
> 
> 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:     linuxppc-dev@ozlabs.org
> +S:     Maintained
> +
>  IBM MCA SCSI SUBSYSTEM DRIVER
>  P:     Michael Lang
>  M:     langa2@kph.uni-mainz.de

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:	linux-media@vger.kernel.org

A list entry could be something like:

	P:	Linux PCMCIA Team
	L:	linux-pcmcia@lists.infradead.org

Re: [PATCH] Add hvc_console to MAINTAINERS

[Andrew Morton]

On Tue, 31 Mar 2009 14:10:23 -0700
Joe Perches <joe@perches.com> 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 <jwboyer@linux.vnet.ibm.com>
> > ---
> > 
> > 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:     linuxppc-dev@ozlabs.org
> > +S:     Maintained
> > +
> >  IBM MCA SCSI SUBSYSTEM DRIVER
> >  P:     Michael Lang
> >  M:     langa2@kph.uni-mainz.de
> 
> 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:	linux-media@vger.kernel.org
> 
> A list entry could be something like:
> 
> 	P:	Linux PCMCIA Team
> 	L:	linux-pcmcia@lists.infradead.org
> 

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

Re: [PATCH] Add hvc_console to MAINTAINERS

[Joe Perches]

On Tue, 2009-03-31 at 14:22 -0700, Andrew Morton wrote:
> On Tue, 31 Mar 2009 14:10:23 -0700
> Joe Perches <joe@perches.com> 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:     linuxppc-dev@ozlabs.org
> > > +S:     Maintained
> > > +
> > >  IBM MCA SCSI SUBSYSTEM DRIVER
> > >  P:     Michael Lang
> > >  M:     langa2@kph.uni-mainz.de
> > 
> > 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:	linux-media@vger.kernel.org
> > 
> > A list entry could be something like:
> > 
> > 	P:	Linux PCMCIA Team
> > 	L:	linux-pcmcia@lists.infradead.org
> > 
> 
> 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.

Re: [PATCH] Add hvc_console to MAINTAINERS

[Josh Boyer]

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 <joe@perches.com> 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:     linuxppc-dev@ozlabs.org
>> > > +S:     Maintained
>> > > +
>> > >  IBM MCA SCSI SUBSYSTEM DRIVER
>> > >  P:     Michael Lang
>> > >  M:     langa2@kph.uni-mainz.de
>> > 
>> > 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:	linux-media@vger.kernel.org
>> > 
>> > A list entry could be something like:
>> > 
>> > 	P:	Linux PCMCIA Team
>> > 	L:	linux-pcmcia@lists.infradead.org
>> > 
>> 
>> 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

Re: [PATCH] Add hvc_console to MAINTAINERS

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]

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 <jwboyer@linux.vnet.ibm.com>
> ---
> 
> 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:     linuxppc-dev@ozlabs.org
> +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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Add hvc_console to MAINTAINERS

[Josh Boyer]

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 <jwboyer@linux.vnet.ibm.com>
>> ---
>> 
>> 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:     linuxppc-dev@ozlabs.org
>> +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

Re: [PATCH] Add hvc_console to MAINTAINERS

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]

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 <jwboyer@linux.vnet.ibm.com>
> >> ---
> >> 
> >> 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:     linuxppc-dev@ozlabs.org
> >> +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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Add hvc_console to MAINTAINERS

[Michael Ellerman]

Add a MAINTAINERS entry for the hypervisor virtual console driver.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 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:	lm-sensors@lm-sensors.org
 W:	http://www.kernel.org/pub/linux/kernel/people/fseidel/hdaps/
 S:	Maintained
 
+HYPERVISOR VIRTUAL CONSOLE DRIVER
+L:	linuxppc-dev@ozlabs.org
+L:	linux-kernel@vger.kernel.org
+S:	Odd Fixes
+F:	drivers/char/hvc_*
+
 GSPCA FINEPIX SUBDRIVER
 P:	Frank Zago
 M:	frank@zago.net
-- 
1.6.1.2

Re: Add hvc_console to MAINTAINERS

[Josh Boyer]

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 <michael@ellerman.id.au>

Acked-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>

>---
> 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:	lm-sensors@lm-sensors.org
> W:	http://www.kernel.org/pub/linux/kernel/people/fseidel/hdaps/
> S:	Maintained
>
>+HYPERVISOR VIRTUAL CONSOLE DRIVER
>+L:	linuxppc-dev@ozlabs.org
>+L:	linux-kernel@vger.kernel.org
>+S:	Odd Fixes
>+F:	drivers/char/hvc_*
>+
> GSPCA FINEPIX SUBDRIVER
> P:	Frank Zago
> M:	frank@zago.net
>-- 
>1.6.1.2
>
>
>