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: <e37c0a65-a3f6-e2e6-c2ad-367db20253a0@meta.com>
Date:   Tue, 16 May 2023 10:08:17 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.10.1
Subject: Re: [bug] kernel: bpf: syscall: a possible sleep-in-atomic bug in
 __bpf_prog_put()
Content-Language: en-US
To:     starmiku1207184332@gmail.com, ast@kernel.org, daniel@iogearbox.net,
        john.fastabend@gmail.com, andrii@kernel.org, martin.lau@linux.dev,
        song@kernel.org, yhs@fb.com, kpsingh@kernel.org, sdf@google.com,
        haoluo@google.com, jolsa@kernel.org, davem@davemloft.net,
        kuba@kernel.org, hawk@kernel.org
Cc:     bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org
References: <20230516111823.2103536-1-starmiku1207184332@gmail.com>
From:   Yonghong Song <yhs@meta.com>
In-Reply-To: <20230516111823.2103536-1-starmiku1207184332@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TFd0TEowMy9OY2o5R2E3cGZ3T0xiaG44dFdXQUJHTTRINEVVMnNLSWovY3B1?=
 =?utf-8?B?aHdzV3Q5MHBmeThZRjhYeW1NYW5OY05YNVRLRmFla0J3ZFl6QWRDNUc2VnBQ?=
 =?utf-8?B?NWVlVHh4S2gxS2RMQlR2VFNZZFdmMWZDOUNCaDJpTkN4QTJqYUN0ZWsxZjhG?=
 =?utf-8?B?UFRaMENtV2l2aStZNXNraVhaLzl0Wk5ndFBZcTkvRjFTSWJiVFFqRTl4QWNx?=
 =?utf-8?B?Y3RrcjhsRGdMY0NheS91Q2tlWXIrNDJzNFMzM3djaU1yRURCNkVacTZvL2Rs?=
 =?utf-8?B?NzJwRHVza0FDWWJWbU4wdGtiSWRGZUdGZDJBWHpaMnh3VkthUHlveFBDSDNN?=
 =?utf-8?B?MENSeW0xRXNrNC9iRURXME9VZlJOY1ZKa2JyVlBMUEsvV1BuNW84ZlNncFlw?=
 =?utf-8?B?bmJJckwwamVlNEFGei9ZNm5OcTBVNGk4WnhEYUZUR1dnVitJT2xCOTFDMnlp?=
 =?utf-8?B?R3E3SEFsSVNocXBtUzFWSXZtQ1NWd3h3TTZ1dmlmT0VKL1lrN241WkVYL1Y5?=
 =?utf-8?B?NXNpMWRjY3dNQ2tvcE1abDRCL1QzWEdrUExpT0M0ZWk4ZVBzRXBndTVISzVp?=
 =?utf-8?B?TGxHU2NsLzRNNjQxbVJDK2oyMnZGR1VNVVRCMy92eUR3M09vZno0L3M1Y2ZS?=
 =?utf-8?B?VXFYaHdRbXdwQWlhbXpOeG01czltQ0l6WXlnTUxFL2phYkprWGF3cU0zOFpt?=
 =?utf-8?B?WDZZbXdiWmhlaW1wM1hnem1rTzZTRHNDQzE4L1hSSVFWMUpZUWZZTjBDTyt1?=
 =?utf-8?B?WHpoT1lJNytWdkNrcmtKWmZNS29OS1pNMTZvQitlZDdsRmJTVHpqWUp4ZTJU?=
 =?utf-8?B?R2RFcmYzbVg2Y3BOenRxb2NOSEpDb2xKYmp4Y3FNbU5NVHlyb0xqMXg2TStF?=
 =?utf-8?B?VjZvMzVpRmVVbzRHa3Y0Y1FIbG1vVFFQVm9nQXR6dHFxaWQzQ0ZMMDZLYjU3?=
 =?utf-8?B?Uk1Wc2JlRXNpZVNVdC9NUTdyeXRuZ1hoVm93Umc0NHEvMU5zbTJlQkxseWRs?=
 =?utf-8?B?cm5vTUJvVUwveUJBd3FSVG11ZmhhWFdTZ0wzWE94YWhXUlh2elhFUjdoWDFC?=
 =?utf-8?B?aFozaEcwTzI2RUpsb2dXWUxKOVB1bkE5RVVuRm5wSXNWdG83eXM1RHFMZWVa?=
 =?utf-8?B?cmFjeEhFK0FLVThEZGYzbnBjMlpnM1JOQS9PMzN2NjMvbksxR21XMkI1MGVM?=
 =?utf-8?B?MG5iU0xrWlhHTFdiWVNhMVBsbmFmZUpMQ2UwWjdyZVNlVEM5emk2ZE45Y1NW?=
 =?utf-8?B?TlVBYmg0Rmw3MkRxckh6SXhJc2lFTk5sc1RNKzQ0S1VDVnFhZ2h3emhWbDdx?=
 =?utf-8?B?a3pta1JubmJ2L0l3TmZwSlRnT3RYRm45VW5VQTNyb0tNTFBiMXN1RDF4Nmlr?=
 =?utf-8?B?anNhZzBRMFA2UWxpQkdjbEd5cURranRQUXBiWkprcFMvRmllWVBMRUE2QWpY?=
 =?utf-8?B?ckZycHhSUXo4bWlSbVlXS080UlBhUXJrM1p6SXlmUjNYeDYwTXJaUUtwYith?=
 =?utf-8?B?NHNqSWlBOUdUdTdWcVhzYUxtZ0o1ZHROd3lvR2ZOc3Bmc0g5TDZYbGJUWENT?=
 =?utf-8?B?blp1RzI0S3hxNmdhMGZnSGZ2VUxNM3I5TDY0REw3UmgvYXRpS0lDYVVtd0RF?=
 =?utf-8?B?azhLSVVpalloQUcrNUpkQjA4bTM1SGZiOHhYM0o2UUovYlBUUGxsWEVDM0Fn?=
 =?utf-8?B?cnlOMmNnYzlPTXpHbnRINDhsVmRkUlU4ZnEvdFR4UFZBd0lHYzNnQTFsSDhl?=
 =?utf-8?B?aDFkNEtPRFMyWVdlemlyMlMvUWluN2ZYWkZCWmMrZEtxTU1ramgwVkJGWVY2?=
 =?utf-8?B?c1pvc3BFVTVUNmNhVGsya0tnczByaTZ1dW5jSDc5QzFITDFOaCtiWVZ4SFVo?=
 =?utf-8?B?N3NPZkVyZVg4eGd5akVXbXdNbkZSQ2dSWEcraDdLcnNkRklWZndLWnFYdEp0?=
 =?utf-8?B?NGlSVkx3M1lzbVV5S0ZXOXhHOUVORnJucW1KbE1qblBFUFBhSzRJTE9mWmRq?=
 =?utf-8?B?MFR5a21IMUpvTzF4WDF2K2dKb0tBemptYWlUNHh4TWxWR1l4NVNMMTFoVTFO?=
 =?utf-8?B?M2xPNThaTmcxRG43OFFOL2RIUG96VDRHWE1XNFpKbVUzdDZCTG1tblZTK0tL?=
 =?utf-8?B?Um9SVXBaTXpLSnBkRzBDblZUbnhNemEvTEo0QXNGdnpEWG85eTc4ZzhWWE9z?=
 =?utf-8?B?QlE9PQ==?=
