app_server: crash when running application from another user

[X512]

This is hrev54723.

Steps to reproduce.

1. Run useradd test_user.
2. Run RunAs test_user /boot/system/apps/StyledEdit. Source of RunAs is attached.
3. app_server will crash in Desktop::Init().

I am not sure what should happen in this case but definitely not crash.

[X512]

On test_app_server there are no crash and new desktop window is created.

[axeld]

A long-term plan of mine was to implement a multi seat ability into app_server. So when a different user starts an application, he should get his own Desktop -- just as what happens in the app_server test environment (however, the input_server does not support something like this yet IIRC).

If there are no resources for a second desktop available (or none are configured), it should probably just fail.

[waddlesplash]

IMHO, due to how much state app_server manages and how easy it will always be for data to get innocently "mixed up", or (worse) an actual bug causes an information leak, I think we should just start separate app_servers for separate user sessions.

The only exception to this would be running applications on one's own desktop as another user, i.e. a lesser user running something as administrator, but that will not lead to creation of a Desktop instance.

[X512]

I think we should just start separate app_servers for separate user sessions.

I prefer single global app_server because it is more resource efficient and easier to manage. Multiple app_servers will require separate server to handle graphics hardware. Running multiple user sessions fully secure is not possible without approach like Genode or each session in virtual machine.

due to how much state app_server manages and how easy it will always be for data to get innocently "mixed up", or (worse) an actual bug causes an information leak

Bugs should be fixed. Compared to win32k.sys or X.Org, app_server has clear architecture and it is possible to introduce proper permissions check. Currently Haiku kernel probably has more security issues, for example ​device_manager that I recently checked.

For first step, user processes (second launch_daemon and everything it launch) should run from separate user, not superuser (as visible on ​screenshot, all processes run with user 0) and privilege elevation should be used for administrative actions.

[waddlesplash]

Multiple app_servers will require separate server to handle graphics hardware.

We should have that anyway, so that we can have "remote desktop" users connected to a machine with app_servers that are not connected to the display output, and some who are.

Running multiple user sessions fully secure is not possible without approach like Genode or each session in virtual machine.

What does "fully secure" mean here? Obviously there will be some things that are unfixable, sure, but using separate processes for separate users solves a ton of problems right up front (for example, there is only one clipboard at present, and if we have multiple app_servers/registrars the code can stay mostly the same, vs. much more complicated logic for per-user clipboards.)

for example ​device_manager that I recently checked.

I am not sure how much of that comment still applies, as the code now uses user_memcpy and appears to reject non-user pointers. Regardless, there are certainly lots of kernel issues to fix indeed, and things that should be redesigned there, too, with security more in mind. We should do the same in userspace.

user processes (second launch_daemon and everything it launch) should run from separate user, not superuser

There are already multiple launch_daemons: one "master" to control the whole system, and then one per desktop session (as there is presently only one, for the root user, both processes of course run as root.)

[X512]

I am not sure how much of that comment still applies, as the code now uses user_memcpy and appears to reject non-user pointers.

Device node pointers are passed directly from userland without checks: device_node* node = (device_node*)cookie;. It can be used to access and modify kernel memory from userland. Some kind of ID like file descriptor should be used to identify device_node in userland. Or userland API should be changed so direct device_node will be not needed, for example use opened file descriptor with pointers to current device_node to communicate with userland.

[X512]

so that we can have "remote desktop" users connected to a machine with app_servers that are not connected to the display output, and some who are.

I see no problems in running single app_server for physical display and remote sessions. It is already working. app_server can still run on servers without display to handle remote desktop sessions.

for example, there is only one clipboard at present

registrar can be extended to handle clipboards and list of running applications for multiple user sessions. I think that it is easier to move clipboard to something like std::map<session_id, Clipboard> then completely redesigning app_server and registrar. registrar handle global system state like user database so global registrar is required. Session management also should be done in global registrar.

[waddlesplash]

registrar is not a very large program, splitting it up so that it can run as multiple processes will not be that much work.

I am still not sure why you are arguing so strongly against my proposal here. Perhaps it is more work to refactor things in this way (though I seriously doubt it; making all servers properly multi-user compliant is almost certainly far more work), but I think it is the better solution.

