[PATCH 0/5] atm: Adjustments for some function implementations

[PATCH 0/5] atm: Adjustments for some function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 21 May 2017 22:09:11 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  Improve a size determination in four functions
  Delete an error message for a failed memory allocation in make_entry()
  Adjust 19 checks for null pointers
  Use seq_puts() in lec_info()
  Use seq_putc() in lec_info()

 net/atm/lec.c | 55 +++++++++++++++++++++++++++----------------------------
 1 file changed, 27 insertions(+), 28 deletions(-)

-- 
2.13.0

[PATCH 1/5] atm: Improve a size determination in four functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 21 May 2017 21:20:44 +0200

Replace the specification of four data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/atm/lec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 09cfe87f0a44..6ad3ca625a44 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -518,5 +518,5 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
 		return -1;
 	skb->len = sizeof(struct atmlec_msg);
 	mesg = (struct atmlec_msg *)skb->data;
-	memset(mesg, 0, sizeof(struct atmlec_msg));
+	memset(mesg, 0, sizeof(*mesg));
 	mesg->type = type;
@@ -690,5 +690,5 @@ static int lec_vcc_attach(struct atm_vcc *vcc, void __user *arg)
 	if (ioc_data.dev_num < 0 || ioc_data.dev_num >= MAX_LEC_ITF ||
 	    !dev_lec[ioc_data.dev_num])
 		return -EINVAL;
-	vpriv = kmalloc(sizeof(struct lec_vcc_priv), GFP_KERNEL);
+	vpriv = kmalloc(sizeof(*vpriv), GFP_KERNEL);
 	if (!vpriv)
