Subject: Re: [RFC PATCH bpf-next v2 4/4] error-injection: Support fault
 injection framework
To: Masami Hiramatsu <mhiramat@kernel.org>
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>
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>
From: Alexei Starovoitov <ast@fb.com>
Message-ID: <099fa7d4-435a-3fa4-841c-17603a45e77e@fb.com>
Date: Thu, 28 Dec 2017 17:11:31 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0)
 Gecko/20100101 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <20171228165140.0f9fc7f6067b5581c018e81d@kernel.org>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Dec 2017 01:11:37.4226 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 63e5d871-6fa2-4a37-2c12-08d54e591daa
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL2PR15MB0962
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-12-29_01:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1529
Lines: 40

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.
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.