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: <ba70b229-317b-9c2d-f3bb-5450eec85694@I-love.SAKURA.ne.jp>
Date:   Fri, 19 May 2023 07:19:57 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [syzbot] [rdma?] KASAN: slab-use-after-free Read in
 siw_query_port
Content-Language: en-US
To:     Fedor Pchelkin <pchelkin@ispras.ru>,
        Guoqing Jiang <guoqing.jiang@linux.dev>
Cc:     syzbot <syzbot+79f283f1f4ccc6e8b624@syzkaller.appspotmail.com>,
        bmt@zurich.ibm.com, jgg@ziepe.ca, leon@kernel.org,
        linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
        syzkaller-bugs@googlegroups.com,
        Alexey Khoroshilov <khoroshilov@ispras.ru>,
        lvc-project@linuxtesting.org
References: <0000000000001f992805fb79ce97@google.com>
 <5eacf66d-053e-d82b-1e73-c808fb4c8aad@linux.dev>
 <20230518202116.rpx53vp7rrtuixoa@fpc>
From:   Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
In-Reply-To: <20230518202116.rpx53vp7rrtuixoa@fpc>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2023/05/19 5:21, Fedor Pchelkin wrote:
> On our local Syzkaller instance the bug started to be caught after
> 266e9b3475ba ("RDMA/siw: Remove namespace check from siw_netdev_event()")
> so CC'ing Tetsuo Handa if maybe he would be also interested in the bug.

UAF could not be observed until that commit because hung task was observed
until that commit because syzkaller is testing non init_net namespace.

> This fix seems to be good and perhaps it just made a bigger opportunity
> for the UAF bug to happen. Actually, the C repro was taken from there [2].
> 
> With your suggested solution the UAF is not reproduced. I don't know the
> exact reasons why dev_put() was placed before calling query_port() but the
> context implies that netdev can be freed in that period. And some of
> ->query_port() realizations may touch netdev. So it seems reasonable to
> move ref count put after performing query_port().

Since ib_device_get_netdev() calls dev_hold() on success, I think that
we need to call dev_put() after query_port(). Please send as a formal patch.