DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6460E218B1
Date: Fri, 29 Dec 2017 16:34:35 +0900
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Alexei Starovoitov <ast@fb.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
        Josef Bacik <jbacik@fb.com>, <rostedt@goodmis.org>, <mingo@redhat.com>,
        <davem@davemloft.net>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <ast@kernel.org>, <kernel-team@fb.com>,
        <daniel@iogearbox.net>, <linux-btrfs@vger.kernel.org>,
        <darrick.wong@oracle.com>, Josef Bacik <josef@toxicpanda.com>,
        Akinobu Mita <akinobu.mita@gmail.com>
Subject: Re: [RFC PATCH bpf-next v2 4/4] error-injection: Support fault
 injection framework
Message-Id: <20171229163435.8ac2a001b3b3422d012bbd95@kernel.org>
In-Reply-To: <099fa7d4-435a-3fa4-841c-17603a45e77e@fb.com>
References: <151427438796.32561.4235654585430455286.stgit@devbox>
        <151427450538.32561.2776225740675148782.stgit@devbox>
        <20171227021255.sahkcpgyzohl5brs@ast-mbp>
        <20171227170910.5ac1074bc86341f194130119@kernel.org>
        <87e5e909-593d-7fd6-c7bb-714c1e3022a0@fb.com>
        <20171228103849.9be1a4507298b280b9df9c20@kernel.org>
        <126a806d-e913-d243-6678-56fc3cff5e9e@fb.com>
        <20171228165140.0f9fc7f6067b5581c018e81d@kernel.org>
        <099fa7d4-435a-3fa4-841c-17603a45e77e@fb.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2043
Lines: 57

On Thu, 28 Dec 2017 17:11:31 -0800
Alexei Starovoitov <ast@fb.com> wrote:

> On 12/27/17 11:51 PM, Masami Hiramatsu wrote:
> >
> > Then what happen if the user set invalid retval to those functions?
> > even if we limit the injectable functions, it can cause a problem,
> >
> > for example,
> >
> >  obj = func_return_object();
> >  if (!obj) {
> >     handling_error...;
> >  }
> >  obj->field = x;
> >
> > In this case, obviously func_return_object() must return NULL if there is
> > an error, not -ENOMEM. But without the correct retval information, how would
> > you check the BPF code doesn't cause a trouble?
> > Currently it seems you are expecting only the functions which return error code.
> >
> >  ret = func_return_state();
> >  if (ret < 0) {
> >     handling_error...;
> >  }
> >
> > But how we can distinguish those?
> >
> > If we have the error range for each function, we can ensure what is
> > *correct* error code, NULL or errno, or any other error numbers. :)
> 
> messing up return values may cause problems and range check is
> not going to magically help.
> The caller may handle only a certain set of errors or interpret
> some of them like EBUSY as a signal to retry.
> It's plain impossible to make sure that kernel will be functional
> after error injection has been made.

Hmm, if so, why we need this injectable table?
If we can not make sure the safeness of the error injection (of course, yes)
why we need to limit the error injection on such limited functions?
I think we don't need it anymore. Any function can be injectable, and no
need to make sure the safeness.

Thank you,

> Like kmalloc() unconditionally returning NULL will be deadly
> for the kernel, hence this patch 4/4 has very limited practical
> use. The bpf program need to make intelligent decisions when
> to return an error and what kind of error to return.
> Doing blank range check adds a false sense of additional safety.
> More so it wastes kilobytes of memory to do this check, hence nack.
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>