* [patch] exofs: add a cap on the memcpy() size
@ 2012-01-30 7:59 Dan Carpenter
2012-01-30 13:37 ` Boaz Harrosh
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-01-30 7:59 UTC (permalink / raw)
To: kernel-janitors
This data comes from the device, so probably it's fairly trustworthy but
it makes the static checkers happy if we check it.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index d22cd16..755812a 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -529,6 +529,8 @@ static int exofs_devs_2_odi(struct exofs_dt_device_info *dt_dev,
struct osd_dev_info *odi)
{
odi->systemid_len = le32_to_cpu(dt_dev->systemid_len);
+ if (odi->systemid_len > OSD_SYSTEMID_LEN)
+ return -EINVAL;
memcpy(odi->systemid, dt_dev->systemid, odi->systemid_len);
odi->osdname_len = le32_to_cpu(dt_dev->osdname_len);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch] exofs: add a cap on the memcpy() size
2012-01-30 7:59 [patch] exofs: add a cap on the memcpy() size Dan Carpenter
@ 2012-01-30 13:37 ` Boaz Harrosh
2012-01-30 13:44 ` Dan Carpenter
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Boaz Harrosh @ 2012-01-30 13:37 UTC (permalink / raw)
To: kernel-janitors
On 01/30/2012 09:59 AM, Dan Carpenter wrote:
> This data comes from the device, so probably it's fairly trustworthy but
> it makes the static checkers happy if we check it.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/fs/exofs/super.c b/fs/exofs/super.c
> index d22cd16..755812a 100644
> --- a/fs/exofs/super.c
> +++ b/fs/exofs/super.c
> @@ -529,6 +529,8 @@ static int exofs_devs_2_odi(struct exofs_dt_device_info *dt_dev,
> struct osd_dev_info *odi)
> {
> odi->systemid_len = le32_to_cpu(dt_dev->systemid_len);
> + if (odi->systemid_len > OSD_SYSTEMID_LEN)
> + return -EINVAL;
> memcpy(odi->systemid, dt_dev->systemid, odi->systemid_len);
>
> odi->osdname_len = le32_to_cpu(dt_dev->osdname_len);
Hi Dan
I was going over this code and for the life of me I can't remember
why I have dt_dev->systemid_len at all. The ->systemid field
is just a constant 20 bytes buffer that is always there. at all
ends of the spectrum. (Including user-mode mkfs.exofs)
I think my thought was that dt_dev->systemid_len could be
either 20 or zero, for ignoring it.
I think I'd like something like:
- memcpy(odi->systemid, dt_dev->systemid, odi->systemid_len);
+ if (likely(odi->systemid_len))
+ memcpy(odi->systemid, dt_dev->systemid, OSD_SYSTEMID_LEN);
Which should also make the static checkers happy. What do you think?
Thanks
Boaz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] exofs: add a cap on the memcpy() size
2012-01-30 7:59 [patch] exofs: add a cap on the memcpy() size Dan Carpenter
2012-01-30 13:37 ` Boaz Harrosh
@ 2012-01-30 13:44 ` Dan Carpenter
2012-01-30 13:50 ` Boaz Harrosh
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-01-30 13:44 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 399 bytes --]
On Mon, Jan 30, 2012 at 03:37:29PM +0200, Boaz Harrosh wrote:
> I think I'd like something like:
> - memcpy(odi->systemid, dt_dev->systemid, odi->systemid_len);
> + if (likely(odi->systemid_len))
> + memcpy(odi->systemid, dt_dev->systemid, OSD_SYSTEMID_LEN);
>
> Which should also make the static checkers happy. What do you think?
>
Sounds good to me.
regards,
dan carpenter
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] exofs: add a cap on the memcpy() size
2012-01-30 7:59 [patch] exofs: add a cap on the memcpy() size Dan Carpenter
2012-01-30 13:37 ` Boaz Harrosh
2012-01-30 13:44 ` Dan Carpenter
@ 2012-01-30 13:50 ` Boaz Harrosh
2012-01-30 14:01 ` Dan Carpenter
2012-01-30 16:20 ` Boaz Harrosh
4 siblings, 0 replies; 6+ messages in thread
From: Boaz Harrosh @ 2012-01-30 13:50 UTC (permalink / raw)
To: kernel-janitors
On 01/30/2012 03:44 PM, Dan Carpenter wrote:
> On Mon, Jan 30, 2012 at 03:37:29PM +0200, Boaz Harrosh wrote:
>> I think I'd like something like:
>> - memcpy(odi->systemid, dt_dev->systemid, odi->systemid_len);
>> + if (likely(odi->systemid_len))
>> + memcpy(odi->systemid, dt_dev->systemid, OSD_SYSTEMID_LEN);
>>
>> Which should also make the static checkers happy. What do you think?
>>
>
> Sounds good to me.
>
> regards,
> dan carpenter
>
OK I'll make a patch. Please send your review-by
Thanks for catching it
Boaz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] exofs: add a cap on the memcpy() size
2012-01-30 7:59 [patch] exofs: add a cap on the memcpy() size Dan Carpenter
` (2 preceding siblings ...)
2012-01-30 13:50 ` Boaz Harrosh
@ 2012-01-30 14:01 ` Dan Carpenter
2012-01-30 16:20 ` Boaz Harrosh
4 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-01-30 14:01 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 793 bytes --]
On Mon, Jan 30, 2012 at 03:50:05PM +0200, Boaz Harrosh wrote:
> On 01/30/2012 03:44 PM, Dan Carpenter wrote:
> > On Mon, Jan 30, 2012 at 03:37:29PM +0200, Boaz Harrosh wrote:
> >> I think I'd like something like:
> >> - memcpy(odi->systemid, dt_dev->systemid, odi->systemid_len);
> >> + if (likely(odi->systemid_len))
> >> + memcpy(odi->systemid, dt_dev->systemid, OSD_SYSTEMID_LEN);
> >>
> >> Which should also make the static checkers happy. What do you think?
> >>
> >
> > Sounds good to me.
> >
> > regards,
> > dan carpenter
> >
>
> OK I'll make a patch. Please send your review-by
Could you give me a Reported-by cookie instead? No one pays any
special attention to Reviewed-by tags when they come from newbies
like me... :P
regards,
dan carpenter
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] exofs: add a cap on the memcpy() size
2012-01-30 7:59 [patch] exofs: add a cap on the memcpy() size Dan Carpenter
` (3 preceding siblings ...)
2012-01-30 14:01 ` Dan Carpenter
@ 2012-01-30 16:20 ` Boaz Harrosh
4 siblings, 0 replies; 6+ messages in thread
From: Boaz Harrosh @ 2012-01-30 16:20 UTC (permalink / raw)
To: kernel-janitors
On 01/30/2012 04:01 PM, Dan Carpenter wrote:
> On Mon, Jan 30, 2012 at 03:50:05PM +0200, Boaz Harrosh wrote:
>> On 01/30/2012 03:44 PM, Dan Carpenter wrote:
>>> On Mon, Jan 30, 2012 at 03:37:29PM +0200, Boaz Harrosh wrote:
>>>> I think I'd like something like:
>>>> - memcpy(odi->systemid, dt_dev->systemid, odi->systemid_len);
>>>> + if (likely(odi->systemid_len))
>>>> + memcpy(odi->systemid, dt_dev->systemid, OSD_SYSTEMID_LEN);
>>>>
>>>> Which should also make the static checkers happy. What do you think?
>>>>
>>>
>>> Sounds good to me.
>>>
>>> regards,
>>> dan carpenter
>>>
>>
>> OK I'll make a patch. Please send your review-by
>
> Could you give me a Reported-by cookie instead? No one pays any
> special attention to Reviewed-by tags when they come from newbies
> like me... :P
>
> regards,
> dan carpenter
>
Sure, yes
Boaz
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-30 16:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-30 7:59 [patch] exofs: add a cap on the memcpy() size Dan Carpenter
2012-01-30 13:37 ` Boaz Harrosh
2012-01-30 13:44 ` Dan Carpenter
2012-01-30 13:50 ` Boaz Harrosh
2012-01-30 14:01 ` Dan Carpenter
2012-01-30 16:20 ` Boaz Harrosh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox