public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] : make sure the buffer head members are zeroed out before using them.
@ 2009-01-20 17:06 Manish Katiyar
  2009-01-25 15:53 ` Manish Katiyar
  0 siblings, 1 reply; 10+ messages in thread
From: Manish Katiyar @ 2009-01-20 17:06 UTC (permalink / raw)
  To: ext4, Theodore Ts'o, cmm; +Cc: mkatiyar

ext2_quota_read doesn't bzeroes tmp_bh before calling ext2_get_block()
where we access the b_size of it. Since it is a local variable it
might contain some garbage. Make sure it is filled with zero before
passing.

Signed-off-by : Manish Katiyar <mkatiyar@gmail.com>
---
 fs/ext2/super.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index da8bdea..d10aa44 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1327,7 +1327,7 @@ static ssize_t ext2_quota_read(struct
super_block *sb, int type, char *data,
 		tocopy = sb->s_blocksize - offset < toread ?
 				sb->s_blocksize - offset : toread;

-		tmp_bh.b_state = 0;
+		memset(&tmp_bh, 0, sizeof(struct buffer_head));
 		err = ext2_get_block(inode, blk, &tmp_bh, 0);
 		if (err < 0)
 			return err;
@@ -1366,7 +1366,7 @@ static ssize_t ext2_quota_write(struct
super_block *sb, int type,
 		tocopy = sb->s_blocksize - offset < towrite ?
 				sb->s_blocksize - offset : towrite;

-		tmp_bh.b_state = 0;
+		memset(&tmp_bh, 0, sizeof(struct buffer_head));
 		err = ext2_get_block(inode, blk, &tmp_bh, 1);
 		if (err < 0)
 			goto out;
-- 
1.5.4.3


Thanks -
Manish

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] : make sure the buffer head members are zeroed out before using them.
  2009-01-20 17:06 [PATCH] : make sure the buffer head members are zeroed out before using them Manish Katiyar
@ 2009-01-25 15:53 ` Manish Katiyar
  2009-01-25 16:15   ` Eric Sandeen
  2009-01-26 12:29   ` Jan Kara
  0 siblings, 2 replies; 10+ messages in thread
From: Manish Katiyar @ 2009-01-25 15:53 UTC (permalink / raw)
  To: ext4, Theodore Ts'o, cmm; +Cc: mkatiyar

On Tue, Jan 20, 2009 at 10:36 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
> ext2_quota_read doesn't bzeroes tmp_bh before calling ext2_get_block()
> where we access the b_size of it. Since it is a local variable it
> might contain some garbage. Make sure it is filled with zero before
> passing.

Hi Ted/mingming,

Any feedback on this ??

Thanks -
Manish

