staging: r8188eu: fix sparse warnings

PATCH Superseded Aakash Hemadri 3 participants 10 messages Aug 23, 2021 – Aug 28, 2021 Staging

lore

Share & tools

lore

Aakash Hemadri

author

2021-08-23 17:00 UTC #0 lore raw

v4 cover staging: r8188eu: fix sparse warnings

Hi,
This patch series fixes some sparse warnings in rtw_br_ext.c

Changes in v3 -> v4
- Added this changelog, as requested by Greg's patch bot

Changes in v2 -> v3
- Fixed incorrect usage/removal of endian swaps and checks

Changes in v1 -> v2
- Split patch

Aakash Hemadri (5):
staging: r8188eu: restricted __be16 degrades to int
staging: r8188eu: cast to restricted __be32
staging: r8188eu: incorrect type in csum_ipv6_magic
staging: r8188eu: restricted __be16 degrades to int
staging: r8188eu: incorrect type in assignment

 drivers/staging/r8188eu/core/rtw_br_ext.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

base-commit: 093991aaadf0fbb34184fa37a46e7a157da3f386

Signature · 1 line

2.32.0

Aakash Hemadri

author

2021-08-23 17:00 UTC #1 lore raw

PATCH v4 1/5 staging: r8188eu: restricted __be16 degrades to int

Fix sparse warning:

Quoted 1 line

rtw_br_ext.c:73:23: warning: restricted __be16 degrades to integer

Here tag->tag_len is be16, use be16_to_cpu()

Signed-off-by Aakash Hemadri <[email protected]>

---

Prefer using be16_to_cpu() over ntohs()

 drivers/staging/r8188eu/core/rtw_br_ext.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/r8188eu/core/rtw_br_ext.c +1 -1

index ee52f28a1e56..f6d1f6029ec3 100644
@@ -70,7 +70,7 @@ static int __nat25_add_pppoe_tag(struct sk_buff *skb, struct pppoe_tag *tag)
 	struct pppoe_hdr *ph = (struct pppoe_hdr *)(skb->data + ETH_HLEN);
 	int data_len;
 
-	data_len = tag->tag_len + TAG_HDR_LEN;
+	data_len = be16_to_cpu(tag->tag_len) + TAG_HDR_LEN;
 	if (skb_tailroom(skb) < data_len) {
 		_DEBUG_ERR("skb_tailroom() failed in add SID tag!\n");
 		return -1;

Signature · 1 line

2.32.0

Aakash Hemadri

author

2021-08-23 17:00 UTC #2 lore raw

PATCH v4 2/5 staging: r8188eu: cast to restricted __be32

In reply to Aakash Hemadri

Fix sparse warning:

Quoted 1 line

rtw_br_ext.c:836:54: warning: cast to restricted __be32

dhpch->cookie is be32, change it's type.

Signed-off-by Aakash Hemadri <[email protected]>

---

I had incorrectly removed bytsewapping without considering the hardware runs in big endian rather change type to __be32 as sugessted by Larry.

 drivers/staging/r8188eu/core/rtw_br_ext.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/staging/r8188eu/core/rtw_br_ext.c +2 -2

index f6d1f6029ec3..f65d94bfa286 100644
@@ -649,7 +649,7 @@ struct dhcpMessage {
 	u_int8_t chaddr[16];
 	u_int8_t sname[64];
 	u_int8_t file[128];
-	u_int32_t cookie;
+	__be32 cookie;
 	u_int8_t options[308]; /* 312 - cookie */
 };

@@ -671,7 +671,7 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb)
 				    (udph->dest == __constant_htons(SERVER_PORT))) { /*  DHCP request */
 					struct dhcpMessage *dhcph =
 						(struct dhcpMessage *)((size_t)udph + sizeof(struct udphdr));
-					u32 cookie = be32_to_cpu((__be32)dhcph->cookie);
+					u32 cookie = be32_to_cpu(dhcph->cookie);
 
 					if (cookie == DHCP_MAGIC) { /*  match magic word */
 						if (!(dhcph->flags & htons(BROADCAST_FLAG))) {

Signature · 1 line

2.32.0

Aakash Hemadri

author

2021-08-23 17:00 UTC #3 lore raw

PATCH v4 3/5 staging: r8188eu: incorrect type in csum_ipv6_magic

In reply to Aakash Hemadri

Fix sparse warning:

Quoted 2 lines

rtw_br_ext.c:771:84: got restricted __be16 [usertype] payload_len
rtw_br_ext.c:773:110: warning: incorrect type in argument 2

(different base types)

Quoted 2 lines

rtw_br_ext.c:773:110: expected int len
rtw_br_ext.c:773:110: got restricted __be16 [usertype] payload_len

csum_ipv6_magic and csum_partial expect int len not __be16, use be16_to_cpu()

Signed-off-by Aakash Hemadri <[email protected]>

---

No changes

 drivers/staging/r8188eu/core/rtw_br_ext.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/staging/r8188eu/core/rtw_br_ext.c +2 -2

index f65d94bfa286..26606093a3c3 100644
@@ -615,9 +615,9 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
 						struct icmp6hdr  *hdr = (struct icmp6hdr *)(skb->data + ETH_HLEN + sizeof(*iph));
 						hdr->icmp6_cksum = 0;
 						hdr->icmp6_cksum = csum_ipv6_magic(&iph->saddr, &iph->daddr,
-										iph->payload_len,
+										be16_to_cpu(iph->payload_len),
 										IPPROTO_ICMPV6,
-										csum_partial((__u8 *)hdr, iph->payload_len, 0));
+										csum_partial((__u8 *)hdr, be16_to_cpu(iph->payload_len), 0));
 					}
 				}
 			}

