Skip to contentSkip to conversation

net/intel/idpf and net/intel/cpfl: four byte store into a bool

DPDK/ethdev Bug 2042 In review [email protected] 3 participants 6 messages Sep 13 – Sep 18 Network drivers

lore
Share & tools
lore
#0 lore raw

https://bugs.dpdk.org/show_bug.cgi?id=2042

Bug ID: 2042
Summary: net/intel/idpf and net/intel/cpfl: four byte store
into a bool
Product: DPDK
Version: 26.11
Hardware: All
OS: All
Status: UNCONFIRMED
Severity: normal
Priority: Normal
Component: ethdev
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---

drivers/net/intel/idpf/idpf_ethdev.c:1248 drivers/net/intel/cpfl/cpfl_ethdev.c (same handler, copied)

parse_bool() stores an int:

        int *i = args;
        ...
        *i = num;

Both callers in each driver pass the address of a bool:

        drivers/net/intel/idpf/idpf_common_device.h:104
                bool is_tx_singleq;
                bool is_rx_singleq;

so "tx_single=1" or "rx_single=1" writes four bytes into a one byte field.

Fixes 549343c25db8 ("net/idpf: support device initialization")
Fixes 2d823ecd671c ("net/cpfl: support device initialization")
Signature · 2 lines
You are receiving this mail because:
You are the assignee for the bug.
#1 lore raw

PATCH 1/2 net/idpf: fix overflow on store of single-queue settings

The Rx and Tx single queue settings in idpf driver are both boolean values, but the parse_bool function writes to them as int. This causes an extra 3 bytes to be written beyond the variable itself. Fix by changing store type to bool.

Bugzilla ID: 2042

Fixes 549343c25db8 ("net/idpf: support device initialization")

===
Note: this fix is superceded by the kvargs numeric args patchset [1]. However, that patchset relies on new kvargs features unlikely to be backported, so this standalone fix is useful for backporting or if the whole other set doesn't make the 26.11 release.

[1] https://patches.dpdk.org/project/dpdk/list/?series=39247

---
 drivers/net/intel/idpf/idpf_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/intel/idpf/idpf_ethdev.c +2 -2
index c13505416a..9fd5b28adb 100644
@@ -1247,7 +1247,7 @@ parse_vport(const char *key, const char *value, void *args)
 static int
 parse_bool(const char *key, const char *value, void *args)
 {
-	int *i = args;
+	bool *i = args;
 	char *end;
 	int num;
 
@@ -1261,7 +1261,7 @@ parse_bool(const char *key, const char *value, void *args)
 		return -EINVAL;
 	}
 
-	*i = num;
+	*i = (num == 1);
 	return 0;
 }
 
Signature · 1 line
2.53.0
#2 lore raw

PATCH 2/2 net/cpfl: fix overflow on store of single-queue settings

The Rx and Tx single queue settings in cpfl driver are both boolean values, but the parse_bool function writes to them as int. This causes an extra 3 bytes to be written beyond the variable itself. Fix by changing store type to bool.

Bugzilla ID: 2042

Fixes 2d823ecd671c ("net/cpfl: support device initialization")

===
Note: this fix is superceded by the kvargs numeric args patchset [1]. However, that patchset relies on new kvargs features unlikely to be backported, so this standalone fix is useful for backporting or if the whole other set doesn't make the 26.11 release.

[1] https://patches.dpdk.org/project/dpdk/list/?series=39247

---
 drivers/net/intel/cpfl/cpfl_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/intel/cpfl/cpfl_ethdev.c +2 -2
index 4315adb68c..b1b7637f48 100644
@@ -1448,7 +1448,7 @@ parse_vport(const char *key, const char *value, void *args)
 static int
 parse_bool(const char *key, const char *value, void *args)
 {
-	int *i = args;
+	bool *i = args;
 	char *end;
 	int num;
 
@@ -1462,7 +1462,7 @@ parse_bool(const char *key, const char *value, void *args)
 		return -EINVAL;
 	}
 
-	*i = num;
+	*i = (num == 1);
 	return 0;
 }
 
Signature · 1 line
2.53.0
Acked-by on v1 1/2
#4 lore raw

RE: [PATCH 1/2] net/idpf: fix overflow on store of single-queue settings

In reply to Bruce Richardson

