Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Date:   Wed, 1 Nov 2023 16:42:10 +0800
From:   Shung-Hsi Yu <shung-hsi.yu@suse.com>
To:     Hao Sun <sunhao.th@gmail.com>
Cc:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        John Fastabend <john.fastabend@gmail.com>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>,
        Yonghong Song <yonghong.song@linux.dev>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
        Mykola Lysenko <mykolal@fb.com>, Shuah Khan <shuah@kernel.org>,
        Eduard Zingerman <eddyz87@gmail.com>, bpf@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        stable@vger.kernel.org
Subject: Re: [PATCH bpf v2 1/2] bpf: Fix check_stack_write_fixed_off() to
 correctly spill imm
Message-ID: <ZUIPYkuuKe6sEv66@u94a>
References: <20231101-fix-check-stack-write-v2-0-cb7c17b869b0@gmail.com>
 <20231101-fix-check-stack-write-v2-1-cb7c17b869b0@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20231101-fix-check-stack-write-v2-1-cb7c17b869b0@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SU1NQ2F6aTc3bFh5bTltcDRBUnVxZHNkNkhxWUxhOEJlcE0yQ0oybXpNekti?=
 =?utf-8?B?TUhQeVQvbm8zSDBFOWlaRmZIMjFvbXdRc0tmZ0VQOE5sT1hPbFZFampaaDVH?=
 =?utf-8?B?MUlNeW1tdDFnWHFVSWNNcklmd3h5eTZrTW5zNHZpb0IzQjRvTE1PQTFWQ1pm?=
 =?utf-8?B?UGhaNmJ0V0RBRkZKN21penpyKzNTSmJkVmVJNHJqUFpjcE5VMU5PT0FNY3RV?=
 =?utf-8?B?S2IzZHVSaEJyQ3BhVFd6R3A4QitvNkRPVnFVdXc3SlJPSWdpcVJZczZlTjVX?=
 =?utf-8?B?VXk5UlFSc2lPS2RkN1N6S2pIZ3owZ0ZjQVo5UmZEd2tBc2ZQZ0p2TmJ6OGJW?=
 =?utf-8?B?a3dkOWZaSFJ2a091K1hndFVoWDdCVWM5VTVFdEg3L1RFQW8yTWxyZGk2c0R6?=
 =?utf-8?B?QlI1ZnBNL1VBTndaZlJyOWRNMFZ2NHRXbW5kMGlTY3I1NWh5UXVFQldOd0ZP?=
 =?utf-8?B?WXVRaHVodWtVOTBYWWhCem40OHhlM252V0xRNTkvc3hPOExQWlRxWk1UQzY5?=
 =?utf-8?B?cXVDbmlmRnZaZmVmbUt0R0kwUVBsRkloRmNHbXcxSTlscDRBMFpDMk5uNEx4?=
 =?utf-8?B?amF3TGtFTi82NjYxRmh6WHpTeVZmSzIwYUhsUDRUUmRuM2gvQ1BWU3JVaHFo?=
 =?utf-8?B?Z0ozdFJLMEg5NDA4UHVwTnhxV3VoWXFkVDNBTVYvVmlKeHE0MG53cDR0UEFx?=
 =?utf-8?B?aE5qRkgzUER6bVN5MERPSWJ1SjdRLzBvYVNwR25MczR1RWs1NWNvdmJ5U0lM?=
 =?utf-8?B?WEF6NnBmaTRZWWpDNmFKdjFrRWtnZHN0b0dDUGUrMVRBUVBLLzd5dEk5by9W?=
 =?utf-8?B?bG43UEpJWVh4ZU9HK0dLYU9kTDNudDVYL0dzaGQ4anEyNlJmbyt1Ui84YUdG?=
 =?utf-8?B?bjUvMFlyS3J6bldZaVlQSzVlektLeWlUTnlYRzBhYjZ3NTNjMnRJWTkzOUN3?=
 =?utf-8?B?QU1Rc1o4VlFTOGpsNnVjdWJjLzZ5Q2N0V2V2azlnSmlPZzRNN05RbUsydTZj?=
 =?utf-8?B?VzdsNWgwSTgrSnhNVnFoMDZPM05sL3RLajluWEM5Vm5jLytiWEYzTUVBdHQ5?=
 =?utf-8?B?OE5adlBBZzlWZk1iWVZ4R1BoL29GbmRqeVFqUEVXc3dDU09neTEyQ0NaNStO?=
 =?utf-8?B?ZTM2NnFIZWFBLzBMdFcySXMwY1VFYmsxNXF6SStYRGRoSXJnVVYyQWY4Ti9U?=
 =?utf-8?B?R294N2wza2xDZGMvK1hEUU1ZODB4MnZJeEJmLzhOdmlScDE4encvbCtrbkdJ?=
 =?utf-8?B?Vnd5dGNQYlRlZnlFUmd5d0ZBTkkzT3l3TEl0aUY3dkRBQWE0dnZMNXRVemM3?=
 =?utf-8?B?TUFDaHBNaExqTEtuOWFKNkNpRnR6dy9LcXpLZjU5Q2paMUMxWjZaYmQxYStN?=
 =?utf-8?B?d25jSFFxY2hpakd4dGNQcm51SDNDOGcyZS9ERTByUWZkejhibjNObHJvT1Va?=
 =?utf-8?B?cjFWRktSa3l4VGx0VHpUS2pOVFR0VHloaGI1ZnZhTXdnQVpBenlGMVJNUmFG?=
 =?utf-8?B?MG1zNnlLamtyWHowUXh3QmV4UnFJYUh6UDZvbUwvbHNnSk9vck9nQ096Z2ZJ?=
 =?utf-8?B?aUlSNmJXZFJGQUNXUTFvdnE5bkxxSzJnVk1uZUtGam5sdTk1c2tyK2hxa2VN?=
 =?utf-8?B?TmNSRXhVeGJIM3RUWU5jUTZCeE5rRjJrOXVwQUlrRXBzTHBIZ3hCNGRNVHF1?=
 =?utf-8?B?bTFJTlRncGtHRnhQQyttbnFYM1N3U0hOV2JyOE1sUUFiTllKR1YrMmhkV3Z6?=
 =?utf-8?B?WDVueHFyRjBPbDlHZnkrWUFWMktSY0F0aG5ESVBYdzY3KytNSjhScTFxbUgx?=
 =?utf-8?B?YlVOU3hueDN0ZmRPYmsybW0rZkVueTdmMzBBWStWK2o5WXJjb0w4V0RKdkpj?=
 =?utf-8?B?UFdHQXNTVHc0dnNSSUdUYjhFc3J0WG1UQTQ3Z04velk2eWFqcytiT3MvUlJY?=
 =?utf-8?B?YnlLU0pEVjJYRENJOXpRUi9jQ2JQemN5aTVuY3BDeXJkMGxsdnFYWVpIZGlS?=
 =?utf-8?B?SE9kL3JHbWlPRDJJNGpXbG5pMEZYVnlNTkEvNENoWDFEVDhNWjk1UFhBbVY0?=
 =?utf-8?B?eWllVWQwSlJOYUxNNDBNV2RkZDl5bUkyTTlTN0dIcDRGN1dVZG5FTEtWTDVl?=
 =?utf-8?B?bGxpYXhrSlJaajAzTXBrTDh6ckJ3NFplVW4rM1hNWTNFdk1RT2lZeUJldHBn?=
 =?utf-8?B?Nnc9PQ==?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a3b37faa-0e76-44a0-6c74-08dbdab67a86
