On Mon, May 12, 2025 at 12:19:01AM +0800, Chen Linxuan wrote: > On Sun, May 11, 2025 at 10:22 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > > Excuse me, just what is that path_get/path_put for? If you have > > an opened file, you do have its ->f_path pinned and unchanging. > > Otherwise this call of path_get() would've itself been unsafe... > > I am not very familiar with how these functions should be used. > I just copied similar logic from proc_exe_link and added a path_put. > Maybe I made a stupid mistake... path_get() grabs an extra reference to mount and dentry; that is enough to guarantee that their refcount will not drop to zero at least until we drop the references. To use it safely you need to be sure that currently refcounts are non-zero and won't be dropped to zero right under you - for rather obvious reasons. Your code also clearly relies upon ->f_path being unchanging - otherwise you would've risked to drop the references not to the objects you've grabbed earlier. If both conditions are satisfied, what's the point of grabbing and dropping these references in the first place? *IF* there was a chance of mount going away under you, what would prevent that happening right before that path_get()? IOW, these dances with path_get()/path_put() are either nowhere near enough or not needed at all. As it is, ->f_path of an open file *IS* stable and mount and dentry references in it do contribute towards mount and dentry refcounts. But my point is, that code does not pass the basic "how could that possibly be right?" test. PS: since you've mentioned proc_exe_link()... the difference there is that we want the reference to be used by the caller. If the file happens to be closed just as we return (that can't happen until proc_exe_link() drops the reference to struct file it got from get_task_exe_file(), but as soon as fput() had been called, there's nothing to guarantee the safety), file's contributions to refcounts are dropped, so we can't count upon them to stay for as long as we need. So in that case we fetch the references out of ->f_path and pin them before we drop the file reference with fput(). PPS: I'm still not convinced regarding the usefulness of having that information; "just because we can display it" isn't a strong argument on its own...
