Bug#813089: qeth/ctc: correct stack corruption for numerous qeth/ctc devices
Package: s390-netdevice
Version: 0.0.39
Severity: important
Tags: d-i patch
Hi,
the s390-netdevice fails to display the list of qeth netword device if there
are numerous network devices available. For example, with 68 or more qeth
network devices (each consisting of 3 devices).
The problem is a static buffer defined on the stack of the get_ctc_channels()
and get_qeth_device() functions. When iterating through the tree/list of
network devices, the buffer is filled to be later displayed to the user.
The strncat() function writes beyond the end the of the buffer and corrupts
the function stack.
To solve this problem, the buffer that contains the network device list is
dynamically allocated. The buffer size is determined from the number of
network devices.
Thanks and kind regards,
Hendrik
>From 223ebc92969fcb5996aef83e4bfdfe93f2861c51 Mon Sep 17 00:00:00 2001
From: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Date: Thu, 21 Jan 2016 19:53:36 +0100
Subject: [PATCH 2/3] netdevice: correct stack corruption due to numerous
devices
If there are numerous network devices present, the size of the
static buffer (on the stack) is exceeded and the program fails.
Dynamically allocate memory to create a complete list of channel
devices to be displayed to the user.
Signed-off-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
---
netdevice.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 68 insertions(+), 21 deletions(-)
diff --git a/netdevice.c b/netdevice.c
index b8512fb..c886170 100644
--- a/netdevice.c
+++ b/netdevice.c
@@ -85,6 +85,12 @@ static const struct driver drivers[] =
{ "qeth", CHANNEL_TYPE_QETH },
};
+struct buffer_desc
+{
+ char *buf;
+ size_t size;
+};
+
enum
{
TYPE_NONE = 0,
@@ -314,54 +320,76 @@ static di_hfunc get_ctc_channels_append;
static void get_ctc_channels_append (void *key __attribute__ ((unused)), void *value, void *user_data)
{
struct channel *channel = value;
- char *buf = user_data;
+ struct buffer_desc *bd = user_data;
+
if (channel->type == CHANNEL_TYPE_CU3088_CTC)
- {
- if (buf[0])
- strncat (buf, ", ", 64 * 8);
- strncat (buf, channel->name, 64 * 8);
- }
+ di_snprintfcat (bd->buf, bd->size, "%s%s",
+ bd->buf[0] ? ", " : "",
+ channel->name);
}
static enum state_wanted get_ctc_channels (void)
{
- char buf[64 * 8] = { 0 }, *ptr;
const char *template;
+ struct buffer_desc bd;
int dev, ret;
+ char *ptr;
- di_tree_foreach (channels, get_ctc_channels_append, buf);
+ /* Allocate memory to create the complete list of channels,
+ * account 2 characters as list separator, 9 characters to
+ * contain the channel bus-ID (xx.y.zzzz), and a NUL to end
+ * the string.
+ */
+ bd.size = di_tree_size (channels) * (2 + 9 + 1);
+ bd.buf = di_malloc0 (bd.size);
- if (!strlen (buf))
+ di_tree_foreach (channels, get_ctc_channels_append, &bd);
+
+ if (!strlen (bd.buf))
{
my_debconf_input ("critical", TEMPLATE_PREFIX "ctc/no", &ptr);
+ di_free (bd.buf);
return WANT_BACKUP;
}
template = TEMPLATE_PREFIX "ctc/choose_read";
- debconf_subst (client, template, "choices", buf);
+ debconf_subst (client, template, "choices", bd.buf);
debconf_input (client, "critical", template);
ret = debconf_go (client);
if (ret == 30)
+ {
+ di_free (bd.buf);
return WANT_BACKUP;
+ }
if (ret)
+ {
+ di_free (bd.buf);
return WANT_ERROR;
+ }
debconf_get (client, template);
dev = channel_device (client->value);
device_current->ctc.channels[0] = di_tree_lookup (channels, &dev);
template = TEMPLATE_PREFIX "ctc/choose_write";
- debconf_subst (client, template, "choices", buf);
+ debconf_subst (client, template, "choices", bd.buf);
debconf_input (client, "critical", template);
ret = debconf_go (client);
if (ret == 30)
+ {
+ di_free (bd.buf);
return WANT_BACKUP;
+ }
if (ret)
+ {
+ di_free (bd.buf);
return WANT_ERROR;
+ }
debconf_get (client, template);
dev = channel_device (client->value);
device_current->ctc.channels[1] = di_tree_lookup (channels, &dev);
+ di_free (bd.buf);
return WANT_NEXT;
}
@@ -408,41 +436,60 @@ static di_hfunc get_qeth_device_append;
static void get_qeth_device_append (void *key __attribute__ ((unused)), void *value, void *user_data)
{
struct device *device = value;
- char *buf = user_data;
+ struct buffer_desc *bd = user_data;
+
if (device->type == DEVICE_TYPE_QETH)
- {
- if (buf[0])
- strncat (buf, ", ", 64 * 28);
- di_snprintfcat (buf, 64 * 28, "%s-%s-%s", device->qeth.channels[0]->name, device->qeth.channels[1]->name, device->qeth.channels[2]->name);
- }
+ di_snprintfcat (bd->buf, bd->size, "%s%s-%s-%s",
+ bd->buf[0] ? ", " : "",
+ device->qeth.channels[0]->name,
+ device->qeth.channels[1]->name,
+ device->qeth.channels[2]->name);
}
static enum state_wanted get_qeth_device (void)
{
- char buf[64 * 28] = { 0 }, *ptr;
const char *template;
+ struct buffer_desc bd;
int dev, ret;
+ char *ptr;
+
+ /* Allocate memory to create the complete list of channels,
+ * account 2 characters as list separator, 10 characters
+ * for each qeth channel bus-ID (xx.y.zzzz), delimited with
+ * "-", and a NUL to end the string.
+ */
+ bd.size = 2 + 3 * 10 + 1;
+ bd.size *= di_tree_size (devices);
+ bd.buf = di_malloc0 (bd.size);
- di_tree_foreach (devices, get_qeth_device_append, buf);
+ di_tree_foreach (devices, get_qeth_device_append, &bd);
- if (!strlen (buf))
+ if (!strlen (bd.buf))
{
my_debconf_input ("critical", TEMPLATE_PREFIX "qeth/no", &ptr);
+ di_free (bd.buf);
return WANT_BACKUP;
}
template = TEMPLATE_PREFIX "qeth/choose";
- debconf_subst (client, template, "choices", buf);
+ debconf_subst (client, template, "choices", bd.buf);
debconf_input (client, "critical", template);
ret = debconf_go (client);
if (ret == 30)
+ {
+ di_free (bd.buf);
return WANT_BACKUP;
+ }
if (ret)
+ {
+ di_free (bd.buf);
return WANT_ERROR;
+ }
debconf_get (client, template);
dev = channel_device (client->value);
device_current = di_tree_lookup (devices, &dev);
+ di_free (bd.buf);
return WANT_NEXT;
}
--
2.7.0.rc3
Reply to: