Skip to contentSkip to conversation

s390: network patches for net-next V1

PATCH In review [email protected] 3 participants 7 messages May 28, 2014 – May 31, 2014

Subsystems (5)
Networking Network drivers s390 s390 network drivers +1 more subsystems
lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover s390: network patches for net-next V1

Hi Dave,

here are some s390 related patches for net-next
Added some style fixing reported by David Laight.

shortlog:

Philipp Hachtmann (1)
af_iucv: Add automatic (source) iucv_name to bind

Ursula Braun (2)
af_iucv: correct cleanup if listen backlog is full s390/net: fix format string mismatches

Rickard Strandqvist (1)
qeth: Fix for possible null pointer dereference

Thanks,
Frank

author
#1 lore raw

PATCH 1/4 af_iucv: Add automatic (source) iucv_name to bind

From: Philipp Hachtmann <[email protected]>

If a socket is bound to an address using before calling connect it is usual to leave it to the network system to choose an appropriate outgoing application name respective port address.
af_iucv on VM uses a counter and uses simple numbers as unique identifiers. This behaviour was missing when af_iucv is used with HiperSockets.

This patch contains a simple approach to harmonize af_iucv's behaviour.

Signed-off-by Philipp Hachtmann <[email protected]>
Signed-off-by Frank Blaschka <[email protected]>
---
 net/iucv/af_iucv.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)
net/iucv/af_iucv.c +18 -11
index 8c9d730..60f5c20 100644
@@ -682,6 +682,18 @@ struct sock *iucv_accept_dequeue(struct sock *parent, struct socket *newsock)
 	return NULL;
 }
 
+static void __iucv_auto_name(struct iucv_sock *iucv)
+{
+	char name[12];
+
+	sprintf(name, "%08x", atomic_inc_return(&iucv_sk_list.autobind_name));
+	while (__iucv_get_sock_by_name(name)) {
+		sprintf(name, "%08x",
+			atomic_inc_return(&iucv_sk_list.autobind_name));
+	}
+	memcpy(iucv->src_name, name, 8);
+}
+
 /* Bind an unbound socket */
 static int iucv_sock_bind(struct socket *sock, struct sockaddr *addr,
 			  int addr_len)

