Advice needed (also warning to others!)

[Slasher]

Hi people,

I've made my first server system interface for writing drivers in my os.
One of the system calls I have is
waitServerRequest(server_id,request_number,void *buffer,unsigned long *length)

server_id is the handle for the server that you made the request from
request_number is the handle for the request. you can have multipe requests at anytime
buffer is where the data is going to be copied to(your buffer)
length is where the system tell you how much data was copied

It works fine. My question is this -
How can I check that some buffer is not pointing to an invalid address?
Am asking cause I did this in my test code

Code: Select all

unsigned char *track_buffer;
unsigned long length;
.....
....

waitServerRequest(floppy_id,request_number,&track_buffer,&length);

as you can see I used &track_buffer (which makes it a pointer to a pointer) which was sending the address of the track_buffer pointer instead of the address in track_buffer.

This was crashing my OS!!

How can I test for this and prevent this from happening?

Thanks

[Solar]

Create a function createBuffer(). That function does the malloc() internally, writes some magic number to the first 4 bytes of the memory area, and returns a pointer beyond those 4 bytes as the buffer.

The waitServerRequest() function then checks whether buffer - 4 actually contains the magic number, and calls bloody murder if not.

Then you need a function releaseBuffer() that calls free() with the correct address (buffer - 4).

It's not 100% save, but it can help.

[paulbarker]

Validity checks (non-NULL pointer and pointer is in user mode area) and access checks (the memory pointed to exists) are the best place to start if you have no checks, but they will not catch this problem. Solar's idea is a good choice if you want low overhead. A better solution using more overhead is to add a couple of system calls:

buffer_create - Creates a buffer and maps it into your app, and puts a pointer to it in a kernel table.

buffer_destroy - Destroy a buffer, checking against the kernel table that it is actually a buffer then removing the entry from the table.

The kernel could then check that a given pointer is in the relevent kernel table, and if it isnt then the buffer has not been allocated for kernel use.

The overhead here is 2 extra syscalls per buffer (create and destroy), the validation step on each syscall involving a buffer (very low if using a hash table), and the space for the kernel table itself (with 1 table per process this could be an issue).

Alternative methods like passing around a buffer id rather than a pointer should also work.

EDIT: I'm not saying this way is correct, it's just one possible way. Experiment a little, and have a good think about how much performance you are willing to sacrifice for this feature. If it's not worth it, don't use it!

[Slasher]

Thanks a lot for the suggestions. It has gotten me thinking. It's good to get a fresh view on a problem!

I'll implement something and see how it goes. Keep you all posted.

Thanks