[ethtool PATCH] ethtool: Resolve use of uninitialized memory in rxclass_get_dev_info

Subsystems: the rest

STALE5141d

4 messages, 2 authors, 2012-07-17 · open the first message on its own page

[ethtool PATCH] ethtool: Resolve use of uninitialized memory in rxclass_get_dev_info

From: Alexander Duyck <hidden>
Date: 2012-07-13 16:55:08

The ethtool function for getting the rule count was not zeroing out the
data field before passing it to the kernel.  As a result the value started
uninitialized and was incorrectly returning a result indicating that
devices supported setting new rule indexes.  In order to correct this I am
adding a one line fix that sets data to zero before we pass the command to
the kernel.

Signed-off-by: Alexander Duyck <redacted>
---

I am resending this since I didn't see any notification that it had been seen.
I also realized that I had not clearly identified that this is an ethtool user
space patch and not an ethtool kernel space patch.

 rxclass.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/rxclass.c b/rxclass.c
index 4d49aa6..e1633a8 100644
--- a/rxclass.c
+++ b/rxclass.c
@@ -207,6 +207,7 @@ static int rxclass_get_dev_info(struct cmd_context *ctx, __u32 *count,
 	int err;
 
 	nfccmd.cmd = ETHTOOL_GRXCLSRLCNT;
+	nfccmd.data = 0;
 	err = send_ioctl(ctx, &nfccmd);
 	*count = nfccmd.rule_cnt;
 	if (driver_select)

Re: [ethtool PATCH] ethtool: Resolve use of uninitialized memory in rxclass_get_dev_info

From: Ben Hutchings <hidden>
Date: 2012-07-16 20:03:40

On Fri, 2012-07-13 at 09:55 -0700, Alexander Duyck wrote:
The ethtool function for getting the rule count was not zeroing out the
data field before passing it to the kernel.  As a result the value started
uninitialized and was incorrectly returning a result indicating that
devices supported setting new rule indexes.  In order to correct this I am
adding a one line fix that sets data to zero before we pass the command to
the kernel.
Right.  For 'get' commands with no parameters (besides the device) the
data copied back to userland is normally zero-initialised and then
filled out by the driver, and I seem to have worked on that assumption.
But because of the odd multiplexing of RX NFC commands
ETHTOOL_GRXCLSRLCNT doesn't work like that.  And for 'my' driver that
didn't matter.  Sorry about that.

(We should really have some explicit documentation of responsibility for
structure initialisation.)
Signed-off-by: Alexander Duyck <redacted>
---

I am resending this since I didn't see any notification that it had been seen.
I also realized that I had not clearly identified that this is an ethtool user
space patch and not an ethtool kernel space patch.
It was perfectly clear and I had queued it up to review but hadn't yet
done so.

Ben.
quoted hunk
 rxclass.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/rxclass.c b/rxclass.c
index 4d49aa6..e1633a8 100644
--- a/rxclass.c
+++ b/rxclass.c
@@ -207,6 +207,7 @@ static int rxclass_get_dev_info(struct cmd_context *ctx, __u32 *count,
 	int err;
 
 	nfccmd.cmd = ETHTOOL_GRXCLSRLCNT;
+	nfccmd.data = 0;
 	err = send_ioctl(ctx, &nfccmd);
 	*count = nfccmd.rule_cnt;
 	if (driver_select)
-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [ethtool PATCH] ethtool: Resolve use of uninitialized memory in rxclass_get_dev_info

From: Alexander Duyck <hidden>
Date: 2012-07-17 00:10:09

On 07/16/2012 01:03 PM, Ben Hutchings wrote:
On Fri, 2012-07-13 at 09:55 -0700, Alexander Duyck wrote:
quoted
The ethtool function for getting the rule count was not zeroing out the
data field before passing it to the kernel.  As a result the value started
uninitialized and was incorrectly returning a result indicating that
devices supported setting new rule indexes.  In order to correct this I am
adding a one line fix that sets data to zero before we pass the command to
the kernel.
Right.  For 'get' commands with no parameters (besides the device) the
data copied back to userland is normally zero-initialised and then
filled out by the driver, and I seem to have worked on that assumption.
But because of the odd multiplexing of RX NFC commands
ETHTOOL_GRXCLSRLCNT doesn't work like that.  And for 'my' driver that
didn't matter.  Sorry about that.

(We should really have some explicit documentation of responsibility for
structure initialisation.)
quoted
Signed-off-by: Alexander Duyck <redacted>
---

I am resending this since I didn't see any notification that it had been seen.
I also realized that I had not clearly identified that this is an ethtool user
space patch and not an ethtool kernel space patch.
It was perfectly clear and I had queued it up to review but hadn't yet
done so.

Ben.
Yeah, that was my mistake.  I thought I hadn't sent it out with the
ethtool prefix when I actually had.

Thanks,

Alex

Re: [ethtool PATCH] ethtool: Resolve use of uninitialized memory in rxclass_get_dev_info

From: Ben Hutchings <hidden>
Date: 2012-07-17 15:32:47

On Mon, 2012-07-16 at 17:10 -0700, Alexander Duyck wrote:
On 07/16/2012 01:03 PM, Ben Hutchings wrote:
quoted
On Fri, 2012-07-13 at 09:55 -0700, Alexander Duyck wrote:
quoted
The ethtool function for getting the rule count was not zeroing out the
data field before passing it to the kernel.  As a result the value started
uninitialized and was incorrectly returning a result indicating that
devices supported setting new rule indexes.  In order to correct this I am
adding a one line fix that sets data to zero before we pass the command to
the kernel.
Right.  For 'get' commands with no parameters (besides the device) the
data copied back to userland is normally zero-initialised and then
filled out by the driver, and I seem to have worked on that assumption.
But because of the odd multiplexing of RX NFC commands
ETHTOOL_GRXCLSRLCNT doesn't work like that.  And for 'my' driver that
didn't matter.  Sorry about that.

(We should really have some explicit documentation of responsibility for
structure initialisation.)
quoted
Signed-off-by: Alexander Duyck <redacted>
---

I am resending this since I didn't see any notification that it had been seen.
I also realized that I had not clearly identified that this is an ethtool user
space patch and not an ethtool kernel space patch.
It was perfectly clear and I had queued it up to review but hadn't yet
done so.

Ben.
Yeah, that was my mistake.  I thought I hadn't sent it out with the
ethtool prefix when I actually had.
So, anyway, I've applied it and just done a bug fix release (3.4.2).

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help