This got introduced as a part of code reuse, which is a good thing :-). However it introduced an additional vfs_ChDir($cwd_fname) in unbecome_root() due to consolidating the restore code in pop_sec_ctx(). This is unexpected behavior (although harmless as it's doing a chdir to where the calling code thinks it already is). become_root()/unbecome_root() should only change security contexts, not filesystem directory position - it's certainly not expected by callers of the API (and indeed no callers expect or depend on this). Discussed with Metze at SambaXP. Patch to follow.
Created attachment 15253 [details] git-am fix for master. Metze, this is what I went through with you at SambaXP. Cheers, Jeremy.
Comment on attachment 15253 [details] git-am fix for master. I just noticed that we have current_user.need_chdir = false; current_user.done_chdir = false; in smbd_change_to_root_user(). I think we should add that also at the end of smbd_become_root(), which would have the same effect as your patch. In addition I'd like to see asserts that we never call change_to_{user,user_by_fsp,guest,root_user}() from within a become_{user,root}() section. And a single become_root() would generate an error if vfs_ChDir() or smb_vfs_call_chdir() is called before the matching unbecome_root(). I'll post some draft patches shortly. global 'bool that is set by smbd_become_root
Created attachment 15254 [details] WIP patch for master Can you have a look at these patches? Having assertions for the invalid API usage feels be much better (at least for master). I haven't tested the patches, so maybe there are off by one errors... Can you fill the TODO's to give useful debug messages, so that it's easy for us and external vfs module writers to debug things when they hit an assertion? Thanks!
OK, will be looking over them tomorrow. Might add lots more comments so I can understand exactly what is going on :-). Thanks, Jeremy.
Fixed by the 9 commits leading up to 35cd91ee4d8. Big changes, master only (which became 4.12), no backports.