Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 28 Nov 2019 12:33:42 -0500 (EST)
From:   Alan Stern <stern@rowland.harvard.edu>
To:     Oliver Neukum <oneukum@suse.com>
cc:     syzbot <syzbot+9ca7a12fd736d93e0232@syzkaller.appspotmail.com>,
        <andreyknvl@google.com>, <hverkuil@xs4all.nl>,
        <linux-kernel@vger.kernel.org>, <linux-media@vger.kernel.org>,
        <linux-usb@vger.kernel.org>, <mchehab@kernel.org>,
        <syzkaller-bugs@googlegroups.com>
Subject: Re: KASAN: use-after-free Read in si470x_int_in_callback (2)
In-Reply-To: <1574938260.21204.5.camel@suse.com>
Message-ID: <Pine.LNX.4.44L0.1911281225340.19734-100000@netrider.rowland.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 28 Nov 2019, Oliver Neukum wrote:

> Hi,
> 
> it is a bit awkward to complain here, as your patch tests as correct
> while mine didn't, but this is a race condition.
> You can't guarantee that urb->status != 0.
> The kill may happen while the completion handler is running for
> a successful transfer.
> 
> I really appreciate your help, but I must understand what is going
> wrong here. You are stopping the resubmit, but how could the resubmit
> ever have not failed?

You probably have figured all this out already, but I'll answer anyway.

The code I changed doesn't race with the error condition you were 
concerned about.  The two are independent, and it doesn't matter if the 
kill happens while the completion handler is running.

In the case my patch addresses, the resubmit does not fail.  Rather, it
succeeds but then the URB completes with a -EPROTO error.  The problem
is that the completion handler then resubmits the URB again, and it
completes again with the same error, right away, in a tight loop --
which prevents the CPU from executing any other code.  The way to fix
the problem is to break the loop by not resubmitting, which is what the
patch does.

Alan Stern