@@ -1552,5 +1552,5 @@ static struct lec_arp_table *make_entry(struct lec_priv *priv,
 {
 	struct lec_arp_table *to_return;
 
-	to_return = kzalloc(sizeof(struct lec_arp_table), GFP_ATOMIC);
+	to_return = kzalloc(sizeof(*to_return), GFP_ATOMIC);
 	if (!to_return) {
@@ -2156,5 +2156,5 @@ static int lec_mcast_make(struct lec_priv *priv, struct atm_vcc *vcc)
 	struct lec_vcc_priv *vpriv;
 	int err = 0;
 
-	vpriv = kmalloc(sizeof(struct lec_vcc_priv), GFP_KERNEL);
+	vpriv = kmalloc(sizeof(*vpriv), GFP_KERNEL);
 	if (!vpriv)
-- 
2.13.0

[PATCH 2/5] atm: Delete an error message for a failed memory allocation in make_entry()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 21 May 2017 21:24:45 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/atm/lec.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 6ad3ca625a44..a11dbd3a5119 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -1556,7 +1556,6 @@ static struct lec_arp_table *make_entry(struct lec_priv *priv,
-	if (!to_return) {
-		pr_info("LEC: Arp entry kmalloc failed\n");
+	if (!to_return)
 		return NULL;
-	}
+
 	ether_addr_copy(to_return->mac_addr, mac_addr);
 	INIT_HLIST_NODE(&to_return->next);
 	setup_timer(&to_return->timer, lec_arp_expire_arp,
-- 
2.13.0

[PATCH 3/5] atm: Adjust 19 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 21 May 2017 21:34:23 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/atm/lec.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index a11dbd3a5119..ca3aec0c0743 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -139,7 +139,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
 		struct atmlec_msg *mesg;
 
 		skb2 = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
-		if (skb2 == NULL)
+		if (!skb2)
 			return;
 		skb2->len = sizeof(struct atmlec_msg);
 		mesg = (struct atmlec_msg *)skb2->data;
@@ -264,7 +264,7 @@ static netdev_tx_t lec_start_xmit(struct sk_buff *skb,
 					       min_frame_size - skb->truesize,
 					       GFP_ATOMIC);
 			dev_kfree_skb(skb);
-			if (skb2 == NULL) {
+			if (!skb2) {
 				dev->stats.tx_dropped++;
 				return NETDEV_TX_OK;
 			}
@@ -431,7 +431,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
 		pr_debug("%s: bridge zeppelin asks about %pM\n",
 			 dev->name, mesg->content.proxy.mac_addr);
 
-		if (br_fdb_test_addr_hook == NULL)
+		if (!br_fdb_test_addr_hook)
 			break;
 
 		if (br_fdb_test_addr_hook(dev, mesg->content.proxy.mac_addr)) {
@@ -442,7 +442,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
 			pr_debug("%s: entry found, responding to zeppelin\n",
 				 dev->name);
 			skb2 = alloc_skb(sizeof(struct atmlec_msg), GFP_ATOMIC);
-			if (skb2 == NULL)
+			if (!skb2)
 				break;
 			skb2->len = sizeof(struct atmlec_msg);
 			skb_copy_to_linear_data(skb2, mesg, sizeof(*mesg));
@@ -520,7 +520,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
 	mesg = (struct atmlec_msg *)skb->data;
 	memset(mesg, 0, sizeof(*mesg));
 	mesg->type = type;
-	if (data != NULL)
+	if (data)
 		mesg->sizeoftlvs = data->len;
 	if (mac_addr)
 		ether_addr_copy(mesg->content.normal.mac_addr, mac_addr);
@@ -534,7 +534,7 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
 	skb_queue_tail(&sk->sk_receive_queue, skb);
 	sk->sk_data_ready(sk);
 
-	if (data != NULL) {
+	if (data) {
 		pr_debug("about to send %d bytes of data\n", data->len);
 		atm_force_charge(priv->lecd, data->truesize);
 		skb_queue_tail(&sk->sk_receive_queue, data);
@@ -663,7 +663,7 @@ static void lec_pop(struct atm_vcc *vcc, struct sk_buff *skb)
 	struct lec_vcc_priv *vpriv = LEC_VCC_PRIV(vcc);
 	struct net_device *dev = skb->dev;
 
-	if (vpriv == NULL) {
+	if (!vpriv) {
 		pr_info("vpriv = NULL!?!?!?\n");
 		return;
 	}
@@ -1066,7 +1066,7 @@ static void __exit lane_module_cleanup(void)
 	deregister_atm_ioctl(&lane_ioctl_ops);
 
 	for (i = 0; i < MAX_LEC_ITF; i++) {
-		if (dev_lec[i] != NULL) {
+		if (dev_lec[i]) {
 			unregister_netdev(dev_lec[i]);
 			free_netdev(dev_lec[i]);
 			dev_lec[i] = NULL;
@@ -1097,11 +1097,11 @@ static int lane2_resolve(struct net_device *dev, const u8 *dst_mac, int force,
 		spin_lock_irqsave(&priv->lec_arp_lock, flags);
 		table = lec_arp_find(priv, dst_mac);
 		spin_unlock_irqrestore(&priv->lec_arp_lock, flags);
-		if (table == NULL)
+		if (!table)
 			return -1;
 
 		*tlvs = kmemdup(table->tlvs, table->sizeoftlvs, GFP_ATOMIC);
-		if (*tlvs == NULL)
+		if (!*tlvs)
 			return -1;
 
 		*sizeoftlvs = table->sizeoftlvs;
@@ -1109,12 +1109,12 @@ static int lane2_resolve(struct net_device *dev, const u8 *dst_mac, int force,
 		return 0;
 	}
 
-	if (sizeoftlvs == NULL)
+	if (!sizeoftlvs)
 		retval = send_to_lecd(priv, l_arp_xmt, dst_mac, NULL, NULL);
 
 	else {
 		skb = alloc_skb(*sizeoftlvs, GFP_ATOMIC);
-		if (skb == NULL)
+		if (!skb)
 			return -1;
 		skb->len = *sizeoftlvs;
 		skb_copy_to_linear_data(skb, *tlvs, *sizeoftlvs);
@@ -1143,12 +1143,12 @@ static int lane2_associate_req(struct net_device *dev, const u8 *lan_dst,
 	kfree(priv->tlvs);	/* NULL if there was no previous association */
 
 	priv->tlvs = kmemdup(tlvs, sizeoftlvs, GFP_KERNEL);
-	if (priv->tlvs == NULL)
+	if (!priv->tlvs)
 		return 0;
 	priv->sizeoftlvs = sizeoftlvs;
 
 	skb = alloc_skb(sizeoftlvs, GFP_ATOMIC);
-	if (skb == NULL)
+	if (!skb)
 		return 0;
 	skb->len = sizeoftlvs;
 	skb_copy_to_linear_data(skb, tlvs, sizeoftlvs);
@@ -1181,13 +1181,13 @@ static void lane2_associate_ind(struct net_device *dev, const u8 *mac_addr,
 				 */
 	struct lec_arp_table *entry = lec_arp_find(priv, mac_addr);
 
-	if (entry == NULL)
+	if (!entry)
 		return;		/* should not happen */
 
 	kfree(entry->tlvs);
 
 	entry->tlvs = kmemdup(tlvs, sizeoftlvs, GFP_KERNEL);
-	if (entry->tlvs == NULL)
+	if (!entry->tlvs)
 		return;
 	entry->sizeoftlvs = sizeoftlvs;
 #endif
@@ -1854,7 +1854,7 @@ lec_arp_update(struct lec_priv *priv, const unsigned char *mac_addr,
 
 	spin_lock_irqsave(&priv->lec_arp_lock, flags);
 	entry = lec_arp_find(priv, mac_addr);
-	if (entry == NULL && targetless_le_arp)
+	if (!entry && targetless_le_arp)
 		goto out;	/*
 				 * LANE2: ignore targetless LE_ARPs for which
 				 * we have no entry in the cache. 7.1.30
@@ -1965,7 +1965,7 @@ lec_vcc_added(struct lec_priv *priv, const struct atmlec_ioc *ioc_data,
 		entry->old_recv_push = old_push;
 #endif
 		entry = make_entry(priv, bus_mac);
-		if (entry == NULL)
+		if (!entry)
 			goto out;
 		del_timer(&entry->timer);
 		memcpy(entry->atm_addr, ioc_data->atm_addr, ATM_ESA_LEN);
@@ -1990,7 +1990,7 @@ lec_vcc_added(struct lec_priv *priv, const struct atmlec_ioc *ioc_data,
 			 ioc_data->atm_addr[16], ioc_data->atm_addr[17],
 			 ioc_data->atm_addr[18], ioc_data->atm_addr[19]);
 		entry = make_entry(priv, bus_mac);
-		if (entry == NULL)
+		if (!entry)
 			goto out;
 		memcpy(entry->atm_addr, ioc_data->atm_addr, ATM_ESA_LEN);
 		eth_zero_addr(entry->mac_addr);
-- 
2.13.0

[PATCH 4/5] atm: Use seq_puts() in lec_info()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 21 May 2017 21:49:10 +0200

A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/atm/lec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index ca3aec0c0743..752891434074 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -804,7 +804,7 @@ static void lec_info(struct seq_file *seq, struct lec_arp_table *entry)
 	if (entry->vcc)
 		seq_printf(seq, "%3d %3d ", entry->vcc->vpi, entry->vcc->vci);
 	else
-		seq_printf(seq, "        ");
+		seq_puts(seq, "        ");
 	if (entry->recv_vcc) {
 		seq_printf(seq, "     %3d %3d", entry->recv_vcc->vpi,
 			   entry->recv_vcc->vci);
-- 
2.13.0

[PATCH 5/5] atm: Use seq_putc() in lec_info()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 21 May 2017 21:50:44 +0200

A single space character should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/atm/lec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 752891434074..1d4edf093987 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -796,7 +796,7 @@ static void lec_info(struct seq_file *seq, struct lec_arp_table *entry)
 
 	for (i = 0; i < ETH_ALEN; i++)
 		seq_printf(seq, "%2.2x", entry->mac_addr[i] & 0xff);
-	seq_printf(seq, " ");
+	seq_putc(seq, ' ');
 	for (i = 0; i < ATM_ESA_LEN; i++)
 		seq_printf(seq, "%2.2x", entry->atm_addr[i] & 0xff);
 	seq_printf(seq, " %s %4.4x", lec_arp_get_status_string(entry->status),
-- 
2.13.0

Re: [PATCH 0/5] atm: Adjustments for some function implementations

[Kees Cook]

On Sun, May 21, 2017 at 1:12 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 21 May 2017 22:09:11 +0200
>
> A few update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (5):
>   Improve a size determination in four functions
>   Delete an error message for a failed memory allocation in make_entry()
>   Adjust 19 checks for null pointers
>   Use seq_puts() in lec_info()
>   Use seq_putc() in lec_info()
>
>  net/atm/lec.c | 55 +++++++++++++++++++++++++++----------------------------
>  1 file changed, 27 insertions(+), 28 deletions(-)

These all look fine to me. Thanks!

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

-- 
Kees Cook
Pixel Security