caltry / Bikeshed

Bikeshed is an operating system being re-written in Node.js
http://www.freelists.org/list/bikeshed
4 stars 1 forks source link

ext2_raw_read() - Doesn't fill buffer properly #5

Closed wlitwin closed 12 years ago

wlitwin commented 12 years ago

Upon further investigation it appears something odd happens if you try to read more than 0x399 (921) bytes using ext2_raw_read(). If I try to read more than that at one time no data is put into the buffer, or splotches of data (don't seem to be relevant to what I want though).

For example:

    void* buffer = __kmalloc(5388);
    ext2_status = ext2_raw_read(bikeshed_ramdisk_context, file_name, buffer,
            &bytes_read, 0, 0x399);
    serial_printf("Buffer at: %x - reading: 5388 - read: %d\n", buffer, bytes_read);
    asm volatile("hlt");

(I narrowed this issue down, because it was failing to read the whole file in one go, which is what the 5388 byte header is for)

If I change the amount to read from 0x399 to 0x400 I fail to get the ELF header. Whatever is causing this is causing the issue with the ELF program sections failing to read, because those are over 0x400 bytes. Any idea why this may be happening?

Probably want some output...

---Elf: attempting to open: /etc/initproc
ELF header location: D0000620
read_ext2.c:667, dirname before excluding filename: /etc/initproc
read_ext2.c:671, dirname after excluding filename: /etc/
get_file_from_dir_path( /etc/, initproc )
read_ext2.c:245 context->sb: E0000400
Feching inode 2
inode_table: E0001400
read_ext2.c:511
get_file_from_dir_inode(etc)
read_ext2.c:536 trying file: . <2>, with length: 1  strncmp(): 55
dirent->entry_length: 12 correct length? 1
read_ext2.c:536 trying file: .. <2>, with length: 2  strncmp(): 55
dirent->entry_length: 12 correct length? 1
read_ext2.c:536 trying file: lost+found <11>, with length: 10  strncmp(): -7
dirent->entry_length: 20 correct length? 1
read_ext2.c:536 trying file: test.txt <12>, with length: 8  strncmp(): -15
dirent->entry_length: 16 correct length? 1
read_ext2.c:536 trying file: etc <13>, with length: 3  strncmp(): 0
dirent->entry_length: 12 correct length? 1
fetching new dir inode: 13
read_ext2.c:245 context->sb: E0000400
Feching inode 13
inode_table: E0001400
read_ext2.c:245 context->sb: E0000400
Feching inode 13
inode_table: E0001400
read_ext2.c:511
get_file_from_dir_inode(initproc)
read_ext2.c:536 trying file: . <13>, with length: 1  strncmp(): 59
dirent->entry_length: 12 correct length? 1
read_ext2.c:536 trying file: .. <2>, with length: 2  strncmp(): 59
dirent->entry_length: 12 correct length? 1
read_ext2.c:536 trying file: welcome <14>, with length: 7  strncmp(): -14
dirent->entry_length: 16 correct length? 1
read_ext2.c:536 trying file: initproc <15>, with length: 8  strncmp(): 0
dirent->entry_length: 16 correct length? 1
read_ext2.c:245 context->sb: E0000400
Feching inode 15
inode_table: E0001400
going to read 52 bytes
read_ext2.c:667, dirname before excluding filename: /etc/initproc
read_ext2.c:671, dirname after excluding filename: /etc/
get_file_from_dir_path( /etc/, initproc )
read_ext2.c:245 context->sb: E0000400
Feching inode 2
inode_table: E0001400
read_ext2.c:511
get_file_from_dir_inode(etc)
read_ext2.c:536 trying file: . <2>, with length: 1  strncmp(): 55
dirent->entry_length: 12 correct length? 1
read_ext2.c:536 trying file: .. <2>, with length: 2  strncmp(): 55
dirent->entry_length: 12 correct length? 1
read_ext2.c:536 trying file: lost+found <11>, with length: 10  strncmp(): -7
dirent->entry_length: 20 correct length? 1
read_ext2.c:536 trying file: test.txt <12>, with length: 8  strncmp(): -15
dirent->entry_length: 16 correct length? 1
read_ext2.c:536 trying file: etc <13>, with length: 3  strncmp(): 0
dirent->entry_length: 12 correct length? 1
fetching new dir inode: 13
read_ext2.c:245 context->sb: E0000400
Feching inode 13
inode_table: E0001400
read_ext2.c:245 context->sb: E0000400
Feching inode 13
inode_table: E0001400
read_ext2.c:511
get_file_from_dir_inode(initproc)
read_ext2.c:536 trying file: . <13>, with length: 1  strncmp(): 59
dirent->entry_length: 12 correct length? 1
read_ext2.c:536 trying file: .. <2>, with length: 2  strncmp(): 59
dirent->entry_length: 12 correct length? 1
read_ext2.c:536 trying file: welcome <14>, with length: 7  strncmp(): -14
dirent->entry_length: 16 correct length? 1
read_ext2.c:536 trying file: initproc <15>, with length: 8  strncmp(): 0
dirent->entry_length: 16 correct length? 1
read_ext2.c:245 context->sb: E0000400
Feching inode 15
inode_table: E0001400
going to read 921 bytes
Buffer at: D0000658 - reading: 5388 - read: 921

Ignore the

Buffer at: D0000658 - reading: 5388 - read: 921

It 'read' the correct amount, but no data was placed into the buffer

caltry commented 12 years ago

On 5/20/12 8:21 PM, Tietokone wrote:

Upon further investigation it appears something odd happens if you try to read more than 0x399 (921) bytes using ext2_raw_read(). If I try to read more than that at one time no data is put into the buffer [...] If I change the amount to read from 0x399 to 0x400 I fail to get the ELF header. Whatever is causing this is causing the issue with the ELF program sections failing to read, because those are over 0x400 bytes. Any idea why this may be happening?

0x400 is the block size on the filesystem.

Are you able to read in smaller chunks to load the ELF file as a workaround while I hunt down the regression? It sounds like this was introduced within the past few days.

wlitwin commented 12 years ago

Yeah, that won't be a problem, just wanted to let you know

caltry commented 12 years ago

Ok, if you're not blocking on the issue, I'd like to put it off until 2:30 tomorrow, when my first final ends.

wlitwin commented 12 years ago

Sure thing, I'm implementing the work around now

caltry commented 12 years ago

Just found the error:

487 char local_block_buffer[block_size+1-block_offset];
488 _kmemcpy( local_block_buffer, data, block_size-block_offset );
489 buffer_I_care_about += block_size;

So, I read the data into a stack allocated buffer, then throw it away. headdesk

caltry commented 12 years ago

There's a fix for this that will be ready soon (read: by 2pm Tuesday); this issue was slightly more complicated than what I indicated in my last comment.

I have a fix working, but I'm not done writing tests. I was working on this at the same time as indirect block support, so I may not be able to decouple the two commits.

caltry commented 12 years ago

c75e37c0a25b15754cd881eea07d2ae6b37b202c should fix the issue. I'll merge it in to master tomorrow afternoon when I'm not tired.

caltry commented 12 years ago

The fix is now in 'master'.

caltry commented 12 years ago

The fix seems to be working in c164e4d0395c245d4f714d28f6b54bfbb1d39452.