X-OriginatorOrg: meta.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 192cc242-ae36-4161-8bc2-08db5630269c
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 May 2023 17:08:21.0945
 (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: WtMtdn5Apz+vBTe2TtX0IIeLhT+BZmmJlbCx76oBICMnszyS6D2ttDVYB5wwyv8a
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR15MB6292
X-Proofpoint-GUID: j2UZKLFRAqC4h16w1r5hrYeosq9Qkv6c
X-Proofpoint-ORIG-GUID: j2UZKLFRAqC4h16w1r5hrYeosq9Qkv6c
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-05-16_09,2023-05-16_01,2023-02-09_01
X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00,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_PASS,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED 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/16/23 4:18 AM, starmiku1207184332@gmail.com wrote:
> From: Teng Qi <starmiku1207184332@gmail.com>
> 
> Hi, bpf developers,
> 
> We are developing a static tool to check the matching between helpers and the
> context of hooks. During our analysis, we have discovered some important
> findings that we would like to report.
> 
> ‘kernel/bpf/syscall.c: 2097 __bpf_prog_put()’ shows that function
> bpf_prog_put_deferred() won`t be called in the condition of
> ‘in_irq() || irqs_disabled()’.
> if (in_irq() || irqs_disabled()) {
>      INIT_WORK(&aux->work, bpf_prog_put_deferred);
>      schedule_work(&aux->work);
> } else {
> 
>      bpf_prog_put_deferred(&aux->work);
> }
> 
> We suspect this condition exists because there might be sleepable operations
> in the callees of the bpf_prog_put_deferred() function:
> kernel/bpf/syscall.c: 2097 __bpf_prog_put()
> kernel/bpf/syscall.c: 2084 bpf_prog_put_deferred()
> kernel/bpf/syscall.c: 2063 __bpf_prog_put_noref()
> kvfree(prog->aux->jited_linfo);
> kvfree(prog->aux->linfo);

Looks like you only have suspicion here. Could you find a real violation 
here where __bpf_prog_put() is called with !in_irq() && 
!irqs_disabled(), but inside spin_lock or rcu read lock? I have not seen
things like that.

> 
> Additionally, we found that array prog->aux->jited_linfo is initialized in
> ‘kernel/bpf/core.c: 157 bpf_prog_alloc_jited_linfo()’:
> prog->aux->jited_linfo = kvcalloc(prog->aux->nr_linfo,
>    sizeof(*prog->aux->jited_linfo), bpf_memcg_flags(GFP_KERNEL | __GFP_NOWARN));

Any problem here?

> 
> Our question is whether the condition 'in_irq() || irqs_disabled() == false' is
> sufficient for calling 'kvfree'. We are aware that calling 'kvfree' within the
> context of a spin lock or an RCU lock is unsafe.
> 
> Therefore, we propose modifying the condition to include in_atomic(). Could we
> update the condition as follows: "in_irq() || irqs_disabled() || in_atomic()"?
> 
> Thank you! We look forward to your feedback.
> 
> Signed-off-by: Teng Qi <starmiku1207184332@gmail.com>