Secretion 0.3

[Stevo14]

After lots of work over the last ~2 months I'm proud to present version 0.3 of my OS.

New features since version 0.2.5 include:
+ATA driver
+Floppy disk driver
+A Virtual File System
+FAT12 read support
+FAT32 read support
+Ability to boot from floppy or disk without recompiling. (basically it detects the boot disk from grub and auto mounts it.)

Download:
http://code.google.com/p/secretion/
If anyone wants the 32mb disk image to test the ATA driver and the FAT32 code please say so.

Notes and known issues:
+ Some people didn't like the name last time. I'm still undecided. If you have any suggestions feel free to give them.
+ Floppy access is DEATHLY SLOW. If anyone can recommend a way to speed it up it would be much appreciated. As long as the clock at the bottom continues to tick you know that the system has not frozen.
+ The window code is fragile. I plan on re-writing it for the next version but until then, expect bugs related to the windows.
+ Please test the file system throughly. Feed it the nastiest path name that you can think of. (something like "cd /dev/../config/kernel/../../bin/./../boot/grub/../../bin/kernel/./../kernel/modules" and see if you actually end up in the folder called "modules".)
+ The beep function seems only to work if you have a proper PC speaker. (I think the problem is related to the frequency of the sound being too low... not sure exactly though.)
+ Feel free to open an issue on the googlecode page so that I can keep track of all of the bugs. (Hopefully there aren't that many...)

[Zenith]

Pros:

• - File System support seems good - haven't manage to confuse the file browser (yet...)
  - Basic console/keyboard support
  - Recognized floppy as root, mounted it as that (complete with /dev folder )
  - Sound, module loading
  - Memory detection (from GRUB) and keeping track of used memory
  - All the other features not visible to the naked eye

Needs Improvement:

• - Minor console issues: Once the prompt gets to the bottom of the screen, text doesn't show, extra blue borders are printed, etc...
  - Some keyboard stuff - pressing Caps Lock breaks backspace, and arrow keys and delete key don't work
  - A little more functionality?

I'm also wondering, is the console in kernel mode or is it a user task? It seems like it's in kernel mode, but it would be impressive if it was its own process...

Overall, it looks really nice. Good job, and keep OSDeving!

[Stevo14]

Needs Improvement:

• - Minor console issues: Once the prompt gets to the bottom of the screen, text doesn't show, extra blue borders are printed, etc...

Yea. Those are bugs related to the windowing code. A re-write is on the to-do list.

• - Some keyboard stuff - pressing Caps Lock breaks backspace, and arrow keys and delete key don't work

As far as backspacing goes, it works (meaning the value is erased from the window's "stdin" and the cursor is moved back) but I can't seem to get the stupid character to actually disappear from the screen. The others have been put on my to-do list.

• - A little more functionality?

What exactly do you mean by this? If you mean "it needs to be more useful" then I totally agree with you. The "usefulness to code" ratio is a bit low right now, but it is still early in development and I imagine that a solid file system will be very useful as I continue.

I'm also wondering, is the console in kernel mode or is it a user task? It seems like it's in kernel mode, but it would be impressive if it was its own process...

Alas, it is, indeed, in kernel mode. User mode is actually quite a ways down the list. As it stands, I plan on having a full system call interface, ELF loading and multitasking before I dive into user mode.
(although this is likely to change.)

Overall, it looks really nice. Good job, and keep OSDeving!

Many thanks to you for taking the time to test it.

[blound]

ok I was looking at:
http://code.google.com/p/secretion/sour ... el_shell.c

and saw:

Code: Select all

void file_browser()
{
        window *fbwin = (window*)kmalloc(sizeof(window));
        fbwin->name = "File Browser";
        fbwin->x = 27;
        fbwin->y = 3;
        fbwin->width = 50;
        fbwin->height = 43;
        fbwin->textwidth = fbwin->width - 2;
        fbwin->textheight = fbwin->height - 2;
        fbwin->csr_x = 0;
        fbwin->csr_y = 0;
        fbwin->dirty = 0;
        fbwin->scroll = 0;
        int i = 0;
        for(i = 0; i < 12288; i++)
        {
                fbwin->stdout[i] = ' ';
        }

first try compiling with strict prototypes, also this function is HUGE ( over 200 lines ) it looks alot better to break code into smaller peices. I also dont like hardcoded number suchs as "12288" in the loop ( which could just be a memset instead of a loop )

also in your code you have a strcpy with 3 args which from the looks of it should be regular strncpy. you could also add a kmalloc wrapper that zeros memory allocated or zero out entire structs after malloc so you don't have to set numerous fields to '0' after each malloc call. you also declare code in the middle of code which as 1010101010 pointed out in a recent post is not standard acceptable and is also annoying to read.

you also have some most likely security problems ( integer over/underflows in your fat driver since you trust all data pulled from the filesystem and use it for math/allocations. your code here:

Code: Select all

int install_syscall(int syscall_number, void (*handler)(syscall_regs_t *r))
{
        //syscall too high
        if(syscall_number > SYSCALL_MAX)
                return 0;

would also fail the 'too high' check if 'syscall_number' was negative since the variable is signed. your 'syscall_handler' seems to explicilty trust the edx register which would let a user call any address in the address space by placing a special edx value.

in your heap allocation code you have:

Code: Select all

if((addr + size) >= heap->end)
                                        {
                                                write_string(" -- Heap Expansion Needed! -- ");
                                                halt();
                                        }

if someone was to send a large 'size' they could make addr + size wrap past zero and then allocate a chunk with a huge 'size'

I am not a computer where i can test the system, I just wanted to point out some coding style and security things I saw.

[Stevo14]

... code ...
first try compiling with strict prototypes,

Er... this may be a stupid question but, what are strict prototypes? I tried searching Google and all I came up with was the gcc -Wstrict-prototypes flag.

also this function is HUGE ( over 200 lines ) it looks alot better to break code into smaller peices. I also dont like hardcoded number suchs as "12288" in the loop ( which could just be a memset instead of a loop )

"kernel_shell.c" is not a permanent home for the shell. When I get program loading working each function in that file will be it's own program and "kernel_shell.c" will cease to exist. At that point each program will be broken into seperate functions like normal.

also in your code you have a strcpy with 3 args which from the looks of it should be regular strncpy.

...your right. I missed the 'n' in the middle when I created the function...

you could also add a kmalloc wrapper that zeros memory allocated or zero out entire structs after malloc so you don't have to set numerous fields to '0' after each malloc call. you also declare code in the middle of code which as 1010101010 pointed out in a recent post is not standard acceptable and is also annoying to read.

The zeroed memory thing gave me trouble at one point. I'll probably go with just zeroing memory inside kmalloc for simplicity. I also tried to compile my kernel with -std=c99 like 69 (01000101) suggested but it complained about inline assembly which would be a great pain to remove from the c source files.

you also have some most likely security problems ( integer over/underflows in your fat driver since you trust all data pulled from the filesystem and use it for math/allocations. your code here:
... code ...

would also fail the 'too high' check if 'syscall_number' was negative since the variable is signed. your 'syscall_handler' seems to explicilty trust the edx register which would let a user call any address in the address space by placing a special edx value.

in your heap allocation code you have:
... code ...

if someone was to send a large 'size' they could make addr + size wrap past zero and then allocate a chunk with a huge 'size'

I am not a computer where i can test the system, I just wanted to point out some coding style and security things I saw.

Thanks for the security stuff. I'll fix everything that you pointed out but I need to get better about coding in a security conscious way.

[blound]

http://www.amazon.com/Art-Software-Secu ... 0321444426

I would recommend reading the chapters in this book pertaining to C/C++/asm

[Adek336]

Wow, reading from floppy is slow even in virtualbox
You have the files at your site be like "floppy-0.3.img", it'd do with names such as "secretion-floppy-0.3.img" so after some time after downloading it (and not changing the filename) I'd still know it's Secretion:D

In virtualbox I see the cursor moving all around the screen when the console is scrolling - do you redraw the windows with a lot of putch()s? You might use memmove()s or memcpy()s (what is the difference anyway?) to move it on a line-per-line basis.

I like it that you have a TUI Not another plain olde console.

When writing text in a bottom line of a window it's black and I can't see it, you probably have that video memory initialised to 0's which means the attribute fields are 0 as well (which happens to be black on black)

[itisiuk]

hi.

very nice works almost perfactley on qemu

just one problem really.
after a while the text prompt disapears.

also reading from the floppy is very slow however least there is no errors
apart from that alls good.

[Stevo14]

Thanks for testing, even almost a month after it was released.

You have the files at your site be like "floppy-0.3.img", it'd do with names such as "secretion-floppy-0.3.img" so after some time after downloading it (and not changing the filename) I'd still know it's Secretion:D

I've done this so I can change the name with less hassle if I want to. Once I become 100% happy with a name I'll do as you suggested.

In virtualbox I see the cursor moving all around the screen when the console is scrolling - do you redraw the windows with a lot of putch()s? You might use memmove()s or memcpy()s (what is the difference anyway?) to move it on a line-per-line basis.

Ok, I admit it... I use putchar()'s for my window drawing functions... Although, on faster processors (real machines) I've found that it's barely noticeable.

When writing text in a bottom line of a window it's black and I can't see it, you probably have that video memory initialised to 0's which means the attribute fields are 0 as well (which happens to be black on black)

This is a known issue. I will, however, take note of what you suggested being that I haven't thought of that possibility yet.

[Love4Boobies]

I got an exception while trying to peek into a file...

[AJ]

You might use memmove()s or memcpy()s (what is the difference anyway?)

memmove() guarantees safety for overlapping memory areas, memcpy() does not.

Cheers,
Adam

[Stevo14]

I got an exception while trying to peek into a file...

Well, there have been many changes since version 0.3 (nearly 2 months of changes). I vaguely remember fixing something like this a while back. I'll look for it again when I get the shell code fully wired back up.