From: Julian Wiedmann <jwi@linux.ibm.com>
Subject: s390: qeth: Fix potential array overrun in cmd/rc lookup
Patch-mainline: v4.19-rc7
Git-commit: 048a7f8b4ec085d5c56ad4a3bf450389a4aed5f9
References: bnc#1113501, LTC#172682, FATE#326350, LTC#169511, bsc#1113509

Description:  qeth: Fix potential array overrun in cmd/rc lookup
Symptom:      Infinite loop when processing a received cmd.
Problem:      qeth_get_ipa_cmd_name() and qeth_get_ipa_msg() are used
              to build human-readable messages for received cmd data.

              They store the to-be translated value in the last entry of a
              global array, and then iterate over each entry until they found
              the queried value (and the corresponding message string).
              If there is no prior match, the lookup is intended to stop at
              the final entry (which was previously prepared).

              If two qeth devices are concurrently processing a received cmd,
              one lookup can over-write the last entry of the global array
              while a second lookup is in process. This second lookup will then
              never hit its stop-condition, and loop.
Solution:     Remove the modification of the global array, and limit the number
              of iterations to the size of the array.
Reproduction: -

Upstream-Description:

              s390: qeth: Fix potential array overrun in cmd/rc lookup

              Functions qeth_get_ipa_msg and qeth_get_ipa_cmd_name are modifying
              the last member of global arrays without any locking that I can see.
              If two instances of either function are running at the same time,
              it could cause a race ultimately leading to an array overrun (the
              contents of the last entry of the array is the only guarantee that
              the loop will ever stop).

              Performing the lookups without modifying the arrays is admittedly
              slower (two comparisons per iteration instead of one) but these
              are operations which are rare (should only be needed in error
              cases or when debugging, not during successful operation) and it
              seems still less costly than introducing a mutex to protect the
              arrays in question.

              As a side bonus, it allows us to declare both arrays as const data.

              Signed-off-by: Jean Delvare <jdelvare@suse.de>
              Cc: Julian Wiedmann <jwi@linux.ibm.com>
              Cc: Ursula Braun <ubraun@linux.ibm.com>
              Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
              Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
              Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
              Signed-off-by: David S. Miller <davem@davemloft.net>


Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
Acked-by: Petr Tesarik <ptesarik@suse.com>
---
 drivers/s390/net/qeth_core_main.c |  2 +-
 drivers/s390/net/qeth_core_mpc.c  | 30 ++++++++++++++++--------------
 drivers/s390/net/qeth_core_mpc.h  |  4 ++--
 3 files changed, 19 insertions(+), 17 deletions(-)

--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -610,7 +610,7 @@ static void qeth_put_reply(struct qeth_reply *reply)
 static void qeth_issue_ipa_msg(struct qeth_ipa_cmd *cmd, int rc,
 		struct qeth_card *card)
 {
-	char *ipa_name;
+	const char *ipa_name;
 	int com = cmd->hdr.command;
 	ipa_name = qeth_get_ipa_cmd_name(com);
 	if (rc)
--- a/drivers/s390/net/qeth_core_mpc.c
+++ b/drivers/s390/net/qeth_core_mpc.c
@@ -148,10 +148,10 @@ EXPORT_SYMBOL_GPL(IPA_PDU_HEADER);
 
 struct ipa_rc_msg {
 	enum qeth_ipa_return_codes rc;
-	char *msg;
+	const char *msg;
 };
 
-static struct ipa_rc_msg qeth_ipa_rc_msg[] = {
+static const struct ipa_rc_msg qeth_ipa_rc_msg[] = {
 	{IPA_RC_SUCCESS,		"success"},
 	{IPA_RC_NOTSUPP,		"Command not supported"},
 	{IPA_RC_IP_TABLE_FULL,		"Add Addr IP Table Full - ipv6"},
@@ -219,22 +219,23 @@ static struct ipa_rc_msg qeth_ipa_rc_msg[] = {
 
 
 
-char *qeth_get_ipa_msg(enum qeth_ipa_return_codes rc)
+const char *qeth_get_ipa_msg(enum qeth_ipa_return_codes rc)
 {
-	int x = 0;
-	qeth_ipa_rc_msg[ARRAY_SIZE(qeth_ipa_rc_msg) - 1].rc = rc;
-	while (qeth_ipa_rc_msg[x].rc != rc)
-		x++;
+	int x;
+
+	for (x = 0; x < ARRAY_SIZE(qeth_ipa_rc_msg) - 1; x++)
+		if (qeth_ipa_rc_msg[x].rc == rc)
+			return qeth_ipa_rc_msg[x].msg;
 	return qeth_ipa_rc_msg[x].msg;
 }
 
 
 struct ipa_cmd_names {
 	enum qeth_ipa_cmds cmd;
-	char *name;
+	const char *name;
 };
 
-static struct ipa_cmd_names qeth_ipa_cmd_names[] = {
+static const struct ipa_cmd_names qeth_ipa_cmd_names[] = {
 	{IPA_CMD_STARTLAN,	"startlan"},
 	{IPA_CMD_STOPLAN,	"stoplan"},
 	{IPA_CMD_SETVMAC,	"setvmac"},
@@ -266,11 +267,12 @@ static struct ipa_cmd_names qeth_ipa_cmd_names[] = {
 	{IPA_CMD_UNKNOWN,	"unknown"},
 };
 
-char *qeth_get_ipa_cmd_name(enum qeth_ipa_cmds cmd)
+const char *qeth_get_ipa_cmd_name(enum qeth_ipa_cmds cmd)
 {
-	int x = 0;
-	qeth_ipa_cmd_names[ARRAY_SIZE(qeth_ipa_cmd_names) - 1].cmd = cmd;
-	while (qeth_ipa_cmd_names[x].cmd != cmd)
-		x++;
+	int x;
+
+	for (x = 0; x < ARRAY_SIZE(qeth_ipa_cmd_names) - 1; x++)
+		if (qeth_ipa_cmd_names[x].cmd == cmd)
+			return qeth_ipa_cmd_names[x].name;
 	return qeth_ipa_cmd_names[x].name;
 }
--- a/drivers/s390/net/qeth_core_mpc.h
+++ b/drivers/s390/net/qeth_core_mpc.h
@@ -797,8 +797,8 @@ enum qeth_ipa_arp_return_codes {
 	QETH_IPA_ARP_RC_Q_NO_DATA    = 0x0008,
 };
 
-extern char *qeth_get_ipa_msg(enum qeth_ipa_return_codes rc);
-extern char *qeth_get_ipa_cmd_name(enum qeth_ipa_cmds cmd);
+extern const char *qeth_get_ipa_msg(enum qeth_ipa_return_codes rc);
+extern const char *qeth_get_ipa_cmd_name(enum qeth_ipa_cmds cmd);
 
 #define QETH_SETASS_BASE_LEN (sizeof(struct qeth_ipacmd_hdr) + \
 			       sizeof(struct qeth_ipacmd_setassparms_hdr))