On Mon, Jan 13, 2025 at 03:55:42PM +0100, Jann Horn wrote: > +Christian and Al Viro to double-check what I'm saying > > On Wed, Jan 8, 2025 at 4:44 PM Mickaël Salaün <[email protected]> wrote: > > -static const void *get_current_exe(const char **path_str, size_t > > *path_size) > > +static const void *get_current_exe(const char **path_str, size_t > > *path_size, > > + struct inode **inode) > > { > > struct mm_struct *mm = current->mm; > > struct file *file __free(fput) = NULL; > > @@ -93,6 +96,8 @@ static const void *get_current_exe(const char **path_str, > > size_t *path_size) > > > > *path_size = size; > > *path_str = path; > > + ihold(file_inode(file)); > > + *inode = file_inode(file); > > return no_free_ptr(buffer); > > } > > This looks unsafe: Once the reference to the file has been dropped > (which happens implicitly on return from get_current_exe()), nothing > holds a reference on the mount point or superblock anymore (the file > was previously holding a reference to the mount point through > ->f_path.mnt), and so the superblock can be torn down and freed. But > the reference to the inode lives longer and is only cleaned up on > return from the caller get_current_details(). > > So I think this code can hit the error check for "Busy inodes after > unmount" in generic_shutdown_super(), which indicates that in theory, > use-after-free can occur. > > For context, here are two older kernel security issues that also > involved superblock UAF due to assuming that it's possible to just > hold refcounted references to inodes: > > https://project-zero.issues.chromium.org/42451116 > https://project-zero.issues.chromium.org/379667898
Thanks for the detailed explanation! > > For fixing this, one option would be to copy the entire "struct path" > (which holds references on both the mount point and the inode) instead > of just copying the inode pointer. Yes, I'll do that.
