* scatterlist.c: bug in sg_next()?
@ 2008-09-28 15:15 Leon Woestenberg
2008-09-28 15:28 ` Boaz Harrosh
0 siblings, 1 reply; 3+ messages in thread
From: Leon Woestenberg @ 2008-09-28 15:15 UTC (permalink / raw)
To: FUJITA Tomonori, linux-kernel, James.Bottomley, jens.axboe
Hello,
I was code-inspecting 2.6.27-r7 through git web, when I came across this:
In sg_next(), after following a chain_ptr, a few more checks should be
performed.
The rare case exists that the first entry in the chained list is a
last marker, in case NULL must be returned.
Can someone confirm and cook a patch?
struct scatterlist *sg_next(struct scatterlist *sg)
{
if (sg_is_last(sg))
return NULL;
sg++;
if (unlikely(sg_is_chain(sg))) {
sg = sg_chain_ptr(sg);
+#ifdef CONFIG_DEBUG_SG
+ BUG_ON(sg->sg_magic != SG_MAGIC);
+#endif
+ if (sg_is_last(sg))
+ return NULL;
}
return sg;
}
Signed-off-by: Leon Woestenberg <leon@sidebranch.com>
Regards,
--
Leon
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: scatterlist.c: bug in sg_next()?
2008-09-28 15:15 scatterlist.c: bug in sg_next()? Leon Woestenberg
@ 2008-09-28 15:28 ` Boaz Harrosh
2008-09-28 15:51 ` Leon Woestenberg
0 siblings, 1 reply; 3+ messages in thread
From: Boaz Harrosh @ 2008-09-28 15:28 UTC (permalink / raw)
To: Leon Woestenberg
Cc: FUJITA Tomonori, linux-kernel, James.Bottomley, jens.axboe
Leon Woestenberg wrote:
> Hello,
>
> I was code-inspecting 2.6.27-r7 through git web, when I came across this:
>
> In sg_next(), after following a chain_ptr, a few more checks should be
> performed.
> The rare case exists that the first entry in the chained list is a
> last marker, in case NULL must be returned.
>
> Can someone confirm and cook a patch?
>
> struct scatterlist *sg_next(struct scatterlist *sg)
> {
> if (sg_is_last(sg))
> return NULL;
> sg++;
> if (unlikely(sg_is_chain(sg))) {
> sg = sg_chain_ptr(sg);
> +#ifdef CONFIG_DEBUG_SG
> + BUG_ON(sg->sg_magic != SG_MAGIC);
> +#endif
> + if (sg_is_last(sg))
> + return NULL;
> }
> return sg;
> }
>
> Signed-off-by: Leon Woestenberg <leon@sidebranch.com>
>
> Regards,
No! the last marker is set on a valid sg entry. Only it's next
is no longer valid. So the check at the top is for the Next-sg
not the passed-in-sg. What you thought of is a NULL terminating
sg-list. The end marker is so to save the extra NULL entry.
Boaz
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: scatterlist.c: bug in sg_next()?
2008-09-28 15:28 ` Boaz Harrosh
@ 2008-09-28 15:51 ` Leon Woestenberg
0 siblings, 0 replies; 3+ messages in thread
From: Leon Woestenberg @ 2008-09-28 15:51 UTC (permalink / raw)
To: Boaz Harrosh, linux-kernel@vger.kernel.org
Hello,
On Sun, Sep 28, 2008 at 5:28 PM, Boaz Harrosh <bharrosh@panasas.com> wrote:
> Leon Woestenberg wrote:
>> In sg_next(), after following a chain_ptr, a few more checks should be
>> performed.
>> The rare case exists that the first entry in the chained list is a
>> last marker, in case NULL must be returned.
>
> No! the last marker is set on a valid sg entry. Only it's next
> is no longer valid. So the check at the top is for the Next-sg
> not the passed-in-sg. What you thought of is a NULL terminating
> sg-list. The end marker is so to save the extra NULL entry.
>
Ah yes, the lower bit magic. Thanks!
Sorry for the noise.
Regards,
--
Leon
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-09-28 15:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-28 15:15 scatterlist.c: bug in sg_next()? Leon Woestenberg
2008-09-28 15:28 ` Boaz Harrosh
2008-09-28 15:51 ` Leon Woestenberg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.