2004-04-16 21:52:45

by Dave Jones

[permalink] [raw]
Subject: NTFS null dereference x2

if vol is NULL, everything falls apart..

Dave

--- linux-2.6.5/fs/ntfs/attrib.c~ 2004-04-16 22:45:53.000000000 +0100
+++ linux-2.6.5/fs/ntfs/attrib.c 2004-04-16 22:46:47.000000000 +0100
@@ -1235,16 +1235,19 @@
u8 *al_end = al + initialized_size;
run_list_element *rl;
struct buffer_head *bh;
- struct super_block *sb = vol->sb;
+ struct super_block *sb;
unsigned long block_size = sb->s_blocksize;
unsigned long block, max_block;
int err = 0;
- unsigned char block_size_bits = sb->s_blocksize_bits;
+ unsigned char block_size_bits;

ntfs_debug("Entering.");
if (!vol || !run_list || !al || size <= 0 || initialized_size < 0 ||
initialized_size > size)
return -EINVAL;
+ sb = vol->sb;
+ block_size_bits = sb->s_blocksize_bits;
+
if (!initialized_size) {
memset(al, 0, size);
return 0;


2004-04-17 13:47:13

by Szabolcs Szakacsits

[permalink] [raw]
Subject: Re: NTFS null dereference x2


Dave Jones <[email protected]> wrote:

> if vol is NULL, everything falls apart..

AFAIS, neither vol nor vol->sb can be NULL below. The !vol check, that
fooled you or an automatic checker, is bogus and probably it slipped
through the user space library, thanks.

Please note, by the patch you would introduce a real bug when you
dereference the now uninitialized sb to assign a value to block_size.

Szaka

> --- linux-2.6.5/fs/ntfs/attrib.c~ 2004-04-16 22:45:53.000000000 +0100
> +++ linux-2.6.5/fs/ntfs/attrib.c 2004-04-16 22:46:47.000000000 +0100
> @@ -1235,16 +1235,19 @@
> u8 *al_end = al + initialized_size;
> run_list_element *rl;
> struct buffer_head *bh;
> - struct super_block *sb = vol->sb;
> + struct super_block *sb;
> unsigned long block_size = sb->s_blocksize;
> unsigned long block, max_block;
> int err = 0;
> - unsigned char block_size_bits = sb->s_blocksize_bits;
> + unsigned char block_size_bits;
>
> ntfs_debug("Entering.");
> if (!vol || !run_list || !al || size <= 0 || initialized_size < 0 ||
> initialized_size > size)
> return -EINVAL;
> + sb = vol->sb;
> + block_size_bits = sb->s_blocksize_bits;
> +
> if (!initialized_size) {
> memset(al, 0, size);
> return 0;

2004-04-23 11:04:44

by Anton Altaparmakov

[permalink] [raw]
Subject: Re: NTFS null dereference x2

On Sat, 17 Apr 2004, Szakacsits Szabolcs wrote:
> Dave Jones <[email protected]> wrote:
> > if vol is NULL, everything falls apart..
>
> AFAIS, neither vol nor vol->sb can be NULL below. The !vol check, that
> fooled you or an automatic checker, is bogus and probably it slipped
> through the user space library, thanks.
>
> Please note, by the patch you would introduce a real bug when you
> dereference the now uninitialized sb to assign a value to block_size.

Thanks. While at present vol nor vol->sb can be NULL, I have changed the
code in my tree to do the assignments after the checks for completeness
sake. I of course also moved the assignment that the patch ignored as
well, otherwise as Szaka said, it would all go horribly wrong...

Best regards,

Anton

> > --- linux-2.6.5/fs/ntfs/attrib.c~ 2004-04-16 22:45:53.000000000 +0100
> > +++ linux-2.6.5/fs/ntfs/attrib.c 2004-04-16 22:46:47.000000000 +0100
> > @@ -1235,16 +1235,19 @@
> > u8 *al_end = al + initialized_size;
> > run_list_element *rl;
> > struct buffer_head *bh;
> > - struct super_block *sb = vol->sb;
> > + struct super_block *sb;
> > unsigned long block_size = sb->s_blocksize;
> > unsigned long block, max_block;
> > int err = 0;
> > - unsigned char block_size_bits = sb->s_blocksize_bits;
> > + unsigned char block_size_bits;
> >
> > ntfs_debug("Entering.");
> > if (!vol || !run_list || !al || size <= 0 || initialized_size < 0 ||
> > initialized_size > size)
> > return -EINVAL;
> > + sb = vol->sb;
> > + block_size_bits = sb->s_blocksize_bits;
> > +
> > if (!initialized_size) {
> > memset(al, 0, size);
> > return 0;

--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/