On Tue, Jun 23, 2020 at 11:05 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> 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...
yeah. sorry. -enocoffee. It needs more work.