Signature · 1 line

2.32.0

Aakash Hemadri

author

2021-08-23 17:00 UTC #6 lore raw

PATCH v4 4/5 staging: r8188eu: restricted __be16 degrades to int

In reply to Aakash Hemadri

Fix sparse warning:

Quoted 4 lines

rtw_br_ext.c:839:70: warning: restricted __be16 degrades to integer rtw_br_ext.c:845:70: warning: invalid assignment: |=
rtw_br_ext.c:845:70: left side has type unsigned short
rtw_br_ext.c:845:70: right side has type restricted __be16

dhcp->flag is be16. Change htons() to cpu_to_be16() for clarity.

Signed-off-by Aakash Hemadri <[email protected]>

---

Incorrectly removed htons() to satisfy sparse warning. Instead changed to appropriate type and changed htons to cpu_to_be16 for clarity.

 drivers/staging/r8188eu/core/rtw_br_ext.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/staging/r8188eu/core/rtw_br_ext.c +3 -3

index 26606093a3c3..83a4594a4214 100644
@@ -641,7 +641,7 @@ struct dhcpMessage {
 	u_int8_t hops;
 	u_int32_t xid;
 	u_int16_t secs;
-	u_int16_t flags;
+	__be16 flags;
 	u_int32_t ciaddr;
 	u_int32_t yiaddr;
 	u_int32_t siaddr;

@@ -674,13 +674,13 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb)
 					u32 cookie = be32_to_cpu(dhcph->cookie);
 
 					if (cookie == DHCP_MAGIC) { /*  match magic word */
-						if (!(dhcph->flags & htons(BROADCAST_FLAG))) {
+						if (!(dhcph->flags & cpu_to_be16(BROADCAST_FLAG))) {
 							/*  if not broadcast */
 							register int sum = 0;
 
 							DEBUG_INFO("DHCP: change flag of DHCP request to broadcast.\n");
 							/*  or BROADCAST flag */
-							dhcph->flags |= htons(BROADCAST_FLAG);
+							dhcph->flags |= cpu_to_be16(BROADCAST_FLAG);
 							/*  recalculate checksum */
 							sum = ~(udph->check) & 0xffff;
 							sum += be16_to_cpu(dhcph->flags);

Signature · 1 line

2.32.0

Aakash Hemadri

author

2021-08-23 17:00 UTC #7 lore raw

PATCH v4 5/5 staging: r8188eu: incorrect type in assignment

In reply to Aakash Hemadri

Fix sparse warning:

Quoted 1 line

rtw_br_ext.c:516:57: warning: incorrect type in assignment

(different base types)

Quoted 2 lines

rtw_br_ext.c:516:57: expected unsigned short
rtw_br_ext.c:516:57: got restricted __be16 [usertype]

*pMagic holds __be16 change it's type to __be16

Signed-off-by Aakash Hemadri <[email protected]>

---

Incorrectly removed htons() to satisfy sparse warning. Instead make pMagic use the appropriate bid endian type

 drivers/staging/r8188eu/core/rtw_br_ext.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/staging/r8188eu/core/rtw_br_ext.c +2 -2

index 83a4594a4214..14cf13516d34 100644
@@ -474,7 +474,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
 		/*                Handle PPPoE frame                 */
 		/*---------------------------------------------------*/
 		struct pppoe_hdr *ph = (struct pppoe_hdr *)(skb->data + ETH_HLEN);
-		unsigned short *pMagic;
+		__be16 *pMagic;
 
 		switch (method) {
 		case NAT25_CHECK:

@@ -512,7 +512,7 @@ int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method)
 						tag->tag_len = htons(MAGIC_CODE_LEN+RTL_RELAY_TAG_LEN+old_tag_len);
 
 						/*  insert the magic_code+client mac in relay tag */
-						pMagic = (unsigned short *)tag->tag_data;
+						pMagic = (__be16 *)tag->tag_data;
 						*pMagic = htons(MAGIC_CODE);
 						memcpy(tag->tag_data+MAGIC_CODE_LEN, skb->data+ETH_ALEN, ETH_ALEN);

Signature · 1 line

2.32.0

David Laight

2021-08-25 09:23 UTC #4 lore raw

RE: [PATCH v4 3/5] staging: r8188eu: incorrect type in csum_ipv6_magic

In reply to Aakash Hemadri

From: Aakash Hemadri

Quoted 11 lines

Sent: 23 August 2021 18:00
Fix sparse warning:
Quoted 2 lines
rtw_br_ext.c:771:84: got restricted __be16 [usertype] payload_len
rtw_br_ext.c:773:110: warning: incorrect type in argument 2
(different base types)
Quoted 2 lines
rtw_br_ext.c:773:110: expected int len
rtw_br_ext.c:773:110: got restricted __be16 [usertype] payload_len
csum_ipv6_magic and csum_partial expect int len not __be16, use be16_to_cpu()

This is a behaviour change on LE systems.
Even if you are testing on BE the commit message should say that it fixes the length passed to the checksum code and was detected by sparse.

...

drivers/staging/r8188eu/core/rtw_br_ext.c int nat25_db_handle(struct adapter *priv, struct sk_buff *skb, int method) · line 620

Quoted 6 lines

drivers/staging/r8188eu/core/rtw_br_ext.c +2 -2

 			hdr->icmp6_cksum = csum_ipv6_magic(&iph->saddr, &iph->daddr,
-						iph->payload_len,
+						be16_to_cpu(iph->payload_len),
 						IPPROTO_ICMPV6,
-						csum_partial((__u8 *)hdr, iph->payload_len, 0));
+						csum_partial((__u8 *)hdr, be16_to_cpu(iph->payload_len), 0));

Oh - that code is indented far too many times.
I had to delete a load of tabs to stop my mail crapping it.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)

Aakash Hemadri

author

2021-08-28 08:45 UTC #5 lore raw

Re: [PATCH v4 3/5] staging: r8188eu: incorrect type in csum_ipv6_magic

Quoted 10 lines · David Laight

From: Aakash Hemadri
Quoted 9 lines
Sent: 23 August 2021 18:00
Fix sparse warning:
Quoted 2 lines
rtw_br_ext.c:771:84: got restricted __be16 [usertype] payload_len
rtw_br_ext.c:773:110: warning: incorrect type in argument 2
(different base types)
Quoted 2 lines
rtw_br_ext.c:773:110: expected int len
rtw_br_ext.c:773:110: got restricted __be16 [usertype] payload_len

Quoted 8 lines

Quoted 2 lines
csum_ipv6_magic and csum_partial expect int len not __be16, use be16_to_cpu()
This is a behaviour change on LE systems.
Even if you are testing on BE the commit message should say that it fixes the length passed to the checksum code and was detected by sparse.

Thanks for the note david.

-Aakash

Aakash Hemadri

author

2021-08-28 08:50 UTC #8 lore raw

In reply to Aakash Hemadri

Quoted 19 lines · Aakash Hemadri

Hi,
This patch series fixes some sparse warnings in rtw_br_ext.c
Changes in v3 -> v4
- Added this changelog, as requested by Greg's patch bot
Changes in v2 -> v3
- Fixed incorrect usage/removal of endian swaps and checks
Changes in v1 -> v2
- Split patch
Aakash Hemadri (5):
staging: r8188eu: restricted __be16 degrades to int
staging: r8188eu: cast to restricted __be32
staging: r8188eu: incorrect type in csum_ipv6_magic
staging: r8188eu: restricted __be16 degrades to int
staging: r8188eu: incorrect type in assignment

Quoted 8 lines

 drivers/staging/r8188eu/core/rtw_br_ext.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
base-commit: 093991aaadf0fbb34184fa37a46e7a157da3f386
Signature · 1 line
2.32.0

Hello greg, if there are any issues with the patchset do let me know, Larry & Philip acked the previous ver of this patch.

I'd rather not add noise to the ml unnecessarily.
Also this patchset doesn't apply cleanly to the current staging-testing I can resend fixing that if that's the issue.

-Aakash

Greg Kroah-Hartman

maintainer

2021-08-28 09:44 UTC #9 lore raw

Quoted 29 lines · Aakash Hemadri

Quoted 19 lines · Aakash Hemadri
Hi,
This patch series fixes some sparse warnings in rtw_br_ext.c
Changes in v3 -> v4
- Added this changelog, as requested by Greg's patch bot
Changes in v2 -> v3
- Fixed incorrect usage/removal of endian swaps and checks
Changes in v1 -> v2
- Split patch
Aakash Hemadri (5):
staging: r8188eu: restricted __be16 degrades to int
staging: r8188eu: cast to restricted __be32
staging: r8188eu: incorrect type in csum_ipv6_magic
staging: r8188eu: restricted __be16 degrades to int
staging: r8188eu: incorrect type in assignment
Quoted 8 lines
 drivers/staging/r8188eu/core/rtw_br_ext.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
base-commit: 093991aaadf0fbb34184fa37a46e7a157da3f386
Signature · 1 line
2.32.0

Quoted 6 lines

Hello greg, if there are any issues with the patchset do let me know, Larry & Philip acked the previous ver of this patch.
I'd rather not add noise to the ml unnecessarily.
Also this patchset doesn't apply cleanly to the current staging-testing I can resend fixing that if that's the issue.

Please fix up and resend, my staging review queue is empty.

thanks,

greg k-h