29jm / SnowflakeOS

"It is very special"
https://jmnl.xyz
MIT License
316 stars 18 forks source link

Ext2 Driver - Incorrect parsing of block group descriptor #33

Closed thesilvanator closed 2 years ago

thesilvanator commented 2 years ago

On my machine mkfs.ext2 defaults to a 4096 byte block size. Cause of this I was getting a [ext2.c] unsupported inode type: 0 error that would then lead to a page fault stacktrace loop when trying to start the first process.

I found the issue in the group_descriptor_t* parse_group_descriptors(ext2_fs_t* fs) function. It just assumed the block_size was 1024 and therefore the block group descriptors at block 2. Changing the function from:

static group_descriptor_t* parse_group_descriptors(ext2_fs_t* fs) {
    uint8_t* block = kmalloc(fs->block_size);
    read_block(fs, 2, block);

    uint32_t size = fs->num_block_groups * sizeof(group_descriptor_t);
    group_descriptor_t* bgd = kmalloc(size);

    memcpy((void*) bgd, (void*) block, size);
    kfree(block);

    return bgd;
}

to

static group_descriptor_t* parse_group_descriptors(ext2_fs_t* fs) {
    uint8_t* block = kmalloc(fs->block_size);

    uint32_t sb_block = 1024/fs->block_size;
    uint32_t bgd_block = sb_block+1;

    read_block(fs, bgd_block, block);

    uint32_t size = fs->num_block_groups * sizeof(group_descriptor_t);
    group_descriptor_t* bgd = kmalloc(size);

    memcpy((void*) bgd, (void*) block, size);
    kfree(block);

    return bgd;
} 

fixed the problem.

29jm commented 2 years ago

Well spotted, thank you for the investigation. I will fix it soon, or if you want to submit a PR you're also welcome to do so.

As a note to myself, there's also a potential read out of bounds with the memcpy call given enough group descriptors, say 33 with a 1024 block size.

thesilvanator commented 2 years ago

Ok! Just sent in a pull request now.

I may take a look at the memcpy issue as well sometime in the near future.

29jm commented 2 years ago

Thanks for the PR! I've opened #35 for the memcpy issue.