On 10/26/2017 04:25 PM, Alexander Graf wrote:
On 26.10.17 20:52, Collin L. Walling wrote:
The sclp console in the s390 bios writes raw data,
leading console emulators (such as virsh console) to
treat a new line ('\n') as just a new line instead
of as a Unix line feed. Because of this, output
appears in a "stair case" pattern.
Let's print \r\n on every occurrence of a new line
in the string passed to write to amend this issue.
This is in sync with the guest Linux code in
drivers/s390/char/sclp_vt220.c which also does a line feed
conversion in the console part of the driver.
This fixes the s390-ccw and s390-netboot output like
$ virsh start test --console
Domain test started
Connected to domain test
Escape character is ^]
Network boot starting...
Using MAC address: 02:01:02:03:04:05
Requesting
information via DHCP: 010
Signed-off-by: Collin L. Walling <wall...@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com>
---
pc-bios/s390-ccw/sclp.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c
index 486fce1..f8ad5ae 100644
--- a/pc-bios/s390-ccw/sclp.c
+++ b/pc-bios/s390-ccw/sclp.c
@@ -68,17 +68,27 @@ void sclp_setup(void)
long write(int fd, const void *str, size_t len)
{
WriteEventData *sccb = (void *)_sccb;
+ const char *p = str;
+ size_t data_len = 0;
+ size_t i;
if (fd != 1 && fd != 2) {
return -EIO;
}
- sccb->h.length = sizeof(WriteEventData) + len;
+ for (i = len; i > 0; i--) {
Where did the bounds check go? If you write(max) before, you were
writing max bytes. If you do it now, you end up writing max + n bytes
and potentially overflow the array, no?
Alex
I wasn't a fan of the code aesthetics and being that the SCCB write buffer
allows about 4k bytes of data to be written to it, I felt it was safe to
remove it. It's unlikely we'd be writing that much data in the bios, plus
that check did not exist prior to this fixup.
Though, reading that out loud, it probably isn't the best idea to sacrifice
code robustness for code aesthetics.
for (i = len; i > 0; i--) {
if (data_len > SCCB_DATA_LEN - 1) {
return -SOME_ERROR
}
if (*p == '\n') {
sccb->data[data_len++] = '\r';
}
sccb->data[data_len++] = *p;
p++;
}
What do you think?
- Collin
+ if (*p == '\n') {
+ sccb->data[data_len++] = '\r';
+ }
+ sccb->data[data_len++] = *p;
+ p++;
+ }
+
+ sccb->h.length = sizeof(WriteEventData) + data_len;
sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
- sccb->ebh.length = sizeof(EventBufferHeader) + len;
+ sccb->ebh.length = sizeof(EventBufferHeader) + data_len;
sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
sccb->ebh.flags = 0;
- memcpy(sccb->data, str, len);
sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb);
--
- Collin L Walling
