Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e186cbd5-4c22-8069-717d-35bb8f8e4fff@fb.com>
Date:   Fri, 29 Jul 2022 10:15:21 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.11.0
Subject: Re: [PATCH] bpf/verifier: fix control flow issues in
 __reg64_bound_u32()
Content-Language: en-US
To:     Zeng Jingxiang <zengjx95@gmail.com>, ast@kernel.org,
        daniel@iogearbox.net, john.fastabend@gmail.com, andrii@kernel.org,
        martin.lau@linux.dev, song@kernel.org, kpsingh@kernel.org,
        sdf@google.com, haoluo@google.com, jolsa@kernel.org
Cc:     bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
        Zeng Jingxiang <linuszeng@tencent.com>
References: <20220729054958.2151520-1-zengjx95@gmail.com>
From:   Yonghong Song <yhs@fb.com>
In-Reply-To: <20220729054958.2151520-1-zengjx95@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?R0l3WWc3eDdXaWVhQ3ZoVndsRGViUDRaTVNublh3dUxodUZXRzFaUjN3WHZ2?=
 =?utf-8?B?STIzc3o5KzdHVlZkZHc3alhrcmptaGpWVmMzNW9iYVFuaHFGNjVnUFhpbnpJ?=
 =?utf-8?B?UEdXU2M3UEljSkd5SzJzeXc0YkhVU29VeC9YaC9TbUZzWHpWM0lNeWV2MHNq?=
 =?utf-8?B?dm1oVUlyb3ZyY2RPSUVQY3JGaG9qbUE2bFBSNVNPUWFsVEZ6bW1zNzMxY1V4?=
 =?utf-8?B?b2FjVEgyWU4vNXN5TUVBMG8xTEI3UWpiMTIwS0JTSThJalZqeUd3RmxoWU9u?=
 =?utf-8?B?citOamRxZDZhVkJMdlZIK1MzQ0R2aE9CODRPTElPNmtYNFB1dFFJQ20rSC9S?=
 =?utf-8?B?R1lyU3ZscGJ3dEwwdmtYMFJWSFZqTVYyS1A5SWp6M1lybDd0RHFucHU4YTFu?=
 =?utf-8?B?N01aa0lzSUh0OGNpd2JBWklmZUFwWTNGSVZMSTVtS2tHbGd4SWhqemRQZmQ3?=
 =?utf-8?B?aHB3K1BscWtUbnRyczFVbGtLNkJlbXIrK0hzOFRhbDZ0anV2QzlRdFgybGx6?=
 =?utf-8?B?NjFBaDRzV2lSalkyWEhLL1dLeVBTYUdNeGNicWZwb252WWw2cDI3L0pGWURQ?=
 =?utf-8?B?Wk12aGZSN2dXdFhvdi9EYko1YWhGckc5RysrbXVoODN1NGtHeDJIWmtFeGpy?=
 =?utf-8?B?STljeXhWeTJlRjE0RzBpdnlaVXU0ZEx3WndzOGZESmJCZVFNNnJ0a0FOZTIz?=
 =?utf-8?B?WkFTK1NLZ0Q2dmErMXk0TUtQZmVsWk9KV25oSUQ1M3RqSHlaRVRjVGNqb05D?=
 =?utf-8?B?SVh2ZlRxalVDQVlEa1hxelJyd0IxMlVWcCs3M1RrY3RqRmNLWk82Q3I1OEZ3?=
 =?utf-8?B?V1lRSCtPWXo3MVBVYTVHZk54ZEl4bG5rRWYyb2JSR2FNVy9iY2hxRVordWZx?=
 =?utf-8?B?V1creWlIKzFpSm9ScDZYZE9ZNGtUQ2l1VGU2ZFVuaDErVVNGZ2FybXV1NUk0?=
 =?utf-8?B?TFhWSFpYeXk3bG5iTG8vREJubC9ZRXR6VThzYnFKN3dZN3N5c0Y3T0FqTk9R?=
 =?utf-8?B?NXlNNURNSGwvS2wyRXJVM201R0Q5NkF5c0k2U1ZZRFJvTzJwL3pSOEd2SlZP?=
 =?utf-8?B?SmUrRHpld1N0RGFCcklralU0cHkxckNkbnlRT01GN3RVNExRUG95citmcjN4?=
 =?utf-8?B?Zzd5RVppZysyVUhtRDQ5REhSZXB6TERicERxSHlKdnVydU9QQVY2WjBHOWhr?=
 =?utf-8?B?VzNYd21Td3dwOGdDakxVbFdrSkFSNEU3eDNjaHRVQkhyemVsN2pNdjVodUNn?=
 =?utf-8?B?S0lwV2JmRk5vTVpZTmpiemZuK0ZJTzl0UnZCbXZiZkV6Z2hVK1RSVTV1bXdl?=
 =?utf-8?B?ZTJtUEpEU0hpb2FEK1FnZGY1YVdQT3RVS28vV3RoTXRFNDhybnloRHFIMEhn?=
 =?utf-8?B?TFdlaDJJMHp2dVZtVmlMU0lHanNFUXY2MWZrNW9GS1hRQTl2T2ZZYllNTjhE?=
 =?utf-8?B?d0lDdjZ3RG9reFFVaFUzTzMzRU91N2l6b1NYU2NTQmRJSzkyZVRBNWxvd2JC?=
 =?utf-8?B?eW9XQ1ByVjgzU0M3YjZTODl6WEFPTXdKQUtnSmlsM1huYW44OU92T0lWbTZn?=
 =?utf-8?B?NlNQdFN1Zzk3TzJGK3pEMjh2d0MrZ0VuYmlGYkNnTFNvSUExYTZzZSt0bXBm?=
 =?utf-8?B?QTF5ekphNCtRS09wbmpRSTBER3JvSFZHVVNzejJQQm83Z3Q4ak15bmlERmdH?=
 =?utf-8?B?MXhWZlBGQjR2VzFubCtpa3F3QnRmT2pVRndXK0p1cVc4alVEd05wQVI4Nlhw?=
 =?utf-8?B?b1NRU2d6eHk2QXFGb2lLT1B6Mng5c2N5a29YR3h5VTk0ci9EQXdNQlBabFA3?=
 =?utf-8?B?dE9TWFovV1NKTkw2WkZXelFuY2lTVHdDMWVJR2laR1lLbWswOU5nejBucnR5?=
 =?utf-8?B?bU1vamNWbkliVHo4STYrSXh1RmxiYzFpcGY5MVErekxteHp1bk43Q0JLWHBL?=
 =?utf-8?B?ejV4ZG4wTEpNbkd2d0tvdTkrUVBYTlJ1SlZqdFh0ejhCYzQyL2xSenFxMHIy?=
 =?utf-8?B?Zk5hWlFwQk1Bd0MvZzJkd3RqQVZ6QldIVWp4U09yQTNCMjNvN01zTUpTaFRY?=
 =?utf-8?B?ZlRTUGhVeUtkSGdmTlBYaVE0TWRvU0lMMFpMM2s4NWt5azdubGhZYjVRdkJk?=
 =?utf-8?B?VVlCNGpMQ3g5WFVqZWQzcVh3aHBPTUViaXc1VzB1KzdqcjZnb1E2Y1FzQkxQ?=
 =?utf-8?B?eXc9PQ==?=
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cfd7346e-c8ff-424f-ea15-08da7185eca8
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jul 2022 17:15:24.2834
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qYIBPaq1gWM0Q54W3w0DtLrQ4oO/KtAee0I9Y5ZwsJJJBkpSbZEsHXftVvd40cbO
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR15MB4269
X-Proofpoint-GUID: H6i5fn7BmQreupWrQi32B6g-vfm4nMSG
X-Proofpoint-ORIG-GUID: H6i5fn7BmQreupWrQi32B6g-vfm4nMSG
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-07-29_18,2022-07-28_02,2022-06-22_01
X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 7/28/22 10:49 PM, Zeng Jingxiang wrote:
> From: Zeng Jingxiang <linuszeng@tencent.com>
> 
> This greater-than-or-equal-to-zero comparison of an unsigned value
> is always true. "a >= U32_MIN".
> 1632	return a >= U32_MIN && a <= U32_MAX;
> 
> Fixes: b9979db83401 ("bpf: Fix propagation of bounds from 64-bit min/max into 32-bit and var_off.")
> Signed-off-by: Zeng Jingxiang <linuszeng@tencent.com>
> ---
>   kernel/bpf/verifier.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 0efbac0fd126..dd67108fb1d7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1629,7 +1629,7 @@ static bool __reg64_bound_s32(s64 a)
>   
>   static bool __reg64_bound_u32(u64 a)
>   {
> -	return a >= U32_MIN && a <= U32_MAX;
> +	return a <= U32_MAX;
>   }

I cannot find the related link. But IIRC, Alexei commented that
the code is written this way to express the intention (within
32bit bounds) so this patch is unnecessary...

>   
>   static void __reg_combine_64_into_32(struct bpf_reg_state *reg)