Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] vboxsf: fix old signature detection
To:     Arnd Bergmann <arnd@kernel.org>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
        Sparse Mailing-list <linux-sparse@vger.kernel.org>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Arnd Bergmann <arnd@arndb.de>,
        Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        llvm@lists.linux.dev
References: <20210927094123.576521-1-arnd@kernel.org>
 <40217483-1b8d-28ec-bbfc-8f979773b166@redhat.com>
 <20210927130253.GH2083@kadam>
 <CAK8P3a3YFh4QTC6dk6onsaKcqCM3Nmb2JhMXK5QdZpHtffjyLg@mail.gmail.com>
 <CAHk-=wheEHQxdSJgTkt7y4yFjzhWxMxE-p7dKLtQSBs4ceHLmw@mail.gmail.com>
 <70a77e44-c43a-f5ce-58d5-297ca2cfe5d9@redhat.com>
 <CAK8P3a3sEy7NAhMHcV7XPpZxo5tHnQz1oCP43YTe_ZQuzOHgPA@mail.gmail.com>
From:   Hans de Goede <hdegoede@redhat.com>
Message-ID: <42797736-a64b-e244-136a-d4526b732a50@redhat.com>
Date:   Tue, 28 Sep 2021 12:31:03 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <CAK8P3a3sEy7NAhMHcV7XPpZxo5tHnQz1oCP43YTe_ZQuzOHgPA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 9/28/21 12:11 PM, Arnd Bergmann wrote:
> On Tue, Sep 28, 2021 at 11:40 AM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 9/27/21 8:33 PM, Linus Torvalds wrote:
>>> On Mon, Sep 27, 2021 at 6:22 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>>>
>>>> More specifically, ' think '\377' may be either -1 or 255 depending on
>>>> the architecture.
>>>> On most architectures, 'char' is implicitly signed, but on some others
>>>> it is not.
>>>
>>> Yeah. That code is just broken.
>>>
>>> And Arnd, your patch may be "conceptually minimal", in that it keeps
>>> thed broken code and makes it work. But it just dials up the oddity to
>>> 11.
> 
> Thank you for addressing it. I usually try to avoid overthinking changes
> to "unusual" code like this, but your solution is clearly an improvement.
> 
> What really threw me off this time is that my first attempt to address
> the warning was an exact revert of 9d682ea6bcc7 ("vboxsf: Fix the
> check for the old binary mount-arguments struct"), which in turn
> came from a tool that is usually correct and and that both Dan
> and Al thought the original patch was correct when it looked like
> it turned a working (though unusual) implementation  into a broken
> one.
> 
>> I agree that your suggestion is to be the best solution,
>> so how do we move forward with this, do I turn this into a
>> proper patch with you as the author and Arnd as Reported-by and
>> if yes may I add your Signed-off-by to the patch ?
> 
> It's already upstream, see d5f6545934c4 ("qnx4: work around gcc
> false positive warning bug").

Ah, actually you mean: 9b3b353ef330 ("vboxfs: fix broken legacy mount
signature checking"), but other then that yes you're right it
is already upstream.

Thank you Arnd and thank you Linus.

Regards,

Hans