Static overrun in reiser3

Static overrun in reiser3

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Hi Hans -

I've been playing around with the Coverity code checker, and while I
think it still sees a few too many false positives, it's a good tool.

Anyway, one of the potential bugs it came up with in reiserfs was this one:

struct tree_balance contains a number of arrays of size MAX_HEIGHT (5).
In fix_nodes(), line 2502, we see:
                        p_s_tb->insert_size[n_h + 1] =
                            (DC_SIZE + KEY_SIZE) * (p_s_tb->blknum[n_h]
- - 1);

I haven't run a thorough analysis, but is it possible for n_h to be 4
there, and then n_h + 1 would be 5, overrunning into the next field of
struct tree_balance? The tool seems to think so, but it also thought
that not checking that dentry->d_inode != NULL after calling
inode->i_op->mkdir was invalid, even though a successful return value
implies that dentry->d_inode != NULL.

- -Jeff

- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEGIkGLPWxlyuTD7IRAno5AJ92Qql/sMnii2Kk2VdFlLs/Hbpc3ACffcjT
qsw0pCCjm2DfeMA67n5sLu4=
=1bzF
-----END PGP SIGNATURE-----

Re: Static overrun in reiser3

[Hans Reiser]

Jeff Mahoney wrote:

>
> Hi Hans -
>
> I've been playing around with the Coverity code checker, and while I
> think it still sees a few too many false positives, it's a good tool.

Thanks for doing that work!  If you could do it for V4, that would be
great too.  If not, maybe Edward could do it.

>
> Anyway, one of the potential bugs it came up with in reiserfs was this
> one:
>
> struct tree_balance contains a number of arrays of size MAX_HEIGHT (5).
> In fix_nodes(), line 2502, we see:
>                         p_s_tb->insert_size[n_h + 1] =
>                             (DC_SIZE + KEY_SIZE) * (p_s_tb->blknum[n_h]
> - 1);
>
> I haven't run a thorough analysis, but is it possible for n_h to be 4
> there, and then n_h + 1 would be 5, overrunning into the next field of
> struct tree_balance? The tool seems to think so, but it also thought
> that not checking that dentry->d_inode != NULL after calling
> inode->i_op->mkdir was invalid, even though a successful return value
> implies that dentry->d_inode != NULL.

I'll let vs answer this.

>
> -Jeff
>
> --
> Jeff Mahoney
> SUSE Labs

Re: Static overrun in reiser3

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hans Reiser wrote:
> Jeff Mahoney wrote:
> 
>> Hi Hans -
>>
>> I've been playing around with the Coverity code checker, and while I
>> think it still sees a few too many false positives, it's a good tool.
> 
> Thanks for doing that work!  If you could do it for V4, that would be
> great too.  If not, maybe Edward could do it.

Ah, sorry, all I can do is review their database. I can't actually run
the checker myself.

- -Jeff

- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEGI2dLPWxlyuTD7IRApBbAJ9UYkwroLoeRtRJhXQKAuezoYoo+gCePed1
P9mZqgnf5FcU9FvrYfq3rmM=
=dE9f
-----END PGP SIGNATURE-----

Re: Static overrun in reiser3

[Hans Reiser]

Jeff Mahoney wrote:

> Hans Reiser wrote:
>
> >Jeff Mahoney wrote:
>
> >>Hi Hans -
> >>
> >>I've been playing around with the Coverity code checker, and while I
> >>think it still sees a few too many false positives, it's a good tool.
>
> >Thanks for doing that work!  If you could do it for V4, that would be
> >great too.  If not, maybe Edward could do it.
>
>
> Ah, sorry, all I can do is review their database. I can't actually run
> the checker myself.

Ah, so there is a database somewhere that we can look at?

>
> -Jeff
>
> --
> Jeff Mahoney
> SUSE Labs

Re: Static overrun in reiser3

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hans Reiser wrote:
> Jeff Mahoney wrote:
> 
>> Hans Reiser wrote:
>>
>>> Jeff Mahoney wrote:
>>>> Hi Hans -
>>>>
>>>> I've been playing around with the Coverity code checker, and while I
>>>> think it still sees a few too many false positives, it's a good tool.
>>> Thanks for doing that work!  If you could do it for V4, that would be
>>> great too.  If not, maybe Edward could do it.
>>
>> Ah, sorry, all I can do is review their database. I can't actually run
>> the checker myself.
> 
> Ah, so there is a database somewhere that we can look at?

Yes, you have to register at scan.coverity.com. There was a medium-sized
thread on LKML about it around two weeks ago.

- -Jeff

- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEGJA6LPWxlyuTD7IRAvcpAKCUvdOSMQYP9WD3IF8Qm3i2mxomaACgm0PL
VnxXK5P6mKc37FDNCl0tvCs=
=wCll
-----END PGP SIGNATURE-----

Re: Static overrun in reiser3

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

On Wed, 15 Mar 2006 14:01:22 PST, Hans Reiser said:
> Jeff Mahoney wrote:

> > Ah, sorry, all I can do is review their database. I can't actually run
> > the checker myself.
> 
> Ah, so there is a database somewhere that we can look at?

You may have missed out - I think the Coverty guys only did the in-Linus-tree
stuff, and I'm pretty sure they didn't cover the -mm branch.  But I could be wrong...

[-- Attachment #2: Type: application/pgp-signature, Size: 228 bytes --]