From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 700EDC43334 for ; Thu, 30 Jun 2022 14:44:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235181AbiF3Oop (ORCPT ); Thu, 30 Jun 2022 10:44:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53288 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234685AbiF3Oom (ORCPT ); Thu, 30 Jun 2022 10:44:42 -0400 Received: from mail-yw1-x112b.google.com (mail-yw1-x112b.google.com [IPv6:2607:f8b0:4864:20::112b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4F2831AF25 for ; Thu, 30 Jun 2022 07:44:41 -0700 (PDT) Received: by mail-yw1-x112b.google.com with SMTP id 00721157ae682-31bf327d4b5so91990847b3.13 for ; Thu, 30 Jun 2022 07:44:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fSGCf3INUeQnbAR4csX9+UIfMipKtpmqBXRf98s34f8=; b=coTWYclPD0QG+vO946f3aUVHNcKK40/FozAR7d7FMaU9d0S9CaVHSPcYb/YSL6wwLB LAMpeUzcB3kZHZlMEksl0lgUprkcJXK/HYpczYvTtKe9ivw3GlnUeTKg4tueLvPdd2Ys +ZKdkU/8i2x24qgQtGpo7N9MCBLCTz9agXxmLHk1aw4Q3KPvPdqjUB7mUOqc8iEovn4s eETT2QXBXm3YqIXn3owvzSxk1KxHKV7SV08dpyOEggDyOGgjGG1iB3vVOMMnKRJ1QJcA 0fmiHj34szI3fIPy0Omf95J1D13wlFs+jxjffMRhG4pqOUGL0/Y5QBlSwOfYdaWGg0c8 5WyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fSGCf3INUeQnbAR4csX9+UIfMipKtpmqBXRf98s34f8=; b=hqCv9bFbRUm62jjnXHE99JVsi/83qDNPdNtOHDOqq/vvz4aT2zrJfckqRSov5rh2M/ 63hy9FOOM8yNezsFm483kn+maI053oxwUfvemy5GFkssxedQvNRqmVlwMtZsGlwUBcyH YeuopTe5xaELJBGsHq55qVXVxK/BctnYHDRCEc8pjxJi0DWvXjL8jUd3BdeVInhNg3Dl X6+lDG0iEaem9LkKzSkHbfL1bsZN2PtkBp4tedd9FUqLquyKQ18ixKjkI2/WkLCD/MPS +3AllTy1CrG64P75xpN34bQOmw5WTd5h8lUfb/duvg/RTKP/H6/7aum2FrBF4DXhHmCz 5hdQ== X-Gm-Message-State: AJIora/cDBcGtRE8SC/HbZExhlTOcxfLNxSMdc5Y51yMXvYe23a4bPnA UmVJ6fyxoLjBxzzYTX3obawWZG7gBUymb+jMxZ3i1A== X-Google-Smtp-Source: AGRyM1t880VoNLUhAwj4c99+ctmx94kBLtQF9aIQgPWyusdKcd0DtDU3GbomNulLgbn12p0J90POkb/NfLdhH6Zram0= X-Received: by 2002:a81:5c9:0:b0:317:b1a5:bf8b with SMTP id 192-20020a8105c9000000b00317b1a5bf8bmr10691972ywf.489.1656600280296; Thu, 30 Jun 2022 07:44:40 -0700 (PDT) MIME-Version: 1.0 References: <20220630143842.24906-1-duoming@zju.edu.cn> In-Reply-To: <20220630143842.24906-1-duoming@zju.edu.cn> From: Eric Dumazet Date: Thu, 30 Jun 2022 16:44:29 +0200 Message-ID: Subject: Re: [PATCH net] net: rose: fix UAF bug caused by rose_t0timer_expiry To: Duoming Zhou Cc: linux-hams@vger.kernel.org, netdev , LKML , Paolo Abeni , Jakub Kicinski , David Miller , Ralf Baechle Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 30, 2022 at 4:38 PM Duoming Zhou wrote: > > There are UAF bugs caused by rose_t0timer_expiry(). The > root cause is that del_timer() could not stop the timer > handler that is running and there is no synchronization. > One of the race conditions is shown below: > > (thread 1) | (thread 2) > | rose_device_event > | rose_rt_device_down > | rose_remove_neigh > rose_t0timer_expiry | rose_stop_t0timer(rose_neigh) > ... | del_timer(&neigh->t0timer) > | kfree(rose_neigh) //[1]FREE > neigh->dce_mode //[2]USE | > > The rose_neigh is deallocated in position [1] and use in > position [2]. > > The crash trace triggered by POC is like below: > > BUG: KASAN: use-after-free in expire_timers+0x144/0x320 > Write of size 8 at addr ffff888009b19658 by task swapper/0/0 > ... > Call Trace: > > dump_stack_lvl+0xbf/0xee > print_address_description+0x7b/0x440 > print_report+0x101/0x230 > ? expire_timers+0x144/0x320 > kasan_report+0xed/0x120 > ? expire_timers+0x144/0x320 > expire_timers+0x144/0x320 > __run_timers+0x3ff/0x4d0 > run_timer_softirq+0x41/0x80 > __do_softirq+0x233/0x544 > ... > > This patch changes del_timer() in rose_stop_t0timer() and > rose_stop_ftimer() to del_timer_sync() in order that the > timer handler could be finished before the resources such as > rose_neigh and so on are deallocated. As a result, the UAF > bugs could be mitigated. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Duoming Zhou > --- > net/rose/rose_link.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/rose/rose_link.c b/net/rose/rose_link.c > index 8b96a56d3a4..9734d1264de 100644 > --- a/net/rose/rose_link.c > +++ b/net/rose/rose_link.c > @@ -54,12 +54,12 @@ static void rose_start_t0timer(struct rose_neigh *neigh) > > void rose_stop_ftimer(struct rose_neigh *neigh) > { > - del_timer(&neigh->ftimer); > + del_timer_sync(&neigh->ftimer); > } Are you sure this is safe ? del_timer_sync() could hang if the caller holds a lock that the timer function would need to acquire. > > void rose_stop_t0timer(struct rose_neigh *neigh) > { > - del_timer(&neigh->t0timer); > + del_timer_sync(&neigh->t0timer); > } Same here, please explain why it is safe. > > int rose_ftimer_running(struct rose_neigh *neigh) > -- > 2.17.1 >