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;
Subject: Re: [syzbot] [hfs?] WARNING in hfs_write_inode
To:     Linus Torvalds <torvalds@linux-foundation.org>
References: <000000000000dbce4e05f170f289@google.com>
 <5f45bb9a-5e00-48dd-82b0-46b19b1b98a3@app.fastmail.com>
 <CAHk-=wi8XyAUF9_z6-oa4Ava6PVZeE-=TVNcFK1puQHpOtqLLw@mail.gmail.com>
 <ab7a9477-ddc7-430f-b4ee-c67251e879b0@app.fastmail.com>
 <1bd49fc0-d64f-4eb8-841a-4b09e178b5fd@gmail.com>
 <CAHk-=wg3U3Y6eaura=xQzTsktpEOMETYYnue+_KSbQmpg7vZ0Q@mail.gmail.com>
Cc:     Arnd Bergmann <arnd@arndb.de>,
        syzbot <syzbot+7bb7cd3595533513a9e7@syzkaller.appspotmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        christian.brauner@ubuntu.com,
        Damien Le Moal <damien.lemoal@opensource.wdc.com>,
        jlayton@kernel.org, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
        Matthew Wilcox <willy@infradead.org>,
        ZhangPeng <zhangpeng362@huawei.com>,
        Viacheslav Dubeyko <slava@dubeyko.com>,
        linux-m68k@lists.linux-m68k.org, flar@allandria.com
From:   Michael Schmitz <schmitzmic@gmail.com>
Message-ID: <1a3d07bf-16f5-71a8-6500-7d37802dbadd@gmail.com>
Date:   Fri, 6 Jan 2023 12:46:04 +1300
User-Agent: Mozilla/5.0 (X11; Linux ppc; rv:45.0) Gecko/20100101
 Icedove/45.4.0
MIME-Version: 1.0
In-Reply-To: <CAHk-=wg3U3Y6eaura=xQzTsktpEOMETYYnue+_KSbQmpg7vZ0Q@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Linus,

Am 06.01.2023 um 10:53 schrieb Linus Torvalds:
> On Thu, Jan 5, 2023 at 1:35 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
>>
>> Looking at Linus' patch, I wonder whether the missing fd.entrylength
>> size test in the HFS_IS_RSRC(inode) case was due to the fact that a
>> file's resource fork may be empty?
>
> But if that is the case, then the subsequent hfs_bnode_read would
> return garbage, no? And then writing it back after the update would be
> even worse.
>
> So adding that
>
> +               if (fd.entrylength < sizeof(struct hfs_cat_file))
> +                       goto out;
>
> would seem to be the right thing anyway. No?

Yes, it would seem to be the right thing (in order to avoid further 
corrupting HFS data structures). Returning -EIO might cause a regression 
though.

> But I really don't know the code, so this is all from just looking at
> it and going "that makes no sense". Maybe it _does_ make sense to
> people who have more background on it.

What had me wondering is that the 'panic?' comment was only present in 
the directory and regular file data cased but not in the resource fork 
case.

But I don't really understand the code too well either. I'll have to see 
for myself whether or not your patch does cause a regression on HFS 
filesystems such as the OF bootstrap partition used on PowerPC Macs.

Cheers,

	Michael


>
>              Linus
>