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 1B84EC433F5 for ; Wed, 23 Feb 2022 19:06:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244122AbiBWTG6 (ORCPT ); Wed, 23 Feb 2022 14:06:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48576 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232675AbiBWTGz (ORCPT ); Wed, 23 Feb 2022 14:06:55 -0500 Received: from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com [IPv6:2a00:1450:4864:20::12e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4983A3EF1D for ; Wed, 23 Feb 2022 11:06:26 -0800 (PST) Received: by mail-lf1-x12e.google.com with SMTP id w27so15128298lfa.5 for ; Wed, 23 Feb 2022 11:06:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=mL0nR+ziEzThvzLITlZK1PYh9ofonS5DJNnVhGA9cEk=; b=ZxfU9J7Sv2Cr5l8MpDBHBUZBrVW4RtmdXoajMkfvwL5M7gMkck9lVPP17E6o9PJ3/z ETsUPxqKeXn2I8IImb0calaYlReEIGEV8iHd0/zOMfzCOmOYGI7/nX2ky2VlME1H418/ TnIcjdzbSuOjm67cwnrFkZVsdsAAO/6HIThdo= 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=mL0nR+ziEzThvzLITlZK1PYh9ofonS5DJNnVhGA9cEk=; b=r7lGxYVwD1xuGybT5aFWZhEOr3dkN5GOrtTfztRMPqDxAMNhChEUWsW5WF2v5cLN0Z Dg+3WVYKFw2aJ9vr7GZCz5WN+spBAzRuKCQU4RtM8aDe5tlxm3UzgoWUkSM6KxEAKHvD YrhN5jrLbrxlEEanQtNu4+D9gAzuswet3BHRJxUfQUo6h/BCgM4fPG4N9rPp8cTLBcgo 4VE4XP7iTLnmTlkD5b7lYgIlajq6b7hjTmqLRICJ02QSc8tojQrRpuC9rEaTFDQkOMEb 2PUmwYH5fWnMAp22If/VZo1KnwW85oFG6lWCiK12wZajYsTXDHDMRvvGbaroqAjHuLGJ pi9Q== X-Gm-Message-State: AOAM531UB0qLEql4pihuzsAqShv3uy4L8R5/bbT2iZ38b56JifuP5WiM D59Kc626AF041YHBNlBuIw2H7Q78iOJ7c8c2424= X-Google-Smtp-Source: ABdhPJz/xhH1EZyPY2/Bhae163Bl7DMER7+k/Fx3ezF2gj2ozgvKrGJXRK3VgR98FV7T3j1qUaIIYQ== X-Received: by 2002:a05:6512:32a6:b0:443:3d93:6f38 with SMTP id q6-20020a05651232a600b004433d936f38mr680901lfe.162.1645643184227; Wed, 23 Feb 2022 11:06:24 -0800 (PST) Received: from mail-lj1-f170.google.com (mail-lj1-f170.google.com. [209.85.208.170]) by smtp.gmail.com with ESMTPSA id z4sm29228lfu.235.2022.02.23.11.06.19 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 23 Feb 2022 11:06:21 -0800 (PST) Received: by mail-lj1-f170.google.com with SMTP id r20so26129318ljj.1 for ; Wed, 23 Feb 2022 11:06:19 -0800 (PST) X-Received: by 2002:a2e:80c6:0:b0:246:3334:9778 with SMTP id r6-20020a2e80c6000000b0024633349778mr543364ljg.443.1645643179121; Wed, 23 Feb 2022 11:06:19 -0800 (PST) MIME-Version: 1.0 References: <20220217184829.1991035-1-jakobkoschel@gmail.com> <20220217184829.1991035-5-jakobkoschel@gmail.com> <20220218151216.GE1037534@ziepe.ca> <6BA40980-554F-45E2-914D-5E4CD02FF21C@gmail.com> In-Reply-To: <6BA40980-554F-45E2-914D-5E4CD02FF21C@gmail.com> From: Linus Torvalds Date: Wed, 23 Feb 2022 11:06:03 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC PATCH 04/13] vfio/mdev: remove the usage of the list iterator after the loop To: Jakob Cc: Jason Gunthorpe , Linux Kernel Mailing List , linux-arch , Greg Kroah-Hartman , Thomas Gleixner , Arnd Bergman , Andy Shevchenko , Andrew Morton , Kees Cook , Mike Rapoport , "Gustavo A. R. Silva" , Brian Johannesmeyer , Cristiano Giuffrida , "Bos, H.J." Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Feb 23, 2022 at 6:18 AM Jakob wrote: > > However, in this example, 'tmp' will be a out-of-bounds pointer > if the loop did finish without hitting the break, so the check past the > loop *could* match 'mdev' even though no break was ever met. So just as context for others, since I was hit with the same confusion and didn't see what the relevance was for type speculation, when these patches seemed to be not about speculation at all. The background for this is that the list_for_each_entry() will set the iterator variable (here 'tmp') to be not the actual internal list pointer, but the pointer to the containing type (which is the whole 'entry' part of the name, of course). And that is all good and true, but it's true only *WITHIN* that loop. At the exit condition, it will have hit the 'head' of the list, and the type that contains the list head is *not* necessarily the same type as the list entries. So that's where the type confusion comes from: if you access the list iterator outside the loop, and it could have fallen off the end of the loop, the list iterator pointer is now not actually really a valid pointer of that 'entry' type at all. And as such, you not only can't dereference it, but you also shouldn't even compare pointer values - because the pointer arithmetic that was valid for loop entries is not valid for the HEAD entry that is embedded in another type. So the pointer arithmetic might have turned it into a pointer outside the real container of the HEAD, and might actually match something else. Now, there are a number of reasons I really dislike the current RFC patch series, so I'm not claiming the patch is something we should apply as-is, but I'm trying to clarify why Jakob is doing what he's doing (because clearly I wasn't the only one taken by surprise by it). The reasons I don't like it is: - patches like these are very random. And very hard to read (and very easy to re-introduce the bug). - I think it conflates the non-speculative "use pointer of the wrong type" issue like in this patch with the speculation - I'm not even convinced that 'list_for_each_entry()' is that special wrt speculative type accesses, considering that we have other uses of doubly linked list *everywhere* - and it can happen in a lot of other situations anyway, so it all seems to be a bit ad hoc. but I do think the problem is real. So elsewhere I suggested that the fix to "you can't use the pointer outside the loop" be made to literally disallow it (using C99 for-loop variables seems the cleanest model), and have the compiler refuse to touch code that tries to use the loop iterator outside. And that is then entirely separate from the issue of actual speculative accesses (but honestly, I think that's a "you have to teach the compiler not to do them" issue, not a "let's randomly change *one* of our loop walkers). Linus