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: <7301b9cc-fdb1-cd4c-2dda-eb97018546a6@fb.com>
Date:   Fri, 20 May 2022 17:27:12 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.9.0
Subject: Re: [PATCH bpf-next 4/4] selftests/bpf: add reason of rejection in
 ld_imm64
Content-Language: en-US
To:     Shung-Hsi Yu <shung-hsi.yu@suse.com>,
        linux-kselftest@vger.kernel.org, netdev@vger.kernel.org,
        bpf@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Shuah Khan <shuah@kernel.org>, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>
References: <20220520113728.12708-1-shung-hsi.yu@suse.com>
 <20220520113728.12708-5-shung-hsi.yu@suse.com>
From:   Yonghong Song <yhs@fb.com>
In-Reply-To: <20220520113728.12708-5-shung-hsi.yu@suse.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?YzBKam4wTUZjQm1INkZxK2twaUVValB2aGk3S3dhN05RU0x2VzJjeG16UWh0?=
 =?utf-8?B?Zmw1ZjFrR09BTGwvYmpVVE8yTDZJMHhucmJ6SXpURFVDNVYwTm1vRmwxYUQx?=
 =?utf-8?B?bmhBaWxWM0hocUEvSDdzRVozeFpCYXBNTis0aVhmVXRHYm42WDZnSHF3VE90?=
 =?utf-8?B?S1N3VkduYXQ2NXJWaXRCelVJaVZ5NW1lSldUdzZPbUN3Qlg3VEtSWVJ0MzJt?=
 =?utf-8?B?Yi9BT1JhU1RLVnBxalFzWUllS3F4d0lyN3h6Z21Rd3VMTFA1SDhLN1F6OGls?=
 =?utf-8?B?a2dnNS9PdXBYcUk0dGNnNVhldEhjNlprNytxVmVYU0E3Z2lMbFRaVFcyOUhx?=
 =?utf-8?B?VXQ2d1ZvWDBoODlmZWdzazRtRXVKN0J3SUxPcFExMzl6TThYRDhrTkozVHdR?=
 =?utf-8?B?VDRNMm5UcEsyN3ljZDM5YlNVNDEvckJqUFNzZm1ja2ppb2FXZ1VjZExlZXBq?=
 =?utf-8?B?TmRnVjlqN3dBSVNLTXRoSnRNRXJvdGdCSGFkM1JlazJTOS9uMFlYSmRybUFL?=
 =?utf-8?B?Rm9sbW9rK1dxaGp5c0JwNmo5UjMzT2xSZ0kwNzhaRVQwdEZwdWJZQVZOa0Nl?=
 =?utf-8?B?TzNsdStMaGZMRnR3OXNQUlNjcDd3WDJscnhzUXdJMWNrb2toM2VNM0JhU3Ju?=
 =?utf-8?B?ZEQ3clp4SitiQ1ZQNnV6anRpVlJsd2xVRWNheU92ZG43bTI5MW5xMUttY2Qv?=
 =?utf-8?B?N1Z2R3Boc0IrbThtU050eE84ODY1UXZ5TnVWNmxYNkp4UDAwZE1CNkNmMU15?=
 =?utf-8?B?M3h0TjVaZVZXVWRkSTJTcUlQTkhnVzRLMHpvR1VhL2ZIQmFrV1hqUDRYODVS?=
 =?utf-8?B?VDBGMEx2dEdWTmo4ZWlyWUQreldDU1R0dDVycFFiOUpJQldsUkVQNkRwK0Z3?=
 =?utf-8?B?aWl6anpRVHNaQ1FDWTM2aGp0UXVydHl3M21DTjVTNTRTTHB1ZjJ6TEcrVjJ5?=
 =?utf-8?B?bkUybHpEZlFCSnJkc1pBbkRCV0xpSXJRVUlpbXhXaEFtRDRpOXp1b0FlanpG?=
 =?utf-8?B?Rm82eXJVdEZNM09EeDN4ZC9ydGo4NmhvdngxTnl0aWdwcU8ydjBmR0Y1Mll0?=
 =?utf-8?B?Ky9aekNQMy90Ujc1aG1WTHo2UnU2ZDRBSE53blp2MEhZQVZaQkhXNENkQ3da?=
 =?utf-8?B?VWJiU1l4d2JkejhyeFNUTUR0a3JZU2lyNEpCd1BHdWh6cFNZSnNRZHIwYzhq?=
 =?utf-8?B?RFBMMkJObUJkTGVjQmxrNmxtRkx1MFhOQTlYOHkyVkZQUndpak54dGFOSU4r?=
 =?utf-8?B?Nlgzd00vTC9OcTVodEZqbXloVVNubUU1emQ1elMxL3NWY2QxZEloSE5JWFNz?=
 =?utf-8?B?M2RuRXhFQkdYN3FXbVgwN1RTL2tHQVpkQzM3ZjJoZGVvUkp5NFBlWVkrOEVk?=
 =?utf-8?B?NDhHazV1U1VuZGN0UGFFMGZkQWJVenlieDdOT2YxMktIbzNEdUIrclVtUVFF?=
 =?utf-8?B?bytvLzR5UzlQRE5uTVBpZU1ZMFcyWTJyVlA3NjhBajNHOWNtR2lWT0llSlBL?=
 =?utf-8?B?Q2w0MG5oRDRCSmpGbUNmS3JtM2R4bFRQMnh3TjZ0U01xQVJZS3BwYUwvN0l2?=
 =?utf-8?B?THhncUtnK3A1ckdXTGo4dEtKN0lGWHo0MFVQOWw5WnVia0szbkhtbGszQTBr?=
 =?utf-8?B?L05vWTJ5RC9UQ0xrS01DQjFvbTFYZmZIWlpRa0FhS0M1NHBjbEQ1bFpLaXY4?=
 =?utf-8?B?YnVaZ1dzTkNRN0xCbkI0R0xYa0c3UzBkeWdKS2RGT2szMWx1V3dpdmg2eWpG?=
 =?utf-8?B?YXhpNnVTb2hlNlpBWjd5a0F0ZDZ0WlF5ckZNeWlFTm1XQ202OXZtZzk4S3Nw?=
 =?utf-8?B?azRSUkxtNThzN1NYYmpNN3RnQkN2S3YrVVBKM1FHVzVFbXVyOVlFbVVCSE5m?=
 =?utf-8?B?di9vckJ2a3pKM2NPZ2RNVm9tTXFYMzlDVkNRQlMvTWlOanJJNGMxMWp4TXdC?=
 =?utf-8?B?b2oyVHlUQ2xFZlI3cU40eTM2aCtNdmpWY1I2N3RCZlBYbmJIK2VFUWNuMWhW?=
 =?utf-8?B?TDBzNFBDZUNGRWxMQjRiMHYzUnJYMXRFRmhNQVYxS05JNm9RanpsVVpEODRr?=
 =?utf-8?B?bVB5WVl2NnV2dGxGSHB3REoveHpZSVNteW5oa29qc3hKZGJ0N09xSTNoL29z?=
 =?utf-8?B?eU5lbE4wZVhlLzIrdlRjZWhJajJ5K0RrNjcxMFE4L2c4ajFVQUNtVzBMdlhZ?=
 =?utf-8?B?dkJjb2d3aHg0YVFhS3dYTHRxMU51dzNvRlJ2bjUvdnRZTjhTNENhb2VVQjhC?=
 =?utf-8?B?MGNweWlERmM1UFc2NDZVZ0JpbFpDMzlxcmM3dVlOLy91TkFESHVhWnNXa1Fy?=
 =?utf-8?B?S2U0dS9sUS9qczZQQjhtNzAxYm4zMlFiTkkzOENmeHlFcldCYW9hS0lDZXVH?=
 =?utf-8?Q?IwIapbmtI47qe/ew=3D?=
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 00a5780e-a54c-43b6-ca8c-08da3ac0a80d
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 May 2022 00:27:15.5360
 (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: +/sLpPdzLtqJ4aL7Jvq4z71DIFST15eP/oWjWeShQpwNt70pjbElz16v6d3l2Kk3
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR15MB1898
X-Proofpoint-ORIG-GUID: EiDGoR0ygz14r71ybi9UXl4W0WLuni-Y
X-Proofpoint-GUID: EiDGoR0ygz14r71ybi9UXl4W0WLuni-Y
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.874,Hydra:6.0.486,FMLib:17.11.64.514
 definitions=2022-05-20_08,2022-05-20_02,2022-02-23_01
X-Spam-Status: No, score=-4.0 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_LOW,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE 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 5/20/22 4:37 AM, Shung-Hsi Yu wrote:
> It may not be immediately clear why that ld_imm64 test cases are
> rejected, especially for test1 and test2 where JMP to the 2nd
> instruction of BPF_LD_IMM64 is performed.
> 
> Add brief explaination of why each test case in verifier/ld_imm64.c
> should be rejected.
> 
> Signed-off-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
> ---
>   .../testing/selftests/bpf/verifier/ld_imm64.c | 20 ++++++++++---------
>   1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/verifier/ld_imm64.c b/tools/testing/selftests/bpf/verifier/ld_imm64.c
> index f9297900cea6..021312641aaf 100644
> --- a/tools/testing/selftests/bpf/verifier/ld_imm64.c
> +++ b/tools/testing/selftests/bpf/verifier/ld_imm64.c
> @@ -1,5 +1,6 @@
> +/* Note: BPF_LD_IMM64 is composed of two instructions of class BPF_LD */

> [...]LD | BPF_IMM | BPF_DW, 0, 0, 0, 0),
> @@ -42,7 +43,7 @@
>   	.result = REJECT,
>   },
>   {
> -	"test4 ld_imm64",
> +	"test4 ld_imm64: reject incomplete BPF_LD_IMM64 instruction",
>   	.insns = {
>   	BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 0),
>   	BPF_EXIT_INSN(),
> @@ -70,7 +71,7 @@
>   	.retval = 1,
>   },
>   {
> -	"test8 ld_imm64",
> +	"test8 ld_imm64: reject 1st off!=0",

Let add some space like 'off != 0'. The same for
some of later test names.

>   	.insns = {
>   	BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 1, 1),
>   	BPF_RAW_INSN(0, 0, 0, 0, 1),
> @@ -80,7 +81,7 @@
>   	.result = REJECT,
>   },
>   {
> -	"test9 ld_imm64",
> +	"test9 ld_imm64: reject 2nd off!=0",
>   	.insns = {
>   	BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 1),
>   	BPF_RAW_INSN(0, 0, 0, 1, 1),
> @@ -90,7 +91,7 @@
>   	.result = REJECT,
>   },
>   {
> -	"test10 ld_imm64",
> +	"test10 ld_imm64: reject 2nd dst_reg!=0",
>   	.insns = {
>   	BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 1),
>   	BPF_RAW_INSN(0, BPF_REG_1, 0, 0, 1),
> @@ -100,7 +101,7 @@
>   	.result = REJECT,
>   },
>   {
> -	"test11 ld_imm64",
> +	"test11 ld_imm64: reject 2nd src_reg!=0",
>   	.insns = {
>   	BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, 0, 0, 1),
>   	BPF_RAW_INSN(0, 0, BPF_REG_1, 0, 1),
> @@ -113,6 +114,7 @@
>   	"test12 ld_imm64",
>   	.insns = {
>   	BPF_MOV64_IMM(BPF_REG_1, 0),
> +	/* BPF_REG_1 is interpreted as BPF_PSEUDO_MAP_FD */
>   	BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, BPF_REG_1, 0, 1),
>   	BPF_RAW_INSN(0, 0, 0, 0, 0),
>   	BPF_EXIT_INSN(),
> @@ -121,7 +123,7 @@
>   	.result = REJECT,
>   },
>   {
> -	"test13 ld_imm64",
> +	"test13 ld_imm64: 2nd src_reg!=0",
>   	.insns = {
>   	BPF_MOV64_IMM(BPF_REG_1, 0),
>   	BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, BPF_REG_1, 0, 1),