X-MS-Exchange-CrossTenant-AuthSource: AS8PR04MB9510.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Nov 2023 08:42:27.8555
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: jz3a8WpR409+niVHZUFxg9BPOylZgNaRuSqal5SO1bcAfIBsRjzHEhFdV7o6bxPV2ug3g+OjbJfpljABksDz8g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB8258
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE,T_SPF_TEMPERROR,URIBL_BLOCKED
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Wed, 01 Nov 2023 01:43:00 -0700 (PDT)

On Wed, Nov 01, 2023 at 08:33:22AM +0100, Hao Sun wrote:
> In check_stack_write_fixed_off(), imm value is cast to u32 before being
> spilled to the stack. Therefore, the sign information is lost, and the
> range information is incorrect when load from the stack again.
> 
> For the following prog:
> 0: r2 = r10
> 1: *(u64*)(r2 -40) = -44
> 2: r0 = *(u64*)(r2 - 40)
> 3: if r0 s<= 0xa goto +2
> 4: r0 = 1
> 5: exit
> 6: r0  = 0
> 7: exit
> 
> The verifier gives:
> func#0 @0
> 0: R1=ctx(off=0,imm=0) R10=fp0
> 0: (bf) r2 = r10                      ; R2_w=fp0 R10=fp0
> 1: (7a) *(u64 *)(r2 -40) = -44        ; R2_w=fp0 fp-40_w=4294967252
> 2: (79) r0 = *(u64 *)(r2 -40)         ; R0_w=4294967252 R2_w=fp0
> fp-40_w=4294967252
> 3: (c5) if r0 s< 0xa goto pc+2
> mark_precise: frame0: last_idx 3 first_idx 0 subseq_idx -1
> mark_precise: frame0: regs=r0 stack= before 2: (79) r0 = *(u64 *)(r2 -40)
> 3: R0_w=4294967252
> 4: (b7) r0 = 1                        ; R0_w=1
> 5: (95) exit
> verification time 7971 usec
> stack depth 40
> processed 6 insns (limit 1000000) max_states_per_insn 0 total_states 0
> peak_states 0 mark_read 0
> 
> So remove the incorrect cast, since imm field is declared as s32, and
> __mark_reg_known() takes u64, so imm would be correctly sign extended
> by compiler.
> 
> Fixes: ecdf985d7615 ("bpf: track immediate values written to stack by BPF_ST instruction")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hao Sun <sunhao.th@gmail.com>

Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>