On Wed, Jun 24, 2020 at 10:51:15AM +0900, Tetsuo Handa wrote: > On 2020/06/24 4:40, Alexei Starovoitov wrote: > > There is no refcnt bug. It was a user error on tomoyo side. > > fork_blob() works as expected. > > Absolutely wrong! Any check which returns an error during current->in_execve == 1 > will cause this refcnt bug. You are simply ignoring that there is possibility > that execve() fails. you mean security_bprm_creds_for_exec() denying exec? hmm. got it. refcnt model needs to change then. > > Not true again. > > usermode_blob is part of the kernel module. > > Disagree. Disagree with what? that blob is part of kernel module? huh? what is it then? > > > Kernel module when loaded doesn't have path. > > Disagree. > > Kernel modules can be trusted via module signature mechanism, and the byte array > (which contains code / data) is protected by keeping that byte array within the > kernel address space. Therefore, pathname based security does not need to complain > that there is no pathname when kernel module is loaded. I already explained upthread that blob is part of .rodata or .init.rodata of kernel module and covered by the same signature mechanism. > However, regarding usermode_blob, although the byte array (which contains code / data) > might be initially loaded from the kernel space (which is protected), that byte array > is no longer protected (e.g. SIGKILL, strace()) when executed because they are placed > in the user address space. Thus, LSM modules (including pathname based security) want > to control how that byte array can behave. It's privileged memory regardless. root can poke into kernel or any process memory. > On 2020/06/24 3:53, Eric W. Biederman wrote: > > This isn't work anyone else can do because there are not yet any real in > > tree users of fork_blob. The fact that no one else can make > > substantials changes to the code because it has no users is what gets in > > the way of maintenance. > > It sounds to me that fork_blob() is a dangerous interface which anonymously > allows arbitrary behavior in an unprotected environment. Therefore, I think you missed the part that user blob is signed as part of kernel module. > > Either a path needs to be provided or the LSMs that work in terms > > of paths need to be fixed. > > LSM modules want to control how that byte array can behave. But Alexei > still does not explain how information for LSM modules can be provided. huh? please see net/bpfilter/. > > > My recomendation for long term maintenance is to split fork_blob into 2 > > functions: fs_from_blob, and the ordinary call_usermodehelper_exec. > > That removes the need for any special support for anything in the exec > > path because your blob will also have a path for your file, and the > > file in the filesystem can be reused for restart. > > Yes, that would be an approach for providing information for LSM modules. > > > But with no in-tree users none of us can do anything bug guess what > > the actual requirements of fork_usermode_blob are. > > Exactly. Since it is not explained why the usermode process started by > fork_usermode_blob() cannot interfere (or be interfered by) the rest of > the system (including normal usermode processes), the byte array comes from > the kernel address space is insufficient for convincing LSM modules to > ignore what that byte array can do. Sounds like tomoyo doesn't trust kernel modules. I don't think that is fixable with any amount of explantation.
