On 12/14/23 12:07, Ján Tomko wrote: > On a Thursday in 2023, Michal Prívozník wrote: >> On 12/13/23 15:47, Ján Tomko wrote: >>> When this check was introduced, virtiofsd required root privileges. >>> >>> This has changed since then - now it does not need to set up all the >>> sandboxing when running as non-root. It even gained support for >>> id mapping, which makes running unprivileged even more useful. >>> >>> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx> >>> --- >>> src/qemu/qemu_virtiofs.c | 4 ---- >>> 1 file changed, 4 deletions(-) >>> >>> diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c >>> index af51d58673..4dacd37a1c 100644 >>> --- a/src/qemu/qemu_virtiofs.c >>> +++ b/src/qemu/qemu_virtiofs.c >>> @@ -257,10 +257,6 @@ qemuVirtioFSStart(virQEMUDriver *driver, >>> if (!(cmd = qemuVirtioFSBuildCommandLine(cfg, fs, &fd))) >>> goto error; >>> >>> - /* so far only running as root is supported */ >>> - virCommandSetUID(cmd, 0); >>> - virCommandSetGID(cmd, 0); >> >> This makes us unable to run C version of virtiofsd, becuase that one >> does privileged syscalls from the very start. I mean, you can't even run >> `virtiofsd --help` as a non-root. > > The users of the C version (and all the Rust versions that make > privileged calls) will still be able to run virtiofsd and it will still > be run as root for privileged libvirtd. > > The downside is a worse error message. > > > If I remember correctly, the only reason I added these lines was > a quick way to give unprivileged users an early error without having > to write an error message. > > The above lines were added by: > commit f0f986efa8a8e352fbdce7079ec440a4f3c8f522 > qemu: add code for handling virtiofsd > The proper validation error (which I also propose to remove earlier in > this series) was added by: > commit efaf46811c909ee5333360fba1d75ae82352964a > qemu: validate virtiofs filesystems > > Both commits were committed upstream at the same time, so I'm not sure > why I kept it there. This makes even less sense than I anticipated. In 6/9 you remove the check which guards running virtiofsd in session mode (which in case of qemu driver means uid === 0). Why we have so many checks is beyond me. Anyway, for some weird reason, after these lines are removed I can still run the C implementation (which is possibly a sign of a deeper problem anyways). So, if you fix the commit message so that it describes what is really happening (libvirtd runs as root and thus virtiofsd is ran as root too), you have my: Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx> Michal _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
