* [patch] net/sched: potential data corruption
@ 2010-07-13 13:21 Dan Carpenter
2010-07-13 15:58 ` jamal
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2010-07-13 13:21 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David S. Miller, Stephen Hemminger, netdev, kernel-janitors,
matthew
The reset_policy() does:
memset(d->tcfd_defdata, 0, SIMP_MAX_DATA);
strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
In the original code, the size of d->tcfd_defdata wasn't fixed and if
strlen(defdata) was less than 31, reset_policy() would cause memory
corruption.
Please Note: The original alloc_defdata() assumes defdata is 32
characters and a NUL terminator while reset_policy() assumes defdata is
31 characters and a NUL. This patch updates alloc_defdata() to match
reset_policy() (ie a shorter string). I'm not very familiar with this
code so please review carefully.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 1b4bc69..4a1d640 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -73,10 +73,10 @@ static int tcf_simp_release(struct tcf_defact *d, int bind)
static int alloc_defdata(struct tcf_defact *d, char *defdata)
{
- d->tcfd_defdata = kstrndup(defdata, SIMP_MAX_DATA, GFP_KERNEL);
+ d->tcfd_defdata = kzalloc(SIMP_MAX_DATA, GFP_KERNEL);
if (unlikely(!d->tcfd_defdata))
return -ENOMEM;
-
+ strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
return 0;
}
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [patch] net/sched: potential data corruption
2010-07-13 13:21 [patch] net/sched: potential data corruption Dan Carpenter
@ 2010-07-13 15:58 ` jamal
2010-07-15 0:56 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: jamal @ 2010-07-13 15:58 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Stephen Hemminger, netdev, kernel-janitors,
matthew
On Tue, 2010-07-13 at 15:21 +0200, Dan Carpenter wrote:
> The reset_policy() does:
> memset(d->tcfd_defdata, 0, SIMP_MAX_DATA);
> strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
>
> In the original code, the size of d->tcfd_defdata wasn't fixed and if
> strlen(defdata) was less than 31, reset_policy() would cause memory
> corruption.
>
> Please Note: The original alloc_defdata() assumes defdata is 32
> characters and a NUL terminator while reset_policy() assumes defdata is
> 31 characters and a NUL. This patch updates alloc_defdata() to match
> reset_policy() (ie a shorter string). I'm not very familiar with this
> code so please review carefully.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Acked-by: Jamal Hadi Salim <hadi@cyberus.ca>
cheers,
jamal
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] net/sched: potential data corruption
2010-07-13 15:58 ` jamal
@ 2010-07-15 0:56 ` David Miller
0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2010-07-15 0:56 UTC (permalink / raw)
To: hadi; +Cc: error27, shemminger, netdev, kernel-janitors, matthew
From: jamal <hadi@cyberus.ca>
Date: Tue, 13 Jul 2010 11:58:14 -0400
> On Tue, 2010-07-13 at 15:21 +0200, Dan Carpenter wrote:
>> The reset_policy() does:
>> memset(d->tcfd_defdata, 0, SIMP_MAX_DATA);
>> strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
>>
>> In the original code, the size of d->tcfd_defdata wasn't fixed and if
>> strlen(defdata) was less than 31, reset_policy() would cause memory
>> corruption.
>>
>> Please Note: The original alloc_defdata() assumes defdata is 32
>> characters and a NUL terminator while reset_policy() assumes defdata is
>> 31 characters and a NUL. This patch updates alloc_defdata() to match
>> reset_policy() (ie a shorter string). I'm not very familiar with this
>> code so please review carefully.
>>
>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
>
> Acked-by: Jamal Hadi Salim <hadi@cyberus.ca>
Applied, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-07-15 0:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-13 13:21 [patch] net/sched: potential data corruption Dan Carpenter
2010-07-13 15:58 ` jamal
2010-07-15 0:56 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).