Skip to contentSkip to conversation

Staging: rtl8192e: Safer allocation and cleaner error handling

PATCH New Felix Schlepper 1 participant 4 messages Jun 24, 2022 Staging

lore
Share & tools
lore

Felix Schlepper

author
#0 lore raw

v5 cover Staging: rtl8192e: Safer allocation and cleaner error handling

v2:
- replaced kmalloc with kzalloc against memory initialization defects, thus also removing a memset
- made error handling more consistent

v3:
- Split into smaller commits so that it's easier to review

v4:
- clearer commit messages
- added missing kfree

v5:
- resend due incorrect email threading caused by using msmtp for send-email (probably some incorrect configuration on my part) Apologies for spamming greg k-h inbox/mailing list.

Felix Schlepper (3):
Staging: rtl8192e: Use struct_size
Staging: rtl8192e: Using kzalloc and delete memset
Staging: rtl8192e: Cleaning up error handling

 drivers/staging/rtl8192e/rtllib_tx.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)
Signature · 1 line
2.36.1
author
#1 lore raw

PATCH v5 1/3 Staging: rtl8192e: Use struct_size

Using struct_size is encouraged because it is safer than using sizeof and calculations by hand.

Signed-off-by Felix Schlepper <[email protected]>
---
 drivers/staging/rtl8192e/rtllib_tx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/rtl8192e/rtllib_tx.c +1 -2
index 37715afb0210..f2ef32e943ae 100644
@@ -205,8 +205,7 @@ static struct rtllib_txb *rtllib_alloc_txb(int nr_frags, int txb_size,
 	struct rtllib_txb *txb;
 	int i;
 
-	txb = kmalloc(sizeof(struct rtllib_txb) + (sizeof(u8 *) * nr_frags),
-		      gfp_mask);
+	txb = kmalloc(struct_size(txb, fragments, nr_frags), gfp_mask);
 	if (!txb)
 		return NULL;
Signature · 1 line
2.36.1
author
#2 lore raw

PATCH v5 2/3 Staging: rtl8192e: Using kzalloc and delete memset

In reply to Felix Schlepper

By using kzalloc, we can delete a memset. The practical difference is that using kzalloc() will zero out the txb->fragments[] array. The original code worked fine, but zeroing everything seems nicer.

Signed-off-by Felix Schlepper <[email protected]>
---
 drivers/staging/rtl8192e/rtllib_tx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/rtl8192e/rtllib_tx.c +1 -2
index f2ef32e943ae..1307cf55741a 100644
@@ -205,11 +205,10 @@ static struct rtllib_txb *rtllib_alloc_txb(int nr_frags, int txb_size,
 	struct rtllib_txb *txb;
 	int i;
 
-	txb = kmalloc(struct_size(txb, fragments, nr_frags), gfp_mask);
+	txb = kzalloc(struct_size(txb, fragments, nr_frags), gfp_mask);
 	if (!txb)
 		return NULL;
 
-	memset(txb, 0, sizeof(struct rtllib_txb));
 	txb->nr_frags = nr_frags;
 	txb->frag_size = cpu_to_le16(txb_size);
Signature · 1 line
2.36.1
author
#3 lore raw

PATCH v5 3/3 Staging: rtl8192e: Cleaning up error handling

In reply to Felix Schlepper

Moved error handling to one common block. This removes double checking if all txb->fragments[] were initialized.
The original code worked fine, but this is cleaner.

Signed-off-by Felix Schlepper <[email protected]>
---
 drivers/staging/rtl8192e/rtllib_tx.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
drivers/staging/rtl8192e/rtllib_tx.c +10 -10
index 1307cf55741a..42f81b23a144 100644
@@ -214,19 +214,19 @@ static struct rtllib_txb *rtllib_alloc_txb(int nr_frags, int txb_size,
 
 	for (i = 0; i < nr_frags; i++) {
 		txb->fragments[i] = dev_alloc_skb(txb_size);
-		if (unlikely(!txb->fragments[i])) {
-			i--;
-			break;
-		}
+		if (unlikely(!txb->fragments[i]))
+			goto err_free;
 		memset(txb->fragments[i]->cb, 0, sizeof(txb->fragments[i]->cb));
 	}
-	if (unlikely(i != nr_frags)) {
-		while (i >= 0)
-			dev_kfree_skb_any(txb->fragments[i--]);
-		kfree(txb);
-		return NULL;
-	}
+
 	return txb;
+
+err_free:
+	while (--i >= 0)
+		dev_kfree_skb_any(txb->fragments[i]);
+	kfree(txb);
+
+	return NULL;
 }
 
 static int rtllib_classify(struct sk_buff *skb, u8 bIsAmsdu)
Signature · 1 line
2.36.1