[openbeosstorage] Re: [Open-beos-cvs]CVS:current/src/kernel/core/disk_device_managerddm_userland_interface.cpp,1.17,1.18

  • From: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
  • To: openbeosstorage@xxxxxxxxxxxxx
  • Date: Sat, 27 Sep 2003 16:11:02 +0200 CEST

Ingo Weinhold <bonefish@xxxxxxxxxxxxxxx> wrote:
> I'm afraid, the cookie handling is not sufficient. At any rate 
> userland memory access should only happen through user_memcpy(). 
> E.g. in user_get_next_port_info():
> 
>       if ((addr)ucookie >= KERNEL_BASE && (addr)ucookie <= KERNEL_TOP)
>               return ERR_VM_BAD_USER_MEMORY;

We even have a handy macro for this:
if (!CHECK_USER_ADDRESS(userCookie))
        return B_BAD_ADDRESS;

Please don't use the ERR_xxx style error codes anymore, they will be 
phased out until R1.

> The latter does little, but provides a bit more type safety, which 
> makes bugs 
> like the one I just fixed in user_get_next_port_info() (info was 
> copied into ucookie) 
> less likely.

Oh, great :-)

> static_cast should be sufficient, BTW. No need for the sledgehammer. 
> ;-)

Indeed.

> However, we should be more careful here. Since bufferSize may 
> have a huge value, we shouldn't malloc() kernel memory unchecked. 
> So, either we set a reasonably great size constraint (100KB or so 
> maybe) or do a dry run with the UserDataWriter to get the exactly 
> required size. I'd prefer the latter solution.

Good thinking, and me too.

> relocated when copied back into the userland buffer.
> There are at least two solutions:
> 
> 1) IIRC, Axel once mentioned, that one could lock userland memory 
> and wouldn't need to do the copying into kernel memory and 
> back, then. I don't remember what the status of this feature was. 
> Unimplemented?

In the current VM, this is not implemented, right. But even BeOS does 
this, so we could do this, too (right now, there is no swap file, so 
nothing is paged out anyway). But of course, those pointers would only 
be valid in the current thread context, other kernel threads might not 
be able to access it.

> 2) Relocate the pointers. It shouldn't be too hard to add the 
> required 
> relocation functionality to UserDataWriter and make the concerned 
> classes use it.

That might be a good idea. Do we need to have pointers there, anyway?

> Given that the amount of memory to be copied is not that big (I guess 
> usually less than 500 bytes per partition) solution 2) should be 
> fine. 
> Since I need the test working, I'll fix that as soon as I managed to 
> track
> the compilation error down.

Sounds good enough to me, yes :)

Bye,
   Axel.

Other related posts:

  1. Home
  2. openbeosstorage
  3. Archive
  4. 09-2003