[X512]

I am still not sure why you are arguing so strongly against my proposal here.

I am just against software bloat and increased resource usage for no good reason (I can understand introducing ICU or package_fs because it give obvious practical benefits but it is not so for splitting processes or compositing). I like Haiku with low resource usage and small number of processes. It is also clear and easy to understand.

When changing something that can increase resource usage or complexity, it should be considered first from user perspective:

1. ICU: users from different countries and cultures get ability to use interface in their native language, proper date formats etc.

2. package_fs: it give ability to port and distribute a lot of new software for Haiku. Currently most Haiku activity is done at HaikuPorts.

3. Process splitting: nothing is changed from user perspective except increased memory usage and confusing duplicate processes in ProcessController.

4. GUI compositing: slow window rendering will change a bit, but memory usage will grow dramatically (especially considering common Haiku practice of using a lot of windows). Various GFX effects are not popular nowadays, mainstream OS GUI are minimalistic and can be done without compositing.

[waddlesplash]

We are not really discussing compositing here, I tend to agree with you on that point, though.

But the whole from having an app_server-per-user is that it will *not* use more resources if you only have one user active. There may be another server in the background handling display ownership (I expect its memory usage would be in the hundreds of KB, like the root launch_daemon is; hardly a significant amount to say the least), but otherwise, app_server's resource usage would be exactly the same.

On the other hand, if all data structures in app_server and registrar were multiuser-compliant, then we would indeed get lots of maps, lists, etc. to potentially handle multiple users, not to mention more logic that could be a performance impediment, even when only one user is in use.

[X512]

On the other hand, if all data structures in app_server and registrar were multiuser-compliant

app_server is already mostly multiuser-compilant. It is even working in test_app_server. For real app_server the only problem that prevents GUI applications of multiple users from running is missing of desktop switching (workspace switching is already present).

not to mention more logic that could be a performance impediment, even when only one user is in use.

Separate port and looper for each session can be used to avoid any table lookup overhead. app_server already has separate desktop threads and ports.

[korli]

Replying to X512:

Device node pointers are passed directly from userland without checks: device_node* node = (device_node*)cookie;. It can be used to access and modify kernel memory from userland. Some kind of ID like file descriptor should be used to identify device_node in userland. Or userland API should be changed so direct device_node will be not needed, for example use opened file descriptor with pointers to current device_node to communicate with userland.

yeah there is even a TODO about it in the one function involved. ​https://git.haiku-os.org/haiku/tree/src/system/kernel/device_manager/device_manager.cpp#n428

[waddlesplash]

Separate port and looper for each session can be used to avoid any table lookup overhead. app_server already has separate desktop threads and ports.

Will these be the only resources that are separated? Again, at this rate, why not use separate processes?

It is also worth noting that app_server has a lot of caches, e.g. Alpha Mask cache, font cache, etc. With multiple active sessions, performance could become degraded for all because the caches get filled faster. Making the caches larger so that it can hold multiple users' worth of data will then cause lookup to become slower. Again the simpler solution is just to use multiple servers.

[X512]

Will these be the only resources that are separated?

Common registrar resources:

1. User database.
2. MIME processing.
3. Shutdown processing.
4. Session table (not implemented).
5. Timers (move to libbe.so?).

Per-session resources:

1. Running applications table. Note that some applications should be inherited from root session like app_server and input_server. libbe.so interacts with input_server by it's signature.
2. Clipboard.

Again, at this rate, why not use separate processes?

Per-session data is just clipboard and application table. I see no benefits of moving this in separate process, threads are enough.

It is also worth noting that app_server has a lot of caches, e.g. Alpha Mask cache, font cache, etc. With multiple active sessions, performance could become degraded for all because the caches get filled faster.

This argument is working in opposite direction because different sessions will likely use same fonts and single process will give cache benefits. Multiple processes will have duplicate cache entries that will waste memory and slow down access.

[waddlesplash]

Common registrar resources:

We are mostly talking about app_server here, and only registrar secondarily. It may well be the case that registrar should have only a single instance but app_server should have multiple; they are not so closely tied.

