From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6DB093CD8DE for ; Thu, 9 Jul 2026 06:47:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.42 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783579648; cv=none; b=Fa1eMkxXjC96lyOE/VRZR5T5/ZDBNxLETZ0QbuF6wqNDUtqmtXws6gsZG179sugTrcKvG+NfuQuxmD6+qut1ngRaq0YCVUKVZvGDgrEDyadi6gcChpmis+0BB+mt6hIo6t7WSASdjD/HmObuvav6u01+rWM/c1zM49DPKxUwAA4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783579648; c=relaxed/simple; bh=P3DdR4s1kLt7WzVKHUj6fTXk+PI5dgeDUEMELoOfajg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Qvdx6ZCzXWowJxJaYgNtyhDWhQvffADbsD+O90nbw0lzKUjyKtbds/YJLu4ZS3lWodL+vLRuLBXB/ykBQMmqVjy+RaHSfI7EcWS6kDgzvuJBvDCMM2oDcyBM8E2KH4T8KBagoNf57JY22kWQx7anObH1D2xGnInOQ4sihEIpN0k= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=UPx+Wyct; arc=none smtp.client-ip=209.85.208.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="UPx+Wyct" Received: by mail-ed1-f42.google.com with SMTP id 4fb4d7f45d1cf-693c51a8a19so814018a12.3 for ; Wed, 08 Jul 2026 23:47:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1783579645; x=1784184445; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:message-id:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to:content-type; bh=h3B/Dq0dBNn27ZKW7Kn4NVCyOhndmbS7oy98sHeCH+0=; b=UPx+WyctGb1piyweU10L55Y8bUQW/MCOHJ3vM3TW0oCsCE2xIG7a9mioBZSXY7LVzc qIU8ni8gQz0Qn6IFpXtPMDjnVsvOgKXjDUNX1nCxFWc/BT8M33/quXqePZL+uLqFgI1S MhkllHKcYgrlj/o7xwGJDz+gbfCnEs4RCNsH9vHpZ7lbOdcrFmPOsjV+LR3vES82OPBj uzkXEh+NJOCX1txEcwsADiaz/yAe/rzEXL5a+5YNvCBVStumBOXJHRJKSX6kWkNNXDik Vi62PJQOLTk6mNAZvFX+QZlrJ1uZnXKUJi4es9MLH0mjQ9zXz6BZDcjsCNPOkmbfTvsU tAng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783579645; x=1784184445; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:message-id:subject:cc:to:from :date:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=h3B/Dq0dBNn27ZKW7Kn4NVCyOhndmbS7oy98sHeCH+0=; b=lbQvOxeOo6ryPEh8HwB91B7Zoxzj12mR6r9wrCrKtcOasxgBRAW9JntTSWNyhvfgsc h+ujQT9K38PD2OFrIoZP7r6xAMdCJiNMOL/IT68qbEkwvG3LMAxXpyWB2S9yzZxz1LJ/ OI4kpjHZcWxTAB99f3qMtDLvoRpFW00cvuEVcWyWlNVKe5zUdgpmd/mcQmbg88zvdy54 /PSyriTZynufGV8I/QbgEMbifGeQOyxfuCHLQmYZPtjTzxOU8yxVNzTylGbscDcUN5QJ X6ZEByDuHK6jeh7NWjAOkw+cvno1bldrAnSjZyHd6/43BOhn+HZ3gTXSyyinZwuVD0ra wmKQ== X-Forwarded-Encrypted: i=1; AHgh+RpdthrVrPMLB8lJI9uysINM1f0jEImJzUIsvNsNEt4lguUuPgIdboVeItwBPG0hzEeDWK+ZoQjHrtU85UE=@vger.kernel.org X-Gm-Message-State: AOJu0Yy81YRVALEeKFgPwn7b7jfrPqMlT9sO+GCK+OVgZcd8+6dwHl/p LSLizoJ959bCA5KpILg5FFhWnOMDy629Z+mdB/47k49FIOKWXGwkvBz2dawvvduZsBg= X-Gm-Gg: AfdE7cmGFqU3lyTR5xuXthySGo3+tWH9NEaGQWPDq3iGUSYESuYbi7+TlXX8mAe1VFh caibq8RWlkY/g1OHPJsQHz0wmtZqsev64JUJ+cLYis7kOQPBE6JJM+SBT1xrxyw3EXZfFdrz6/m c1u4UvHFCU0IWAOfPPSeInXa93yxHsVKssWUSPXaHikDtQFMUVlFoIXJQ5lrAIr66frGlrMMRVH M1u2ABJ6K+CUkPeH6DIHMomTwjyGFoDbuumkxXBs2u5wlCaS//OtTY1Yd1x2e7wbxQGwZmNmIRt bi3vt3NlOjCTMLIz12h5NiEQyDtal1BSh/tR/yDbbqR/Oa5R+GfQj6hZGxYPNcw0Qm+8BcguUO3 +dY9XJU84/zdZtQBbgaVby0nliFUkFHzq1/9VMTNfZSfocUEQrBFncaa3wwutIGWrjb6kUFlXcG NtwizkIwDl9koNp9u2NZhtapXx5+HaA9HN+kM= X-Received: by 2002:a17:907:1c0e:b0:c15:b68a:55c with SMTP id a640c23a62f3a-c15cdffedb4mr269256466b.24.1783579644714; Wed, 08 Jul 2026 23:47:24 -0700 (PDT) Received: from u94a (27-53-128-132.adsl.fetnet.net. [27.53.128.132]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-848530c5a12sm969893b3a.5.2026.07.08.23.47.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Jul 2026 23:47:23 -0700 (PDT) Date: Thu, 9 Jul 2026 14:47:03 +0800 From: Shung-Hsi Yu To: sun jian Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, john.fastabend@gmail.com, andrii@kernel.org, eddyz87@gmail.com, memxor@gmail.com, martin.lau@linux.dev, song@kernel.org, yonghong.song@linux.dev, jolsa@kernel.org, emil@etsalapatis.com, shuah@kernel.org, mmullins@fb.com, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH bpf v4 1/2] bpf: Reject negative const offsets for buffer pointers Message-ID: References: <20260708090151.151729-1-sun.jian.kdev@gmail.com> <20260708090151.151729-2-sun.jian.kdev@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote: [...] > > > @@ -5326,14 +5326,18 @@ static int check_max_stack_depth(struct bpf_verifier_env *env) > > > static int __check_buffer_access(struct bpf_verifier_env *env, > > > const char *buf_info, > > > const struct bpf_reg_state *reg, > > > - argno_t argno, int off, int size) > > > + argno_t argno, int off, int size, > > > + u32 *access_end) > > > { > > > + s64 start, var_off; > > > + > > > if (off < 0) { > > > verbose(env, > > > "%s invalid %s buffer access: off=%d, size=%d\n", > > > reg_arg_name(env, argno), buf_info, off, size); > > > return -EACCES; > > > } > > > + > > > if (!tnum_is_const(reg->var_off)) { > > > char tn_buf[48]; > > > > > > @@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env *env, > > > return -EACCES; > > > } > > > > > > + var_off = (s64)reg->var_off.value; > > > + if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF) { > > > + verbose(env, "%s %s buffer offset %lld is not allowed\n", > > > + reg_arg_name(env, argno), buf_info, var_off); > > > + return -EACCES; > > > + } > > > + > > > + start = var_off + off; > > > + if (start < 0) { > > > + verbose(env, > > > + "%s invalid negative %s buffer offset: off=%d, var_off=%lld\n", > > > + reg_arg_name(env, argno), buf_info, off, var_off); > > > + return -EACCES; > > > + } > > > > I was thinking of suggest to just do a single unsigned check > > > > var_off = reg->var_off.value; > > if (var_off >= BPF_MAX_VAR_OFF) { > > ... > > > > But looking at the code before 022ac0750883, what you have is closer > > aligned to the previous behavior, let's stick to this. Actually looking again at 022ac0750883, moving the `off < 0` after tnum_is_const() and bringing back the `off += reg->off` removed from check_mem_access() is perhaps the more faithful restoration of the original behavior. Though reg->off no longer exists, we have to use reg->var_off.value instead. IIUC any register of pointer type should already have its var_off bounded to +-BPF_MAX_VAR_OFF by adjust_ptr_min_max_vals() in theory, and thus shouldn't overflow `int off`. See the diff below. [...] > I agree that the size check is redundant given the current call path. I’ll leave > v4 as-is for now to avoid another respin unless maintainers prefer dropping it > or adding a short comment around the access_end calculation. Agree and make sense. Let's see what @Eduard thinks. --- diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index d46f7db20d8f..e116b33ad83e 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -5359,12 +5359,6 @@ static int __check_buffer_access(struct bpf_verifier_env *env, const struct bpf_reg_state *reg, argno_t argno, int off, int size) { - if (off < 0) { - verbose(env, - "%s invalid %s buffer access: off=%d, size=%d\n", - reg_arg_name(env, argno), buf_info, off, size); - return -EACCES; - } if (!tnum_is_const(reg->var_off)) { char tn_buf[48]; @@ -5375,6 +5369,14 @@ static int __check_buffer_access(struct bpf_verifier_env *env, return -EACCES; } + off += reg->var_off.value; + if (off < 0) { + verbose(env, + "%s invalid %s buffer access: off=%d, size=%d\n", + reg_arg_name(env, argno), buf_info, off, size); + return -EACCES; + } + return 0; }