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

  • From: Tyler Dauwalder <tyler@xxxxxxxxxxxxx>
  • To: openbeosstorage@xxxxxxxxxxxxx
  • Date: Sat, 27 Sep 2003 13:10:08 -0700

> 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():
>
[snip]
>
> This can probably be simplified a bit by introducing template functions for 
> dealing with reference variables copied from
> userland. They may look like this:
>
[snip]
>
> 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.

Okay, I'll adjust accordingly.

> [...]
> > @@ -538,11 +554,20 @@
> >  // _kern_get_disk_device_data
> >  status_t
> >  _kern_get_disk_device_data(partition_id id, bool deviceOnly, bool shadow,
> > -                           user_disk_device_data *buffer, size_t
> > bufferSize,
> > +                           user_disk_device_data *_buffer, size_t
> > bufferSize,
> >                             size_t *neededSize)
> >  {
> > -    if (!buffer && bufferSize > 0)
> > +    if (!_buffer && bufferSize > 0)
> >          return B_BAD_VALUE;
> > +
> > +    // copy in
> > +    user_disk_device_data *buffer = bufferSize > 0
> > +                                    ?
> > reinterpret_cast<user_disk_device_data*>(malloc(bufferSize))
> 
> static_cast should be sufficient, BTW. No need for the sledgehammer. ;-)

I'm not sure I see how reinterpret_cast<> is a sledgehammer.

> 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.

Okay, I go for the latter.

[snip]
> Damn, I didn't think of that -- I'm afraid this doesn't work 
> The problem is, that user_disk_device_data/user_partition_data make use of 
> pointers into the same buffer, that would need to be
> 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?
> 
> 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.
> 
> 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.

Okay.

-Tyler

Other related posts:

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