Received-SPF: pass (google.com: domain of linux-kernel+bounces-138845-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <4bfc3494-a3e9-4b4c-9d93-fa1049a10235@linux.dev>
Date: Wed, 10 Apr 2024 08:09:45 -0700
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PATCH] bpf: strnchr not suitable for getting NUL-terminator
Content-Language: en-GB
To: Edward Adam Davis <eadavis@qq.com>, bpf@vger.kernel.org
Cc: andrii@kernel.org, ast@kernel.org, daniel@iogearbox.net,
 haoluo@google.com, john.fastabend@gmail.com, jolsa@kernel.org,
 kpsingh@kernel.org, linux-kernel@vger.kernel.org, martin.lau@linux.dev,
 sdf@google.com, song@kernel.org, syzkaller-bugs@googlegroups.com
References: <tencent_EC72CD3879FA6F102FC56E4495F0E822EC0A@qq.com>
From: Yonghong Song <yonghong.song@linux.dev>
In-Reply-To: <tencent_EC72CD3879FA6F102FC56E4495F0E822EC0A@qq.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 4/9/24 5:33 PM, Edward Adam Davis wrote:
> The strnchr() is not suitable for obtaining the end of a string with a length
> exceeding 1 and ending with a NUL character.

Could you give more detailed explanation with specific examples? I think
strnchr() does the right thing here. Note that if fmt is not NULL,
strnchrnul() never returns NULL pointer so in the change below,
'if (!fmt_end)' will be always false.

>
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>   kernel/bpf/helpers.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 449b9a5d3fe3..07490eba24fe 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -826,7 +826,7 @@ int bpf_bprintf_prepare(char *fmt, u32 fmt_size, const u64 *raw_args,
>   	u64 cur_arg;
>   	char fmt_ptype, cur_ip[16], ip_spec[] = "%pXX";
>   
> -	fmt_end = strnchr(fmt, fmt_size, 0);
> +	fmt_end = strnchrnul(fmt, fmt_size, 0);
>   	if (!fmt_end)
>   		return -EINVAL;
>   	fmt_size = fmt_end - fmt;