>
> Signed-off-by : Manish Katiyar <mkatiyar@gmail.com>
> ---
>  fs/ext2/super.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index da8bdea..d10aa44 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -1327,7 +1327,7 @@ static ssize_t ext2_quota_read(struct
> super_block *sb, int type, char *data,
>                tocopy = sb->s_blocksize - offset < toread ?
>                                sb->s_blocksize - offset : toread;
>
> -               tmp_bh.b_state = 0;
> +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
>                err = ext2_get_block(inode, blk, &tmp_bh, 0);
>                if (err < 0)
>                        return err;
> @@ -1366,7 +1366,7 @@ static ssize_t ext2_quota_write(struct
> super_block *sb, int type,
>                tocopy = sb->s_blocksize - offset < towrite ?
>                                sb->s_blocksize - offset : towrite;
>
> -               tmp_bh.b_state = 0;
> +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
>                err = ext2_get_block(inode, blk, &tmp_bh, 1);
>                if (err < 0)
>                        goto out;
> --
> 1.5.4.3
>
>
> Thanks -
> Manish
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] : make sure the buffer head members are zeroed out before using them.
  2009-01-25 15:53 ` Manish Katiyar
@ 2009-01-25 16:15   ` Eric Sandeen
  2009-01-25 16:22     ` Manish Katiyar
  2009-01-26 12:29   ` Jan Kara
  1 sibling, 1 reply; 10+ messages in thread
From: Eric Sandeen @ 2009-01-25 16:15 UTC (permalink / raw)
  To: Manish Katiyar; +Cc: ext4, Theodore Ts'o, cmm

Manish Katiyar wrote:
> On Tue, Jan 20, 2009 at 10:36 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
>> ext2_quota_read doesn't bzeroes tmp_bh before calling ext2_get_block()
>> where we access the b_size of it. Since it is a local variable it
>> might contain some garbage. Make sure it is filled with zero before
>> passing.
> 
> Hi Ted/mingming,
> 
> Any feedback on this ??

This looks ok to me, Manish.  I'm curious, did you see this fail in real
life, and if so, what'd the failure look like?

With the change, the tmp_bh bh_size is 0, so maxblocks down the
get_block path is also 0, but I guess that works out ok.

-Eric

> Thanks -
> Manish
> 
>> Signed-off-by : Manish Katiyar <mkatiyar@gmail.com>
>> ---
>>  fs/ext2/super.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
>> index da8bdea..d10aa44 100644
>> --- a/fs/ext2/super.c
>> +++ b/fs/ext2/super.c
>> @@ -1327,7 +1327,7 @@ static ssize_t ext2_quota_read(struct
>> super_block *sb, int type, char *data,
>>                tocopy = sb->s_blocksize - offset < toread ?
>>                                sb->s_blocksize - offset : toread;
>>
>> -               tmp_bh.b_state = 0;
>> +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
>>                err = ext2_get_block(inode, blk, &tmp_bh, 0);
>>                if (err < 0)
>>                        return err;
>> @@ -1366,7 +1366,7 @@ static ssize_t ext2_quota_write(struct
>> super_block *sb, int type,
>>                tocopy = sb->s_blocksize - offset < towrite ?
>>                                sb->s_blocksize - offset : towrite;
>>
>> -               tmp_bh.b_state = 0;
>> +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
>>                err = ext2_get_block(inode, blk, &tmp_bh, 1);
>>                if (err < 0)
>>                        goto out;
>> --
>> 1.5.4.3
>>
>>
>> Thanks -
>> Manish
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" 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] 10+ messages in thread

* Re: [PATCH] : make sure the buffer head members are zeroed out before using them.
  2009-01-25 16:15   ` Eric Sandeen
@ 2009-01-25 16:22     ` Manish Katiyar
  2009-01-25 17:01       ` Eric Sandeen
  2009-01-26 12:32       ` Jan Kara
  0 siblings, 2 replies; 10+ messages in thread
From: Manish Katiyar @ 2009-01-25 16:22 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: ext4, Theodore Ts'o, cmm

On Sun, Jan 25, 2009 at 9:45 PM, Eric Sandeen <sandeen@redhat.com> wrote:
> Manish Katiyar wrote:
>> On Tue, Jan 20, 2009 at 10:36 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
>>> ext2_quota_read doesn't bzeroes tmp_bh before calling ext2_get_block()
>>> where we access the b_size of it. Since it is a local variable it
>>> might contain some garbage. Make sure it is filled with zero before
>>> passing.
>>
>> Hi Ted/mingming,
>>
>> Any feedback on this ??
>
> This looks ok to me, Manish.  I'm curious, did you see this fail in real
> life, and if so, what'd the failure look like?

Actually no......I realised this while going through the code. I was
also wondering why we haven't hit this till now. Since ext{3,4} don't
have this issue, the only reason I can think of is because ext2 with
quota is not very much used or somehow we are lucky.

>
> With the change, the tmp_bh bh_size is 0, so maxblocks down the
> get_block path is also 0, but I guess that works out ok.

Yes, but that is better than having a random garbage. Isn't it ?

Thanks -
Manish

>
> -Eric
>
>> Thanks -
>> Manish
>>
>>> Signed-off-by : Manish Katiyar <mkatiyar@gmail.com>
>>> ---
>>>  fs/ext2/super.c |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
>>> index da8bdea..d10aa44 100644
>>> --- a/fs/ext2/super.c
>>> +++ b/fs/ext2/super.c
>>> @@ -1327,7 +1327,7 @@ static ssize_t ext2_quota_read(struct
>>> super_block *sb, int type, char *data,
>>>                tocopy = sb->s_blocksize - offset < toread ?
>>>                                sb->s_blocksize - offset : toread;
>>>
>>> -               tmp_bh.b_state = 0;
>>> +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
>>>                err = ext2_get_block(inode, blk, &tmp_bh, 0);
>>>                if (err < 0)
>>>                        return err;
>>> @@ -1366,7 +1366,7 @@ static ssize_t ext2_quota_write(struct
>>> super_block *sb, int type,
>>>                tocopy = sb->s_blocksize - offset < towrite ?
>>>                                sb->s_blocksize - offset : towrite;
>>>
>>> -               tmp_bh.b_state = 0;
>>> +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
>>>                err = ext2_get_block(inode, blk, &tmp_bh, 1);
>>>                if (err < 0)
>>>                        goto out;
>>> --
>>> 1.5.4.3
>>>
>>>
>>> Thanks -
>>> Manish
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" 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] 10+ messages in thread

* Re: [PATCH] : make sure the buffer head members are zeroed out before using them.
  2009-01-25 16:22     ` Manish Katiyar
@ 2009-01-25 17:01       ` Eric Sandeen
  2009-01-26 12:32       ` Jan Kara
  1 sibling, 0 replies; 10+ messages in thread
From: Eric Sandeen @ 2009-01-25 17:01 UTC (permalink / raw)
  To: Manish Katiyar; +Cc: ext4, Theodore Ts'o, cmm

Manish Katiyar wrote:
> On Sun, Jan 25, 2009 at 9:45 PM, Eric Sandeen <sandeen@redhat.com> wrote:
>> Manish Katiyar wrote:
>>> On Tue, Jan 20, 2009 at 10:36 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
>>>> ext2_quota_read doesn't bzeroes tmp_bh before calling ext2_get_block()
>>>> where we access the b_size of it. Since it is a local variable it
>>>> might contain some garbage. Make sure it is filled with zero before
>>>> passing.
>>> Hi Ted/mingming,
>>>
>>> Any feedback on this ??
>> This looks ok to me, Manish.  I'm curious, did you see this fail in real
>> life, and if so, what'd the failure look like?
> 
> Actually no......I realised this while going through the code. I was
> also wondering why we haven't hit this till now. Since ext{3,4} don't
> have this issue, the only reason I can think of is because ext2 with
> quota is not very much used or somehow we are lucky.
> 
>> With the change, the tmp_bh bh_size is 0, so maxblocks down the
>> get_block path is also 0, but I guess that works out ok.
> 
> Yes, but that is better than having a random garbage. Isn't it ?

Absolutely; it just struck me as a little odd but it's exactly what
__ext2_get_block does as well, I probably just need to take a closer
look at what it does w/ a 0 max.

-Eric

> Thanks -
> Manish


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] : make sure the buffer head members are zeroed out before using them.
  2009-01-25 15:53 ` Manish Katiyar
  2009-01-25 16:15   ` Eric Sandeen
@ 2009-01-26 12:29   ` Jan Kara
  2009-01-26 12:33     ` Manish Katiyar
  1 sibling, 1 reply; 10+ messages in thread
From: Jan Kara @ 2009-01-26 12:29 UTC (permalink / raw)
  To: Manish Katiyar; +Cc: ext4, Theodore Ts'o, cmm

> On Tue, Jan 20, 2009 at 10:36 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
> > ext2_quota_read doesn't bzeroes tmp_bh before calling ext2_get_block()
> > where we access the b_size of it. Since it is a local variable it
> > might contain some garbage. Make sure it is filled with zero before
> > passing.
> 
> Hi Ted/mingming,
> 
> Any feedback on this ??
  Ops, sorry. I wanted to reply but first I wanted to research more
whether we can set b_size to 0 and then forgot about it. Looking into
other code (e.g. in fs/mpage.c or fs/buffer.c) I think it would be
better to set b_size to sb->s_blocksize and be done with that. Mapping
code does not need anything else set to a deterministic value so using
memset is a bit overkill.

								Honza

> > Signed-off-by : Manish Katiyar <mkatiyar@gmail.com>
> > ---
> >  fs/ext2/super.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> > index da8bdea..d10aa44 100644
> > --- a/fs/ext2/super.c
> > +++ b/fs/ext2/super.c
> > @@ -1327,7 +1327,7 @@ static ssize_t ext2_quota_read(struct
> > super_block *sb, int type, char *data,
> >                tocopy = sb->s_blocksize - offset < toread ?
> >                                sb->s_blocksize - offset : toread;
> >
> > -               tmp_bh.b_state = 0;
> > +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
> >                err = ext2_get_block(inode, blk, &tmp_bh, 0);
> >                if (err < 0)
> >                        return err;
> > @@ -1366,7 +1366,7 @@ static ssize_t ext2_quota_write(struct
> > super_block *sb, int type,
> >                tocopy = sb->s_blocksize - offset < towrite ?
> >                                sb->s_blocksize - offset : towrite;
> >
> > -               tmp_bh.b_state = 0;
> > +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
> >                err = ext2_get_block(inode, blk, &tmp_bh, 1);
> >                if (err < 0)
> >                        goto out;
> > --
> > 1.5.4.3
> >
> >
> > Thanks -
> > Manish
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] : make sure the buffer head members are zeroed out before using them.
  2009-01-25 16:22     ` Manish Katiyar
  2009-01-25 17:01       ` Eric Sandeen
@ 2009-01-26 12:32       ` Jan Kara
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Kara @ 2009-01-26 12:32 UTC (permalink / raw)
  To: Manish Katiyar; +Cc: Eric Sandeen, ext4, Theodore Ts'o, cmm

> On Sun, Jan 25, 2009 at 9:45 PM, Eric Sandeen <sandeen@redhat.com> wrote:
> > Manish Katiyar wrote:
> >> On Tue, Jan 20, 2009 at 10:36 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
> >>> ext2_quota_read doesn't bzeroes tmp_bh before calling ext2_get_block()
> >>> where we access the b_size of it. Since it is a local variable it
> >>> might contain some garbage. Make sure it is filled with zero before
> >>> passing.
> >>
> >> Hi Ted/mingming,
> >>
> >> Any feedback on this ??
> >
> > This looks ok to me, Manish.  I'm curious, did you see this fail in real
> > life, and if so, what'd the failure look like?
> 
> Actually no......I realised this while going through the code. I was
> also wondering why we haven't hit this till now. Since ext{3,4} don't
> have this issue, the only reason I can think of is because ext2 with
> quota is not very much used or somehow we are lucky.
  Well, I think ext2 with quotas is not used very often. But also note
that if a random garbage is passed in it has high chances that maxblocks
is >= 1. And that is all what is needed for ext2_get_blocks() to return
what we want.

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] : make sure the buffer head members are zeroed out before using them.
  2009-01-26 12:29   ` Jan Kara
@ 2009-01-26 12:33     ` Manish Katiyar
  2009-01-26 12:48       ` Manish Katiyar
  0 siblings, 1 reply; 10+ messages in thread
From: Manish Katiyar @ 2009-01-26 12:33 UTC (permalink / raw)
  To: Jan Kara; +Cc: ext4, Theodore Ts'o, cmm

On Mon, Jan 26, 2009 at 5:59 PM, Jan Kara <jack@suse.cz> wrote:
>> On Tue, Jan 20, 2009 at 10:36 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
>> > ext2_quota_read doesn't bzeroes tmp_bh before calling ext2_get_block()
>> > where we access the b_size of it. Since it is a local variable it
>> > might contain some garbage. Make sure it is filled with zero before
>> > passing.
>>
>> Hi Ted/mingming,
>>
>> Any feedback on this ??
>  Ops, sorry. I wanted to reply but first I wanted to research more
> whether we can set b_size to 0 and then forgot about it. Looking into
> other code (e.g. in fs/mpage.c or fs/buffer.c) I think it would be
> better to set b_size to sb->s_blocksize and be done with that. Mapping
> code does not need anything else set to a deterministic value so using
> memset is a bit overkill.

Thanks Jan for your comments. Yes memset is an overkill. I did it just
because other users of ext2_get_block were doing the same way. Will
rework the patch with setting b_size as blocksize and send again.

Thanks -
Manish

>
>                                                                Honza
>
>> > Signed-off-by : Manish Katiyar <mkatiyar@gmail.com>
>> > ---
>> >  fs/ext2/super.c |    4 ++--
>> >  1 files changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/fs/ext2/super.c b/fs/ext2/super.c
>> > index da8bdea..d10aa44 100644
>> > --- a/fs/ext2/super.c
>> > +++ b/fs/ext2/super.c
>> > @@ -1327,7 +1327,7 @@ static ssize_t ext2_quota_read(struct
>> > super_block *sb, int type, char *data,
>> >                tocopy = sb->s_blocksize - offset < toread ?
>> >                                sb->s_blocksize - offset : toread;
>> >
>> > -               tmp_bh.b_state = 0;
>> > +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
>> >                err = ext2_get_block(inode, blk, &tmp_bh, 0);
>> >                if (err < 0)
>> >                        return err;
>> > @@ -1366,7 +1366,7 @@ static ssize_t ext2_quota_write(struct
>> > super_block *sb, int type,
>> >                tocopy = sb->s_blocksize - offset < towrite ?
>> >                                sb->s_blocksize - offset : towrite;
>> >
>> > -               tmp_bh.b_state = 0;
>> > +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
>> >                err = ext2_get_block(inode, blk, &tmp_bh, 1);
>> >                if (err < 0)
>> >                        goto out;
>> > --
>> > 1.5.4.3
>> >
>> >
>> > Thanks -
>> > Manish
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> Jan Kara <jack@suse.cz>
> SuSE CR Labs
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] : make sure the buffer head members are zeroed out before using them.
  2009-01-26 12:33     ` Manish Katiyar
@ 2009-01-26 12:48       ` Manish Katiyar
  2009-01-26 13:00         ` Jan Kara
  0 siblings, 1 reply; 10+ messages in thread
From: Manish Katiyar @ 2009-01-26 12:48 UTC (permalink / raw)
  To: Jan Kara; +Cc: ext4, Theodore Ts'o, cmm

On Mon, Jan 26, 2009 at 6:03 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
> On Mon, Jan 26, 2009 at 5:59 PM, Jan Kara <jack@suse.cz> wrote:
>>> On Tue, Jan 20, 2009 at 10:36 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
>>> > ext2_quota_read doesn't bzeroes tmp_bh before calling ext2_get_block()
>>> > where we access the b_size of it. Since it is a local variable it
>>> > might contain some garbage. Make sure it is filled with zero before
>>> > passing.
>>>
>>> Hi Ted/mingming,
>>>
>>> Any feedback on this ??
>>  Ops, sorry. I wanted to reply but first I wanted to research more
>> whether we can set b_size to 0 and then forgot about it. Looking into
>> other code (e.g. in fs/mpage.c or fs/buffer.c) I think it would be
>> better to set b_size to sb->s_blocksize and be done with that. Mapping
>> code does not need anything else set to a deterministic value so using
>> memset is a bit overkill.
>
> Thanks Jan for your comments. Yes memset is an overkill. I did it just
> because other users of ext2_get_block were doing the same way. Will
> rework the patch with setting b_size as blocksize and send again.

Here is the updated patch. compile tested.

Signed-off-by : Manish Katiyar <mkatiyar@gmail.com>
---
 fs/ext2/super.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index da8bdea..2a1c0c6 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1328,6 +1328,7 @@ static ssize_t ext2_quota_read(struct
super_block *sb, int type, char *data,
 				sb->s_blocksize - offset : toread;

 		tmp_bh.b_state = 0;
+		tmp_bh.b_size = sb->s_blocksize;
 		err = ext2_get_block(inode, blk, &tmp_bh, 0);
 		if (err < 0)
 			return err;
@@ -1367,6 +1368,7 @@ static ssize_t ext2_quota_write(struct
super_block *sb, int type,
 				sb->s_blocksize - offset : towrite;

 		tmp_bh.b_state = 0;
+		tmp_bh.b_size = sb->s_blocksize;
 		err = ext2_get_block(inode, blk, &tmp_bh, 1);
 		if (err < 0)
 			goto out;
-- 
1.5.4.3



Thanks -
Manish


>
> Thanks -
> Manish
>
>>
>>                                                                Honza
>>
>>> > Signed-off-by : Manish Katiyar <mkatiyar@gmail.com>
>>> > ---
>>> >  fs/ext2/super.c |    4 ++--
>>> >  1 files changed, 2 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/fs/ext2/super.c b/fs/ext2/super.c
>>> > index da8bdea..d10aa44 100644
>>> > --- a/fs/ext2/super.c
>>> > +++ b/fs/ext2/super.c
>>> > @@ -1327,7 +1327,7 @@ static ssize_t ext2_quota_read(struct
>>> > super_block *sb, int type, char *data,
>>> >                tocopy = sb->s_blocksize - offset < toread ?
>>> >                                sb->s_blocksize - offset : toread;
>>> >
>>> > -               tmp_bh.b_state = 0;
>>> > +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
>>> >                err = ext2_get_block(inode, blk, &tmp_bh, 0);
>>> >                if (err < 0)
>>> >                        return err;
>>> > @@ -1366,7 +1366,7 @@ static ssize_t ext2_quota_write(struct
>>> > super_block *sb, int type,
>>> >                tocopy = sb->s_blocksize - offset < towrite ?
>>> >                                sb->s_blocksize - offset : towrite;
>>> >
>>> > -               tmp_bh.b_state = 0;
>>> > +               memset(&tmp_bh, 0, sizeof(struct buffer_head));
>>> >                err = ext2_get_block(inode, blk, &tmp_bh, 1);
>>> >                if (err < 0)
>>> >                        goto out;
>>> > --
>>> > 1.5.4.3
>>> >
>>> >
>>> > Thanks -
>>> > Manish
>>> >
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> --
>> Jan Kara <jack@suse.cz>
>> SuSE CR Labs
>>
>

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] : make sure the buffer head members are zeroed out before using them.
  2009-01-26 12:48       ` Manish Katiyar
@ 2009-01-26 13:00         ` Jan Kara
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Kara @ 2009-01-26 13:00 UTC (permalink / raw)
  To: Manish Katiyar; +Cc: ext4, Theodore Ts'o, cmm

On Mon 26-01-09 18:18:19, Manish Katiyar wrote:
> On Mon, Jan 26, 2009 at 6:03 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
> > On Mon, Jan 26, 2009 at 5:59 PM, Jan Kara <jack@suse.cz> wrote:
> >>> On Tue, Jan 20, 2009 at 10:36 PM, Manish Katiyar <mkatiyar@gmail.com> wrote:
> >>> > ext2_quota_read doesn't bzeroes tmp_bh before calling ext2_get_block()
> >>> > where we access the b_size of it. Since it is a local variable it
> >>> > might contain some garbage. Make sure it is filled with zero before
> >>> > passing.
> >>>
> >>> Hi Ted/mingming,
> >>>
> >>> Any feedback on this ??
> >>  Ops, sorry. I wanted to reply but first I wanted to research more
> >> whether we can set b_size to 0 and then forgot about it. Looking into
> >> other code (e.g. in fs/mpage.c or fs/buffer.c) I think it would be
> >> better to set b_size to sb->s_blocksize and be done with that. Mapping
> >> code does not need anything else set to a deterministic value so using
> >> memset is a bit overkill.
> >
> > Thanks Jan for your comments. Yes memset is an overkill. I did it just
> > because other users of ext2_get_block were doing the same way. Will
> > rework the patch with setting b_size as blocksize and send again.
> 
> Here is the updated patch. compile tested.
> 
> Signed-off-by : Manish Katiyar <mkatiyar@gmail.com>
  Yes, now it looks fine. Thanks for the fix.

  Acked-by: Jan Kara <jack@suse.cz>

									Honza
> ---
>  fs/ext2/super.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index da8bdea..2a1c0c6 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -1328,6 +1328,7 @@ static ssize_t ext2_quota_read(struct
> super_block *sb, int type, char *data,
>  				sb->s_blocksize - offset : toread;
> 
>  		tmp_bh.b_state = 0;
> +		tmp_bh.b_size = sb->s_blocksize;
>  		err = ext2_get_block(inode, blk, &tmp_bh, 0);
>  		if (err < 0)
>  			return err;
> @@ -1367,6 +1368,7 @@ static ssize_t ext2_quota_write(struct
> super_block *sb, int type,
>  				sb->s_blocksize - offset : towrite;
> 
>  		tmp_bh.b_state = 0;
> +		tmp_bh.b_size = sb->s_blocksize;
>  		err = ext2_get_block(inode, blk, &tmp_bh, 1);
>  		if (err < 0)
>  			goto out;
> -- 
> 1.5.4.3
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-01-26 13:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-20 17:06 [PATCH] : make sure the buffer head members are zeroed out before using them Manish Katiyar
2009-01-25 15:53 ` Manish Katiyar
2009-01-25 16:15   ` Eric Sandeen
2009-01-25 16:22     ` Manish Katiyar
2009-01-25 17:01       ` Eric Sandeen
2009-01-26 12:32       ` Jan Kara
2009-01-26 12:29   ` Jan Kara
2009-01-26 12:33     ` Manish Katiyar
2009-01-26 12:48       ` Manish Katiyar
2009-01-26 13:00         ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox