* [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 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-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-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