Code review comment for lp://qastaging/~kyrofa/snap-confine/create_user_data_directory

Revision history for this message
Kyle Fazzari (kyrofa) wrote :

Thanks for taking a look, Seth.

> Does this code run with more privileges than the services that it is working for?

Yes, the launcher runs with setuid root.

> If so, does something prevent the services from running at the same time?

No, nothing prevents services (or binaries) from running at the same time, but from what I've read mkdir() is atomic. Perhaps I misunderstood your concern here?

> If so, does something fix the directories' owner and group later?

Ah, great catch, I need to do that. Services would have been fine, but as-is this will create directories owned by root for binaries, which isn't what we want here. There are two ways I see to do that: the chown() syscall after the fact, or fork and drop privs before creating the directories. Normally I'd say the latter would be more secure so as to mitigate race condition attacks, but since we'd only ever be dropping privilege levels with the chown() (from root to user, never the other way around), I wonder if it's really a concern. Thoughts?

« Back to merge proposal