Bug#504721: Possible reason for serial console misdetection
On Tue, Dec 02, 2008 at 10:50:26PM +0100, Frans Pop wrote:
Someone who can actually write in C please check the code and let me know
if anything can be removed or is missing. I may well have added unneeded
includes for example and I have no idea what the original sprintf
statements were supposed to do.
Attached is a patch with a trimmed down version of console-type.c.
As we are not interested in the settings of the serial line or by the
state of the VT, we can just omit the struct declarations and be fine
with a dummy buffer. As printf() and ioctl() are the only functions
used, we can trim down the #include's to sys/ioctl.h and
stdio.h.
Cheers,
--
Jérémy Bobbio.''`.
[EMAIL PROTECTED]: :Ⓐ : # apt-get install anarchism
`. `'`
`-
diff --git a/packages/rootskel/src/sbin/Makefile b/packages/rootskel/src/sbin/Makefile
index b5e374e..83fb4bc 100644
--- a/packages/rootskel/src/sbin/Makefile
+++ b/packages/rootskel/src/sbin/Makefile
@@ -6,14 +6,18 @@ files_exec = \
shutdown \
init \
reopen-console \
+ console-type \
steal-ctty
+console-type: console-type.c
+ gcc -Os -Wall console-type.c -o console-type
+
steal-ctty: steal-ctty.c
gcc -Os -Wall steal-ctty.c -o steal-ctty
-build: steal-ctty
+build: console-type steal-ctty
clean:
- rm -f steal-ctty
+ rm -f console-type steal-ctty
include ../../Makefile.inc
diff --git a/packages/rootskel/src/sbin/console-type.c b/packages/rootskel/src/sbin/console-type.c
new file mode 100644
index 000..6105a3e
--- /dev/null
+++ b/packages/rootskel/src/sbin/console-type.c
@@ -0,0 +1,30 @@
+/* vi: set sw=4 ts=4: */
+/*
+ *
+ * Licensed under GPLv2
+ *
+ * Adapted for Debian Installer by Frans Pop fjp.debian.org from
+ * cttyhack from busybox 1.11, which is
+ *
+ * Copyright (c) 2007 Denys Vlasenko [EMAIL PROTECTED]
+ */
+
+#include sys/ioctl.h
+#include stdio.h
+
+enum { VT_GETSTATE = 0x5603 }; /* get global vt state info */
+
+int main(int argc, char ** argv)
+{
+ char buffer[256]; /* filled by ioctl */
+
+ if (ioctl(0, TIOCGSERIAL, buffer) == 0) {
+ /* this is a serial console */
+ printf(serial\n);
+ } else if (ioctl(0, VT_GETSTATE, buffer) == 0) {
+ /* this is linux virtual tty */
+ printf(virtual\n);
+ }
+
+ return 0;
+}
diff --git a/packages/rootskel/src/sbin/reopen-console b/packages/rootskel/src/sbin/reopen-console
index c6c55b5..702ddc3 100755
--- a/packages/rootskel/src/sbin/reopen-console
+++ b/packages/rootskel/src/sbin/reopen-console
@@ -6,11 +6,18 @@
NL=
+console=
if ! [ -f /var/run/console-device ]; then
- # If the kernel emitted an handover message, then it's the one
+ # If the kernel emitted a handover message, then it's the one
console=$(dmesg -s 65535 |
sed -n -e 's/.*\] console handover: boot \[.*\] - real \[\(.*\)\]$/\1/p')
+ # Except if it is the wrong type...
+ if [ $console ] [ $(console-type) = serial ] \
+ expr $console : tty[0-9] /dev/null; then
+ console=
+ fi
+
consoles=
if [ -z $console ]; then
# Retrieve all enabled consoles from boot log; ignore those
signature.asc
Description: Digital signature
Bug#504721: Possible reason for serial console misdetection
Jérémy Bobbio [EMAIL PROTECTED] writes: On Tue, Dec 02, 2008 at 10:50:26PM +0100, Frans Pop wrote: As we are not interested in the settings of the serial line or by the state of the VT, we can just omit the struct declarations and be fine with a dummy buffer. Don't you risk overflowing the buffer by not using a union of the two structs? Or are both guarranteed to never grow above 256 bytes? -- Feri. -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#504721: Possible reason for serial console misdetection
On Wednesday 03 December 2008, Ferenc Wagner wrote: Don't you risk overflowing the buffer by not using a union of the two structs? Or are both guarranteed to never grow above 256 bytes? The original code looks to have included a paranoia field in struct u of 3 times the size of serial_struct, probably for that reason. I'd think using 512 or even 1024 as buffer size may indeed be safer. Jérémy? -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#504721: Possible reason for serial console misdetection
tags 504721 patch
thanks
On Wednesday 26 November 2008, Jérémy Bobbio wrote:
So, the problem is that reopen-console gives preference to the value
in 'console handover' line, which is incorrect on sparc (refers to a
real terminal console even if one is connecting through serial). If
it is obvious, that sparc is unique that way, and it works correctly
on other arches, the following (untested) patch might do the trick:
[…]
In order to implement this, I have tried to understand the kernel logic
by reading at the relevant part of its source code as I have not been
able to find any relevant documentation about all this.
I've done some testing today on my Sparc Ultra 10 and can simply reproduce
the problem by booting it with no keyboard connected, which means it will
default to serial console.
If I do that I get kernel boot messages on serial console, but the D-I
frontend is shown on the display that is connected to the VGA port as if
it's running on virtual console.
dmesg shows:
[...]
OF stdout device is: /[EMAIL PROTECTED],0/[EMAIL PROTECTED],1/[EMAIL
PROTECTED]/[EMAIL PROTECTED],40:a
[...]
Console: colour dummy device 80x25
console handover: boot [earlyprom0] - real [tty0]
[...]
f0061c64: ttyS0 at MMIO 0x1fff140 (irq = 5) is a SAB82532 V3.2
Console: ttyS0 (SAB82532)
console [ttyS0] enabled
f0061c64: ttyS1 at MMIO 0x1fff1400040 (irq = 5) is a SAB82532 V3.2
Replacing /sbin/reopen-console by /bin/cttyhack (which we were using
previously) in /etc/inittab fixes the problem. Which also means that
there really is absolutely no reason to just accept this regression for
Lenny.
So, cttyhack basically does the right thing by checking what type of
console the initial shell is actually running in. The only real problem
with it was that it supported nothing else than /dev/ttyS0 for the
subsequent console stealing. That last part was implemented quite nicely
in reopen-console, but the first part is missing.
So here is a patch that adds that. The new 'console-type.c' is a very
basic copy-and-naive-cleanup of cttyhack and does only one thing:
print virtual or serial to stdout.
Strange thing is that it seems to work perfectly. Tested only on sparc,
both with and without keyboard connected, i.e. both for serial and
virtual console.
Here's a netboot image that includes the patched rootskel; please test!
http://people.debian.org/%7Efjp/tmp/d-i/boot_sparc.img
Someone who can actually write in C please check the code and let me know
if anything can be removed or is missing. I may well have added unneeded
includes for example and I have no idea what the original sprintf
statements were supposed to do.
Disclaimer: this patch was written after having been awake for 25 hours,
including a visit to the gym; if reading up blows up your mind I cannot
be held responsible.
Property changes on: .
___
Added: svn:ignore
+ console-type
steal-ctty
Index: console-type.c
===
--- console-type.c (revision 0)
+++ console-type.c (revision 0)
@@ -0,0 +1,72 @@
+/* vi: set sw=4 ts=4: */
+/*
+ *
+ * Licensed under GPLv2
+ *
+ * Adapted for Debian Installer by Frans Pop fjp.debian.org from
+ * cttyhack from busybox 1.11, which is
+ *
+ * Copyright (c) 2007 Denys Vlasenko [EMAIL PROTECTED]
+ */
+
+#include sys/ioctl.h
+#include sys/types.h
+#include sys/stat.h
+#include fcntl.h
+#include unistd.h
+#include stdio.h
+#include string.h
+
+/* From linux/vt.h */
+struct vt_stat {
+ unsigned short v_active;/* active vt */
+ unsigned short v_signal;/* signal to send */
+ unsigned short v_state; /* vt bitmask */
+};
+enum { VT_GETSTATE = 0x5603 }; /* get global vt state info */
+
+/* From linux/serial.h */
+struct serial_struct {
+ int type;
+ int line;
+ unsigned int port;
+ int irq;
+ int flags;
+ int xmit_fifo_size;
+ int custom_divisor;
+ int baud_base;
+ unsigned short close_delay;
+ char io_type;
+ char reserved_char[1];
+ int hub6;
+ unsigned short closing_wait; /* time to wait before closing */
+ unsigned short closing_wait2; /* no longer used... */
+ unsigned char *iomem_base;
+ unsigned short iomem_reg_shift;
+ unsigned int port_high;
+ unsigned long iomap_base; /* cookie passed into ioremap */
+ int reserved[1];
+};
+
+int main(int argc, char ** argv)
+{
+ char console[sizeof(int)*3 + 16];
+ union {
+ struct vt_stat vt;
+ struct serial_struct sr;
+ char paranoia[sizeof(struct serial_struct) * 3];
+ } u;
+
+ strcpy(console, /dev/tty);
+ if (ioctl(0, TIOCGSERIAL, u.sr) == 0) {
+ /* this is a serial console */
+ printf(serial\n);
+ // sprintf(console + 8, S%d, u.sr.line);
+ } else if (ioctl(0, VT_GETSTATE, u.vt) == 0) {
+ /* this is linux virtual tty */
+ printf(virtual\n);
+ // sprintf(console + 8, S%d + 1, u.vt.v_active);
+ }
+
+ return 0;
+}
Index: reopen-console
===
Bug#504721: Possible reason for serial console misdetection
On Tuesday 02 December 2008, Frans Pop wrote:
Someone who can actually write in C please check the code and let me
know if anything can be removed or is missing. I may well have added
unneeded includes for example and I have no idea what the original
sprintf statements were supposed to do.
main can probably be safely reduced to:
+int main(int argc, char ** argv)
+{
+ union {
+ struct vt_stat vt;
+ struct serial_struct sr;
+ char paranoia[sizeof(struct serial_struct) * 3];
+ } u;
+
+ if (ioctl(0, TIOCGSERIAL, u.sr) == 0) {
+ /* this is a serial console */
+ printf(serial\n);
+ } else if (ioctl(0, VT_GETSTATE, u.vt) == 0) {
+ /* this is linux virtual tty */
+ printf(virtual\n);
+ }
+
+ return 0;
+}
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#504721: Possible reason for serial console misdetection
On Sun, Nov 23, 2008 at 10:15:47PM +, Jurij Smakov wrote: The logic in reopen-console is the following: [… a very good analysis …] So, the problem is that reopen-console gives preference to the value in 'console handover' line, which is incorrect on sparc (refers to a real terminal console even if one is connecting through serial). If it is obvious, that sparc is unique that way, and it works correctly on other arches, the following (untested) patch might do the trick: […] In order to implement this, I have tried to understand the kernel logic by reading at the relevant part of its source code as I have not been able to find any relevant documentation about all this. Bastian mentioned the console handover message while the issue was initally discussed, but I might just have been entirely wrong in my understanding of its meaning. I'll try to ask around and see whether current console detection in reopen-console is correct for other arches. It would be really great if you could come up with a definite answer on this! :) Cheers, -- Jérémy Bobbio.''`. [EMAIL PROTECTED]: :Ⓐ : # apt-get install anarchism `. `'` `- signature.asc Description: Digital signature
Bug#504721: Possible reason for serial console misdetection
Hi, I looked into it, and here's what I get on my SunBlade 1000 when I boot using serial console: ~ # dmesg | grep -i console [0.00] console [earlyprom0] enabled [ 53.918290] Console: colour dummy device 80x25 [ 53.971361] console handover: boot [earlyprom0] - real [tty0] [ 59.676537] Console: switching to mono PROM 80x34 [ 64.238061] Console: ttyS0 (SAB82532) [ 64.308951] console [ttyS0] enabled ~ # The logic in reopen-console is the following: 1. Look for the 'console handover' line in dmesg output and extract whatever is listed there as a real console. If it's successful, then we are done. In this case this line matches, but it contains an incorrect console setting 'tty0', referring to the terminal. As a result of this setting the information is still displayed on the serial console, but you cannot enter information through it, only keyboard input is accepted. 2. If we still don't have a value for the console, look for 'console [...] enabled' line(s), and extract console values from it. In this case two lines would match, producing console values 'earlyprom0' and 'ttyS0'. Function also checks whether corresponding devices in /dev exist, and if this reduces the number of values to a single console value, then this value is chosen. In this case /dev/earlyprom0 does not exist, so we would end up with a single 'ttyS0' value, which is correct. So, the problem is that reopen-console gives preference to the value in 'console handover' line, which is incorrect on sparc (refers to a real terminal console even if one is connecting through serial). If it is obvious, that sparc is unique that way, and it works correctly on other arches, the following (untested) patch might do the trick: --- a/src/sbin/reopen-console 2008-09-21 09:29:17.0 +0100 +++ b/src/sbin/reopen-console 2008-11-23 22:09:56.0 + @@ -12,7 +12,7 @@ sed -n -e 's/.*\] console handover: boot \[.*\] - real \[\(.*\)\]$/\1/p') consoles= - if [ -z $console ]; then + if [ -z $console ] || [ $(/bin/archdetect) = sparc/sparc64 ]; then # Retrieve all enabled consoles from boot log; ignore those # for which no device file exists for cons in $(dmesg -s 65535 | I'll try to ask around and see whether current console detection in reopen-console is correct for other arches. By the way, appending console=ttyS0 at the boot prompt fixes the problem, so can be used as a temporary workaround. Cheers. -- Jurij Smakov [EMAIL PROTECTED] Key: http://www.wooyd.org/pgpkey/ KeyID: C99E03CC -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#504721: Possible reason for serial console misdetection
severity 504721 serious thanks On Sunday 23 November 2008, Jurij Smakov wrote: So, the problem is that reopen-console gives preference to the value in 'console handover' line, which is incorrect on sparc (refers to a real terminal console even if one is connecting through serial). As serial console boots (or boots without keyboard connected) have always been supported for sparc and given this analysis, this is IMO a clear regression. Thus increasing severity. -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