The Rx and Tx single queue settings in idpf driver are both boolean values, but the parse_bool function writes to them as int. This causes an extra 3 bytes to be written beyond the variable itself. Fix by changing store type to bool.

Bugzilla ID: 2042

Fixes 549343c25db8 ("net/idpf: support device initialization")

Note: this fix is superceded by the kvargs numeric args patchset [1].
However, that patchset relies on new kvargs features unlikely to be backported, so this standalone fix is useful for backporting or if the whole other set doesn't make the 26.11 release.

[1] https://patches.dpdk.org/project/dpdk/list/?series=39247

---
 drivers/net/intel/idpf/idpf_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/intel/idpf/idpf_ethdev.c +2 -2
index c13505416a..9fd5b28adb 100644
@@ -1247,7 +1247,7 @@ parse_vport(const char *key, const char *value, void *args)  static int  parse_bool(const char *key, const char *value, void *args)  {
-	int *i = args;
+	bool *i = args;
 	char *end;
 	int num;
 
@@ -1261,7 +1261,7 @@ parse_bool(const char *key, const char *value, void *args)
 		return -EINVAL;
 	}
 
-	*i = num;
+	*i = (num == 1);
 	return 0;
 }
 

--

Looks good to me,

2.53.0

Acked-by on v1 2/2
#3 lore raw

RE: [PATCH 2/2] net/cpfl: fix overflow on store of single-queue settings

In reply to Bruce Richardson

The Rx and Tx single queue settings in cpfl driver are both boolean values, but the parse_bool function writes to them as int. This causes an extra 3 bytes to be written beyond the variable itself. Fix by changing store type to bool.

Bugzilla ID: 2042

Fixes 2d823ecd671c ("net/cpfl: support device initialization")

Note: this fix is superceded by the kvargs numeric args patchset [1].
However, that patchset relies on new kvargs features unlikely to be backported, so this standalone fix is useful for backporting or if the whole other set doesn't make the 26.11 release.

[1] https://patches.dpdk.org/project/dpdk/list/?series=39247

---
 drivers/net/intel/cpfl/cpfl_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/intel/cpfl/cpfl_ethdev.c +2 -2
index 4315adb68c..b1b7637f48 100644
@@ -1448,7 +1448,7 @@ parse_vport(const char *key, const char *value, void *args)  static int  parse_bool(const char *key, const char *value, void *args)  {
-	int *i = args;
+	bool *i = args;
 	char *end;
 	int num;
 
@@ -1462,7 +1462,7 @@ parse_bool(const char *key, const char *value, void *args)
 		return -EINVAL;
 	}
 
-	*i = num;
+	*i = (num == 1);
 	return 0;
 }
 

--

Looks good to me,

2.53.0

#5 lore raw

Re: [PATCH 1/2] net/idpf: fix overflow on store of single-queue settings

In reply to Shetty, Praveen
Quoted 37 lines · Praveen

The Rx and Tx single queue settings in idpf driver are both boolean values, but the parse_bool function writes to them as int. This causes an extra 3 bytes to be written beyond the variable itself. Fix by changing store type to bool.

Bugzilla ID: 2042

Fixes 549343c25db8 ("net/idpf: support device initialization")

Note: this fix is superceded by the kvargs numeric args patchset [1].
However, that patchset relies on new kvargs features unlikely to be backported, so this standalone fix is useful for backporting or if the whole other set doesn't make the 26.11 release.

[1] https://patches.dpdk.org/project/dpdk/list/?series=39247

---
 drivers/net/intel/idpf/idpf_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/intel/idpf/idpf_ethdev.c +2 -3
index c13505416a..9fd5b28adb 100644
@@ -1247,7 +1247,7 @@ parse_vport(const char *key, const char *value, void *args)  static int  parse_bool(const char *key, const char *value, void *args)  {
-	int *i = args;
+	bool *i = args;
 	char *end;
 	int num;
 
@@ -1261,7 +1261,7 @@ parse_bool(const char *key, const char *value, void *args)
 		return -EINVAL;
 	}
 
-	*i = num;
+	*i = (num == 1);
 	return 0;
 }
 
--
Quoted 3 lines

Looks good to me,

Thanks.

Both patches squashed into one and applied to dpdk-next-net-intel.

/Bruce