hvc_console: prevent wrapping in hvc_console_print()

PATCH In review Roel Kluin 5 participants 14 messages Mar 17, 2009 – Apr 1, 2009 Char and misc drivers

Share & tools

Roel Kluin

author

2009-03-17 12:20 UTC #0 lore raw

PATCH 1/1 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]>

---

drivers/char/hvc_console.c +2 -2

index 94e7e3c..d06313c 100644
@@ -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);

Andrew Morton

2009-03-30 20:37 UTC #1 lore raw

Quoted 13 lines · 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]>
---
drivers/char/hvc_console.c +0 -0
index 94e7e3c..d06313c 100644
@@ -161,10 +161,10 @@ static void hvc_console_print(struct console *co, const char *b,
 			}
 		} else {

drivers/char/hvc_console.c static void hvc_console_print(struct console *co, const char *b, · line 170

Quoted 10 lines

drivers/char/hvc_console.c +2 -2

 			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?

Josh Boyer

2009-03-30 20:56 UTC #2 lore raw

Quoted 1 line · Andrew Morton

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

Andrew Morton

2009-03-30 21:22 UTC #3 lore raw

Quoted 7 lines · On Mon, 30 Mar 2009 16:55:53 -0400 Josh Boyer <[email protected]>…

Quoted 1 line · Andrew Morton
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).

Josh Boyer

2009-03-31 13:29 UTC #4 lore raw

PATCH 1/1 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]>

---

MAINTAINERS +5 -0

index c5f4e9d..387ad45 100644
@@ -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]

Joe Perches

2009-03-31 21:11 UTC #5 lore raw

Re: [PATCH] Add hvc_console to MAINTAINERS

Quoted 12 lines · Josh Boyer

Add a MAINTAINERS entry for the hypervisor virtual console support used on IBM POWER servers.
Signed-off-by Josh Boyer <[email protected]>
---
MAINTAINERS +0 -0
index c5f4e9d..387ad45 100644
@@ -2176,6 +2176,11 @@ W:       http://www.ia64-linux.org/
 T:     git kernel.org:/pub/scm/linux/kernel/git/aegl/linux-2.6.git

MAINTAINERS W: http://www.ia64-linux.org/ · line 2186

Quoted 10 lines

MAINTAINERS +5 -0

 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]

Andrew Morton

2009-03-31 21:25 UTC #6 lore raw

Re: [PATCH] Add hvc_console to MAINTAINERS

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

Quoted 14 lines · Josh Boyer
Add a MAINTAINERS entry for the hypervisor virtual console support used on IBM POWER servers.
Signed-off-by Josh Boyer <[email protected]>
---
MAINTAINERS +0 -0
index c5f4e9d..387ad45 100644
@@ -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
 

Quoted 11 lines

MAINTAINERS W: http://www.ia64-linux.org/ · line 2186
Quoted 8 lines
MAINTAINERS +5 -0
+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.

Quoted 12 lines

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

Joe Perches

2009-03-31 21:41 UTC #7 lore raw

Re: [PATCH] Add hvc_console to MAINTAINERS

Quoted 39 lines · Andrew Morton

Quoted 11 lines · On Tue, 31 Mar 2009 14:10:23 -0700 Joe Perches <[email protected]> wrote:
Quoted 10 lines · Josh Boyer
Add a MAINTAINERS entry for the hypervisor virtual console support used on IBM POWER servers.
MAINTAINERS +0 -0
index c5f4e9d..387ad45 100644
@@ -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
 
Quoted 11 lines
MAINTAINERS W: http://www.ia64-linux.org/ · line 2186
Quoted 8 lines
MAINTAINERS +5 -0
+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.
Quoted 7 lines
An exploder entry should be OK though.
For instance, the DVB subsystem:
DVB SUBSYSTEM AND DRIVERS
P: LinuxTV.org Project
M: [email protected]

Quoted 7 lines

Quoted 5 lines
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.

Josh Boyer

2009-03-31 23:53 UTC #8 lore raw

Re: [PATCH] Add hvc_console to MAINTAINERS

Quoted 48 lines · Joe Perches

