Opened 10 years ago

Closed 10 years ago

#4224 closed enhancement (invalid)

patch: private areas, mlock()/munlock() and MAP_LOCKED for mmap()

Reported by: Blub Owned by: axeld
Priority: normal Milestone: R1
Component: System/Kernel Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Recently I found myself trying to create a private region of memory which is locked into RAM (so it is not swapped), yet not accessible to others, for storing the input of a password field in it. I've come across several problems there: Areas have a B_FULL_LOCK flag, however, it is not possible to prevent them from being cloned. mmap() allocates areas named "mmap area" which can just as well be cloned, and there's no implementation of mlock() (and munlock())

I have implemented mlock() and munlock(), and also added the possibility to use MAP_LOCKED with mmap(). Also I have added a B_PRIVATE_AREA flag which may be used by the user to prevent people from cloning the area, which is also used by mmap() when using the MAP_PRIVATE flag.

With this, it is possible for applications to allocate private, protected and locked memory regions where they can store sensitive data when needed.

Looking forward to feedback of any kind :) (about the implementation as well as coding style and anything else which would be useful for me to know to be able to provide patches of a good quality if my skills allow it :) )

Attachments (1)

haiku_mlock_map_locked_and_private_area.diff (6.1 KB) - added by Blub 10 years ago.
implements: B_PRIVATE_AREA, mlock(), munlock(), MAP_LOCKED to mmap() and using B_PRIVATE_AREA when using MAP_PRIVATE in mmap()

Download all attachments as: .zip

Change History (6)

Changed 10 years ago by Blub

implements: B_PRIVATE_AREA, mlock(), munlock(), MAP_LOCKED to mmap() and using B_PRIVATE_AREA when using MAP_PRIVATE in mmap()

comment:1 Changed 10 years ago by bonefish

You have misunderstood the meaning of MAP_PRIVATE. It does only mean that when you map a file in this mode, changes you make to the mapped memory are not written to the file. It does not mean no-one else can read the area. Another process of the same (or a more privileged) user will always have a way to read the memory (and be it via the debugger interface), even when we fully support multi-user.

Regarding the m[un]lock() patch, the implementation is unfortunately unsafe. lock_memory()/unlock_memory() must strictly be balanced, i.e. you can't just make them available to userland without additional bookkeeping. One idea was to use the spare bit of the per-page protection nibble. If we also want to support posix_madvise(), we will probably have to allocate a complete byte per page. At any rate neither posix_madvise() nor mlock() are particularly important features. Both only are performance-relevant.

comment:2 Changed 10 years ago by Blub

What about the B_PRIVATE_AREA flag as a flag to create_area() though? It would at keep lower-priviledged users from reading the data, and higher-priviledged users would be forced to user the debugger interface. I could provide a patch with only that implemented. Personally I think it's important to be able to allocate "more private" areas. Of course, a high-priviledged user will always have their way of reading the memory, but at least it would require means which you could at least consider "abuse" when used solely to get access to information which they're not supposed to have.

comment:3 Changed 10 years ago by axeld

Also, you can only clone all areas right now, as you are root. Once we have proper multi user support, this won't work anymore that easily. And it's always better to be secure by default, and not having to ask for it specifically.

comment:4 Changed 10 years ago by Blub

Makes sense to me, okay then, thanks for the replies. So I guess this ticket can be closed then?

After reading the mlock manpage again I can see the problem now. Particularly the part about munlock() not affecting any locks established by other processes. So one spare-bit wouldn't be enough for this.

I guess the multiuser support will be worked on more extensively after the upcoming release? I'm already looking forward to it :)

comment:5 in reply to:  4 Changed 10 years ago by bonefish

Resolution: invalid
Status: newclosed

Replying to Blub:

After reading the mlock manpage again I can see the problem now. Particularly the part about munlock() not affecting any locks established by other processes. So one spare-bit wouldn't be enough for this.

The locks of other processes shall not be affected. The bits are associated with the area though, which belongs to the process calling m[un]lock(). I.e. one effectively has on bit per process and page. And there's already a per-page counter indicating whether the page is locked.

I guess the multiuser support will be worked on more extensively after the upcoming release? I'm already looking forward to it :)

Well, at some point after R1 multi-user will be attacked. It's quite a big chunk of work, though.

Note: See TracTickets for help on using tickets.