* [patch] nfs: check kmalloc() return
@ 2010-10-28 4:44 Dan Carpenter
2010-10-28 7:16 ` walter harms
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2010-10-28 4:44 UTC (permalink / raw)
To: Trond Myklebust; +Cc: linux-nfs, kernel-janitors
The decode_and_add_ds() should return NULL on failure.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
index 51fe64a..098113c 100644
--- a/fs/nfs/nfs4filelayoutdev.c
+++ b/fs/nfs/nfs4filelayoutdev.c
@@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
goto out_err;
}
buf = kmalloc(rlen + 1, GFP_KERNEL);
+ if (!buf)
+ goto out_err;
buf[rlen] = '\0';
memcpy(buf, r_addr, rlen);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch] nfs: check kmalloc() return
2010-10-28 4:44 [patch] nfs: check kmalloc() return Dan Carpenter
@ 2010-10-28 7:16 ` walter harms
2010-10-28 13:41 ` Trond Myklebust
0 siblings, 1 reply; 6+ messages in thread
From: walter harms @ 2010-10-28 7:16 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Trond Myklebust, linux-nfs, kernel-janitors
Dan Carpenter schrieb:
> The decode_and_add_ds() should return NULL on failure.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
> index 51fe64a..098113c 100644
> --- a/fs/nfs/nfs4filelayoutdev.c
> +++ b/fs/nfs/nfs4filelayoutdev.c
> @@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
> goto out_err;
> }
> buf = kmalloc(rlen + 1, GFP_KERNEL);
> + if (!buf)
> + goto out_err;
> buf[rlen] = '\0';
> memcpy(buf, r_addr, rlen);
>
it seems that r_addr is a string, then kstdup() is emulated here.
re,
wh
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] nfs: check kmalloc() return
2010-10-28 7:16 ` walter harms
@ 2010-10-28 13:41 ` Trond Myklebust
2010-10-28 14:14 ` Benny Halevy
0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2010-10-28 13:41 UTC (permalink / raw)
To: wharms; +Cc: Dan Carpenter, linux-nfs, kernel-janitors
On Thu, 2010-10-28 at 09:16 +0200, walter harms wrote:
>
> Dan Carpenter schrieb:
> > The decode_and_add_ds() should return NULL on failure.
> >
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> >
> > diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
> > index 51fe64a..098113c 100644
> > --- a/fs/nfs/nfs4filelayoutdev.c
> > +++ b/fs/nfs/nfs4filelayoutdev.c
> > @@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
> > goto out_err;
> > }
> > buf = kmalloc(rlen + 1, GFP_KERNEL);
> > + if (!buf)
> > + goto out_err;
> > buf[rlen] = '\0';
> > memcpy(buf, r_addr, rlen);
> >
>
> it seems that r_addr is a string, then kstdup() is emulated here.
>
> re,
> wh
Not quite. kstrdup() requires that the argument be a NUL-terminated
string. The above code doesn't.
Trond
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] nfs: check kmalloc() return
2010-10-28 13:41 ` Trond Myklebust
@ 2010-10-28 14:14 ` Benny Halevy
2010-10-28 16:57 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Benny Halevy @ 2010-10-28 14:14 UTC (permalink / raw)
To: Trond Myklebust; +Cc: wharms, Dan Carpenter, linux-nfs, kernel-janitors
On 2010-10-28 15:41, Trond Myklebust wrote:
> On Thu, 2010-10-28 at 09:16 +0200, walter harms wrote:
>>
>> Dan Carpenter schrieb:
>>> The decode_and_add_ds() should return NULL on failure.
>>>
>>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>>>
>>> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
>>> index 51fe64a..098113c 100644
>>> --- a/fs/nfs/nfs4filelayoutdev.c
>>> +++ b/fs/nfs/nfs4filelayoutdev.c
>>> @@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
>>> goto out_err;
>>> }
>>> buf = kmalloc(rlen + 1, GFP_KERNEL);
>>> + if (!buf)
>>> + goto out_err;
>>> buf[rlen] = '\0';
>>> memcpy(buf, r_addr, rlen);
>>>
>>
>> it seems that r_addr is a string, then kstdup() is emulated here.
>>
>> re,
>> wh
>
> Not quite. kstrdup() requires that the argument be a NUL-terminated
> string. The above code doesn't.
Right. kmemdup is the right one.
Benny
>
> Trond
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] nfs: check kmalloc() return
2010-10-28 14:14 ` Benny Halevy
@ 2010-10-28 16:57 ` Dan Carpenter
2010-10-29 7:33 ` Benny Halevy
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2010-10-28 16:57 UTC (permalink / raw)
To: Benny Halevy; +Cc: Trond Myklebust, wharms, linux-nfs, kernel-janitors
On Thu, Oct 28, 2010 at 04:14:12PM +0200, Benny Halevy wrote:
> >>> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
> >>> index 51fe64a..098113c 100644
> >>> --- a/fs/nfs/nfs4filelayoutdev.c
> >>> +++ b/fs/nfs/nfs4filelayoutdev.c
> >>> @@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
> >>> goto out_err;
> >>> }
> >>> buf = kmalloc(rlen + 1, GFP_KERNEL);
> >>> + if (!buf)
> >>> + goto out_err;
> >>> buf[rlen] = '\0';
> >>> memcpy(buf, r_addr, rlen);
> >>>
> >>
> >> it seems that r_addr is a string, then kstdup() is emulated here.
> >>
> >> re,
> >> wh
> >
> > Not quite. kstrdup() requires that the argument be a NUL-terminated
> > string. The above code doesn't.
>
> Right. kmemdup is the right one.
>
We need to duplicate the data and also add a NUL char on the end.
kmemdup() only does the first bit. You could copy one char past the end
so you have space for the NUL but that's not the right idea.
Anyway, I'm out of here for the next few days. :) See you after the
weekend.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] nfs: check kmalloc() return
2010-10-28 16:57 ` Dan Carpenter
@ 2010-10-29 7:33 ` Benny Halevy
0 siblings, 0 replies; 6+ messages in thread
From: Benny Halevy @ 2010-10-29 7:33 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Trond Myklebust, wharms, linux-nfs, kernel-janitors
On 2010-10-28 18:57, Dan Carpenter wrote:
> On Thu, Oct 28, 2010 at 04:14:12PM +0200, Benny Halevy wrote:
>>>>> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
>>>>> index 51fe64a..098113c 100644
>>>>> --- a/fs/nfs/nfs4filelayoutdev.c
>>>>> +++ b/fs/nfs/nfs4filelayoutdev.c
>>>>> @@ -219,6 +219,8 @@ decode_and_add_ds(__be32 **pp, struct inode *inode)
>>>>> goto out_err;
>>>>> }
>>>>> buf = kmalloc(rlen + 1, GFP_KERNEL);
>>>>> + if (!buf)
>>>>> + goto out_err;
>>>>> buf[rlen] = '\0';
>>>>> memcpy(buf, r_addr, rlen);
>>>>>
>>>>
>>>> it seems that r_addr is a string, then kstdup() is emulated here.
>>>>
>>>> re,
>>>> wh
>>>
>>> Not quite. kstrdup() requires that the argument be a NUL-terminated
>>> string. The above code doesn't.
>>
>> Right. kmemdup is the right one.
>>
>
> We need to duplicate the data and also add a NUL char on the end.
You're right.
> kmemdup() only does the first bit. You could copy one char past the end
> so you have space for the NUL but that's not the right idea.
If rlen is divisible by 4 you can't be sure you'll have an extra character to
copy.
So the check you added is probably the simplest solution.
Benny
>
> Anyway, I'm out of here for the next few days. :) See you after the
> weekend.
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-29 7:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-28 4:44 [patch] nfs: check kmalloc() return Dan Carpenter
2010-10-28 7:16 ` walter harms
2010-10-28 13:41 ` Trond Myklebust
2010-10-28 14:14 ` Benny Halevy
2010-10-28 16:57 ` Dan Carpenter
2010-10-29 7:33 ` Benny Halevy
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).