On Tue, Jun 23, 2020 at 9:00 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> 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.
I think the following trivial change should do it:
diff --git a/kernel/umh.c b/kernel/umh.c
index 79f139a7ca03..f80dd2a93ca4 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -512,7 +512,9 @@ int fork_usermode_blob(void *data, size_t len,
struct umh_info *info)
file = shmem_kernel_file_setup("", len, 0);
if (IS_ERR(file))
return PTR_ERR(file);
-
+ err = deny_write_access(file);
+ if (err)
+ goto out_fput;
written = kernel_write(file, data, len, &pos);
if (written != len) {
err = written;
@@ -532,8 +534,11 @@ int fork_usermode_blob(void *data, size_t len,
struct umh_info *info)
mutex_lock(&umh_list_lock);
list_add(&info->list, &umh_list);
mutex_unlock(&umh_list_lock);
+ return 0;
}
out:
+ allow_write_access(file);
+out_fput:
fput(file);
return err;
}
I'll do more tests tomorrow and send it with SOB.
