Coverity complains about implicit sign conversions though it can't happen.
250 if (dp_packet_gso_partial_nr_segs(p) != 1) {
CID 556379: (#1 of 4): Unintended sign extension (SIGN_EXTENSION)
sign_extension: Suspicious implicit sign extension: tso_segsz with type
uint16_t (16 bits, unsigned) is promoted in (n_segs - 1) * tso_segsz to
type int (32 bits, signed), then sign-extended to type unsigned long
(64 bits, unsigned). If (n_segs - 1) * tso_segsz is greater than
0x7FFFFFFF, the upper bits of the result will all be 1.
251 dp_packet_set_size(p, hdr_len + (n_segs - 1) * tso_segsz);
In practice, the number of segments is > 1 in this part of the code, but
the current helpers returning a signed int may confuse coverity (and
some compilers?).
Switch to unsigned int for referring to segment count and add a check
on an (impossible) n_segs == 0.
Fixes: ef762327f6d3 ("dp-packet-gso: Refactor software segmentation code.")
Signed-off-by: David Marchand <[email protected]>
Reviewed-by: Eelco Chaudron <[email protected]>
Tested-by: Eelco Chaudron <[email protected]>
---
lib/dp-packet-gso.c | 15 ++++++++-------
lib/dp-packet-gso.h | 4 ++--
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c
index 9076c674a1..bceb851fb9 100644
--- a/lib/dp-packet-gso.c
+++ b/lib/dp-packet-gso.c
@@ -62,7 +62,7 @@ dp_packet_gso_seg_new(const struct dp_packet *p, size_t
hdr_len,
}
/* Returns the calculated number of TCP segments in packet 'p'. */
-int
+unsigned int
dp_packet_gso_nr_segs(struct dp_packet *p)
{
uint16_t segsz = dp_packet_get_tso_segsz(p);
@@ -81,7 +81,7 @@ dp_packet_gso_nr_segs(struct dp_packet *p)
* packet (up to the final number of segments on the wire).
* If there is still some data left, we need an extra "little" packet
* (shorter than tso_segsz). */
-int
+unsigned int
dp_packet_gso_partial_nr_segs(struct dp_packet *p)
{
if ((dp_packet_tunnel_geneve(p) || dp_packet_tunnel_vxlan(p))
@@ -95,8 +95,9 @@ dp_packet_gso_partial_nr_segs(struct dp_packet *p)
}
static void
-dp_packet_gso_update_segment(struct dp_packet *seg, int seg_no, int n_segs,
- uint16_t tso_segsz, bool udp_tnl, bool gre_tnl)
+dp_packet_gso_update_segment(struct dp_packet *seg, unsigned int seg_no,
+ unsigned int n_segs, uint16_t tso_segsz,
+ bool udp_tnl, bool gre_tnl)
{
struct tcp_header *tcp_hdr;
struct ip_header *ip_hdr;
@@ -180,12 +181,12 @@ dp_packet_gso__(struct dp_packet *p, struct
dp_packet_batch **batches,
{
struct dp_packet_batch *curr_batch = *batches;
struct dp_packet *seg;
+ unsigned int n_segs;
uint16_t tso_segsz;
size_t data_len;
size_t hdr_len;
bool udp_tnl;
bool gre_tnl;
- int n_segs;
tso_segsz = dp_packet_get_tso_segsz(p);
ovs_assert(tso_segsz);
@@ -200,7 +201,7 @@ dp_packet_gso__(struct dp_packet *p, struct dp_packet_batch
**batches,
}
dp_packet_batch_add(curr_batch, p);
- if (n_segs == 1) {
+ if (n_segs <= 1) {
goto out;
}
@@ -221,7 +222,7 @@ dp_packet_gso__(struct dp_packet *p, struct dp_packet_batch
**batches,
goto first_seg;
}
- for (int i = 1; i < n_segs - 1; i++) {
+ for (unsigned int i = 1; i < n_segs - 1; i++) {
seg = dp_packet_gso_seg_new(p, hdr_len, hdr_len + i * tso_segsz,
tso_segsz);
dp_packet_gso_update_segment(seg, i, n_segs, tso_segsz, udp_tnl,
diff --git a/lib/dp-packet-gso.h b/lib/dp-packet-gso.h
index ed55a11d79..8d16179026 100644
--- a/lib/dp-packet-gso.h
+++ b/lib/dp-packet-gso.h
@@ -18,8 +18,8 @@
#define DP_PACKET_GSO_H 1
void dp_packet_gso(struct dp_packet *, struct dp_packet_batch **);
-int dp_packet_gso_nr_segs(struct dp_packet *);
+unsigned int dp_packet_gso_nr_segs(struct dp_packet *);
void dp_packet_gso_partial(struct dp_packet *, struct dp_packet_batch **);
-int dp_packet_gso_partial_nr_segs(struct dp_packet *);
+unsigned int dp_packet_gso_partial_nr_segs(struct dp_packet *);
#endif /* dp-packet-gso.h */
--
2.52.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