Quoted 39 lines · Andrew Morton
Quoted 11 lines · On Tue, 31 Mar 2009 14:10:23 -0700 Joe Perches <[email protected]> wrote:
Quoted 10 lines · Josh Boyer
Add a MAINTAINERS entry for the hypervisor virtual console support used on IBM POWER servers.
MAINTAINERS +0 -0
index c5f4e9d..387ad45 100644
@@ -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
 
Quoted 11 lines
MAINTAINERS W: http://www.ia64-linux.org/ · line 2186
Quoted 8 lines
MAINTAINERS +5 -0
+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.
Quoted 7 lines
An exploder entry should be OK though.
For instance, the DVB subsystem:
DVB SUBSYSTEM AND DRIVERS
P: LinuxTV.org Project
M: [email protected]
Quoted 7 lines
Quoted 5 lines
A list entry could be something like:
P: Linux PCMCIA Team
L: [email protected]
I'm unclear on what the difference is here..

Quoted 8 lines

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

Quoted 5 lines

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

Michael Ellerman

2009-03-31 23:44 UTC #9 lore raw

Re: [PATCH] Add hvc_console to MAINTAINERS

In reply to Josh Boyer

Quoted 11 lines · Josh Boyer

Add a MAINTAINERS entry for the hypervisor virtual console support used on IBM POWER servers.
Signed-off-by Josh Boyer <[email protected]>
---
MAINTAINERS +0 -0
index c5f4e9d..387ad45 100644

MAINTAINERS W: http://www.ia64-linux.org/ · line 2182

Quoted 7 lines

MAINTAINERS +4 -0

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

Signature · 8 lines

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

Josh Boyer

2009-03-31 23:51 UTC #10 lore raw

Re: [PATCH] Add hvc_console to MAINTAINERS

Quoted 15 lines · Michael Ellerman

Quoted 14 lines · Josh Boyer
Add a MAINTAINERS entry for the hypervisor virtual console support used on IBM POWER servers.
Signed-off-by Josh Boyer <[email protected]>
---
MAINTAINERS +0 -0
index c5f4e9d..387ad45 100644
@@ -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
 

Quoted 7 lines

MAINTAINERS W: http://www.ia64-linux.org/ · line 2182
Quoted 4 lines
MAINTAINERS +4 -0
+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

Michael Ellerman

2009-04-01 00:04 UTC #11 lore raw

Re: [PATCH] Add hvc_console to MAINTAINERS

Quoted 24 lines · Josh Boyer

Quoted 15 lines · Michael Ellerman
Quoted 14 lines · Josh Boyer
Add a MAINTAINERS entry for the hypervisor virtual console support used on IBM POWER servers.
Signed-off-by Josh Boyer <[email protected]>
---
MAINTAINERS +0 -0
index c5f4e9d..387ad45 100644
@@ -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
 
Quoted 7 lines
MAINTAINERS W: http://www.ia64-linux.org/ · line 2182
Quoted 4 lines
MAINTAINERS +4 -0
+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.

Quoted 4 lines

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

Signature · 8 lines

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

Michael Ellerman

2009-04-01 00:20 UTC #12 lore raw

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?

MAINTAINERS +6 -0

index 1c2ca1d..854437a 100644
@@ -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]

Signature · 1 line

1.6.1.2

Josh Boyer

Acked-by

2009-04-01 00:34 UTC #13 lore raw

Re: Add hvc_console to MAINTAINERS

Quoted 3 lines · Michael Ellerman

Add a MAINTAINERS entry for the hypervisor virtual console driver.
Signed-off-by Michael Ellerman <[email protected]>

Acked-by Josh Boyer <[email protected]>

Quoted 28 lines

---
MAINTAINERS | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
How about this?
MAINTAINERS +0 -0
index 1c2ca1d..854437a 100644
@@ -1918,6 +1918,12 @@ L:	[email protected]
W: http://www.kernel.org/pub/linux/kernel/people/fseidel/hdaps/
S: Maintained
MAINTAINERS +6 -0
+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]
Signature · 1 line
1.6.1.2