MIME processing.

This actually needs to be at least partially per-user, since different users can have different applications installed, or different custom mimetypes registered, so what a file is sniffed as for one user can be different for another, or what path is launched for a mimetype different by user.

Timers (move to libbe.so?).

Yes, they should be moved indeed.

I see no benefits of moving this in separate process, threads are enough.

I was talking about app_server regarding port/looper separation, not registrar, here.

This argument is working in opposite direction because different sessions will likely use same fonts and single process will give cache benefits.

That is only half true, as different users can have different fonts installed, in use, set as default, etc. and so the glyph and fallback caches, despite the font sets being very similar, wind up being extremely different.

But fonts are the only thing this is true of. The alpha-mask cache, as I mentioned earlier, will be almost entirely different between users (unless they are doing almost the exact same things!) and so to share the alpha mask cache between users would just seriously degrade performance. And these are only examples, there are other resources shared inside app_server which make little sense to share across users (or cannot be shared for security reasons.)

Multiple processes will have duplicate cache entries that will waste memory and slow down access.

If we have got to the level where the slowed-down access is due to duplication in CPU caches... that is the last level of argument about performance, IMO. Most of the time, the CPU cache is not anything close to being a bottleneck for most algorithms.

But if we are talking purely about font caching, I am pretty sure this is exactly what fontconfig does on Linux, and there, every application will load some portion of the fontconfig cache into memory to draw text. On Haiku, only app_server does, so it is hardly a big deal if multiple processes use it.

It is also the case that we are talking about a small amount of memory here, maybe 4-8MB at absolute most. The vast majority of app_server memory is dedicated to per-application/per-desktop resources, not the small number of things like this that can even be potentially shared.

[X512]

We are mostly talking about app_server here, and only registrar secondarily.

Multi-user support in app_server is almost completed. Remaining part is desktop switching mechanism for same screen and corresponding protocol extensions. Most multi-user support work should be done in registrar because it currently have no support of user sessions.

I am surprised that I managed to get almost working multi-user sessions with test_app_server. Steps I did:

1. Create test_user with /boot/home/test_user as home directory in my Users utility.
2. Copy default user home directory from haiku-nightly.image to /boot/home/test_user and run chown -R test_user:users . in it.
3. Start test_registrar and test_app_server.
4. In separate Terminal run su test_user and then run Tracker &, Deskbar & etc. test_app_server window for test_user will open.
5. Tracker and other software successfully recognized home directory and write separate settings. For example different Deskbar settings can be used. Access rights are working.

Other users and home directories can be also created.

I was talking about app_server regarding port/looper separation, not registrar, here.

It is already working just now. The only problem is desktop switching on same screen. I see no reason of spending time for major refactor of already working code.

The alpha-mask cache, as I mentioned earlier, will be almost entirely different between users (unless they are doing almost the exact same things!) and so to share the alpha mask cache between users would just seriously degrade performance.

Alpha-mask cache is related to applications, not user sessions. Common alpha-mask cache will be also not useful for multiple applications of same session that are doing different things.

[waddlesplash]

I see no reason of spending time for major refactor of already working code.

Because I really think that when you add up all the smaller issues, it simply makes more sense to go the multi-process route. Take the security aspect for instance: besides potentially leaking data between users, it is also the case that the attack surface in app_server is just massive. Having app_server run as each individual user means that an exploit in it will not really gain a user anything in particular, whereas an exploit to a root app_server would result in privilege escalation, etc.

Or, there is the aspect of memory usage: on 32-bit systems, it is possible to exhaust app_server's virtual memory space without exhausting physical memory (even without PAE). Under heavy use, it is theoretically to fragment the address space so heavily that despite having plenty free, there is no way to allocate a large enough chunk. This rarely if ever occurs now, but with multiple users, it will be far easier to trigger; and 32-bit is hardly a dead architecture.

There are plenty of other concerns, too. I think when you add all these up, the benefit from a multiprocess app_server is just far greater than the few MB of memory usage saved by sharing some data. (Or, for that matter, we could even retain that memory sharing in a multiprocess app_server.)