@@ -724,8 +736,12 @@ static int iucv_sock_bind(struct socket *sock, struct sockaddr *addr,
 	rcu_read_lock();
 	for_each_netdev_rcu(&init_net, dev) {
 		if (!memcmp(dev->perm_addr, uid, 8)) {
-			memcpy(iucv->src_name, sa->siucv_name, 8);
 			memcpy(iucv->src_user_id, sa->siucv_user_id, 8);
+			/* Check for unitialized siucv_name */
+			if (strncmp(sa->siucv_name, "        ", 8) == 0)
+				__iucv_auto_name(iucv);
+			else
+				memcpy(iucv->src_name, sa->siucv_name, 8);
 			sk->sk_bound_dev_if = dev->ifindex;
 			iucv->hs_dev = dev;
 			dev_hold(dev);

@@ -763,7 +779,6 @@ done:
 static int iucv_sock_autobind(struct sock *sk)
 {
 	struct iucv_sock *iucv = iucv_sk(sk);
-	char name[12];
 	int err = 0;
 
 	if (unlikely(!pr_iucv))

@@ -772,17 +787,9 @@ static int iucv_sock_autobind(struct sock *sk)
 	memcpy(iucv->src_user_id, iucv_userid, 8);
 
 	write_lock_bh(&iucv_sk_list.lock);
-
-	sprintf(name, "%08x", atomic_inc_return(&iucv_sk_list.autobind_name));
-	while (__iucv_get_sock_by_name(name)) {
-		sprintf(name, "%08x",
-			atomic_inc_return(&iucv_sk_list.autobind_name));
-	}
-
+	__iucv_auto_name(iucv);
 	write_unlock_bh(&iucv_sk_list.lock);
 
-	memcpy(&iucv->src_name, name, 8);
-
 	if (!iucv->msglimit)
 		iucv->msglimit = IUCV_QUEUELEN_DEFAULT;
author
#2 lore raw

PATCH 2/4 af_iucv: correct cleanup if listen backlog is full

In reply to [email protected]

From: Ursula Braun <[email protected]>

In case of transport HIPER a sock struct is allocated for an incoming connect request. If the backlog queue is full this socket is not needed, but is left in the list of af_iucv sockets. Final socket release posts console message "Attempt to release alive iucv socket". This patch makes sure the new created socket is cleaned up correctly if the backlog queue is full.

Signed-off-by Ursula Braun <[email protected]>
Signed-off-by Frank Blaschka <[email protected]>
Reported-by Philipp Hachtmann <[email protected]>
---
 net/iucv/af_iucv.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
net/iucv/af_iucv.c +1 -2
index 60f5c20..7a95fa4 100644
@@ -1943,11 +1943,10 @@ static int afiucv_hs_callback_syn(struct sock *sk, struct sk_buff *skb)
 	    sk_acceptq_is_full(sk) ||
 	    !nsk) {
 		/* error on server socket - connection refused */
-		if (nsk)
-			sk_free(nsk);
 		afiucv_swap_src_dest(skb);
 		trans_hdr->flags = AF_IUCV_FLAG_SYN | AF_IUCV_FLAG_FIN;
 		err = dev_queue_xmit(skb);
+		iucv_sock_kill(nsk);
 		bh_unlock_sock(sk);
 		goto out;
 	}
author
#3 lore raw

PATCH 3/4 qeth: Fix for possible null pointer dereference

In reply to [email protected]

From: Rickard Strandqvist <[email protected]>

There is otherwise a risk of a possible null pointer dereference.

Was largely found by using a static code analysis program called cppcheck.

Signed-off-by Rickard Strandqvist <[email protected]>
Signed-off-by Frank Blaschka <[email protected]>
---
 drivers/s390/net/qeth_core_main.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/s390/net/qeth_core_main.c +2 -2
@@ -1013,7 +1013,7 @@ static long __qeth_check_irb_error(struc
 
 	card = CARD_FROM_CDEV(cdev);
 
-	if (!IS_ERR(irb))
+	if (!card || !IS_ERR(irb))
 		return 0;
 
 	switch (PTR_ERR(irb)) {

@@ -1029,7 +1029,7 @@ static long __qeth_check_irb_error(struc
 		QETH_CARD_TEXT(card, 2, "ckirberr");
 		QETH_CARD_TEXT_(card, 2, "  rc%d", -ETIMEDOUT);
 		if (intparm == QETH_RCD_PARM) {
-			if (card && (card->data.ccwdev == cdev)) {
+			if (card->data.ccwdev == cdev) {
 				card->data.state = CH_STATE_DOWN;
 				wake_up(&card->wait_q);
 			}
author
#4 lore raw

PATCH 4/4 s390/net: fix format string mismatches

In reply to [email protected]

From: Ursula Braun <[email protected]>

cppcheck blamed some issues in drivers/s390/net/... They are fixed here.

Signed-off-by Ursula Braun <[email protected]>
Signed-off-by Frank Blaschka <[email protected]>
Reported-by Toralf Foerster <[email protected]>
---
 drivers/s390/net/ctcm_sysfs.c   |   14 +++++++++-----
 drivers/s390/net/lcs.c          |   13 +++++++++----
 drivers/s390/net/qeth_l3_main.c |    2 +-
 3 files changed, 19 insertions(+), 10 deletions(-)
drivers/s390/net/ctcm_sysfs.c +9 -5
@@ -34,8 +34,9 @@ static ssize_t ctcm_buffer_write(struct
 		struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct net_device *ndev;
-	int bs1;
+	unsigned int bs1;
 	struct ctcm_priv *priv = dev_get_drvdata(dev);
+	int rc;
 
 	ndev = priv->channel[CTCM_READ]->netdev;
 	if (!(priv && priv->channel[CTCM_READ] && ndev)) {

@@ -43,7 +44,9 @@ static ssize_t ctcm_buffer_write(struct
 		return -ENODEV;
 	}
 
-	sscanf(buf, "%u", &bs1);
+	rc = sscanf(buf, "%u", &bs1);
+	if (rc != 1)
+		goto einval;
 	if (bs1 > CTCM_BUFSIZE_LIMIT)
 					goto einval;
 	if (bs1 < (576 + LL_HEADER_LENGTH + 2))

@@ -143,13 +146,14 @@ static ssize_t ctcm_proto_show(struct de
 static ssize_t ctcm_proto_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
-	int value;
+	int value, rc;
 	struct ctcm_priv *priv = dev_get_drvdata(dev);
 
 	if (!priv)
 		return -ENODEV;
-	sscanf(buf, "%u", &value);
-	if (!((value == CTCM_PROTO_S390)  ||
+	rc = sscanf(buf, "%d", &value);
+	if ((rc != 1) ||
+	    !((value == CTCM_PROTO_S390)  ||
 	      (value == CTCM_PROTO_LINUX) ||
 	      (value == CTCM_PROTO_MPC) ||
 	      (value == CTCM_PROTO_OS390)))
drivers/s390/net/lcs.c +9 -4
@@ -1943,14 +1943,16 @@ static ssize_t
 lcs_portno_store (struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
 {
         struct lcs_card *card;
-        int value;
+	int value, rc;
 
 	card = dev_get_drvdata(dev);
 
         if (!card)
                 return 0;
 
-        sscanf(buf, "%u", &value);
+	rc = sscanf(buf, "%d", &value);
+	if (rc != 1)
+		return -EINVAL;
         /* TODO: sanity checks */
         card->portno = value;
 

@@ -1997,14 +1999,17 @@ static ssize_t
 lcs_timeout_store (struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
 {
         struct lcs_card *card;
-        int value;
+	unsigned int value;
+	int rc;
 
 	card = dev_get_drvdata(dev);
 
         if (!card)
                 return 0;
 
-        sscanf(buf, "%u", &value);
+	rc = sscanf(buf, "%u", &value);
+	if (rc != 1)
+		return -EINVAL;
         /* TODO: sanity checks */
         card->lancmd_timeout = value;
drivers/s390/net/qeth_l3_main.c +1 -1
@@ -63,7 +63,7 @@ void qeth_l3_ipaddr4_to_string(const __u
 int qeth_l3_string_to_ipaddr4(const char *buf, __u8 *addr)
 {
 	int count = 0, rc = 0;
-	int in[4];
+	unsigned int in[4];
 	char c;
 
 	rc = sscanf(buf, "%u.%u.%u.%u%c",
#5 lore raw

Re: [patch 4/4] s390/net: fix format string mismatches

Hi Frank:

A trivial note:

On Wed, 2014-05-28 at 10:22 +0200, [email protected] wrote:

(submitting a patch containing changes like)

+ rc = sscanf(buf, "%u", &bs1);
+ if (rc != 1)
+ goto einval;

All of these single var sscanf uses could be
changed to kstrto<foo>