[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <af24da42-02cb-e60e-d1df-365801aa686b@I-love.SAKURA.ne.jp>
Date: Mon, 31 Oct 2022 20:28:27 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Ian Kent <raven@...maw.net>, Hawkins Jiawei <yin31149@...il.com>,
viro@...iv.linux.org.uk
Cc: 18801353760@....com, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
cmaiolino@...hat.com, dhowells@...hat.com, hughd@...gle.com,
miklos@...redi.hu, oliver.sang@...el.com, siddhesh@...plt.org,
syzbot+db1d2ea936378be0e4ea@...kaller.appspotmail.com,
syzkaller-bugs@...glegroups.com, tytso@....edu, smfrench@...il.com,
pc@....nz, lsahlber@...hat.com, sprasad@...rosoft.com,
tom@...pey.com
Subject: Re: [PATCH -next 0/5] fs: fix possible null-ptr-deref when parsing
param
On 2022/10/24 12:34, Ian Kent wrote:
>
> On 24/10/22 08:42, Hawkins Jiawei wrote:
>> On Mon, 24 Oct 2022 at 00:48, Al Viro <viro@...iv.linux.org.uk> wrote:
>>> On Mon, Oct 24, 2022 at 12:39:41AM +0800, Hawkins Jiawei wrote:
>>>> According to commit "vfs: parse: deal with zero length string value",
>>>> kernel will set the param->string to null pointer in vfs_parse_fs_string()
>>>> if fs string has zero length.
>>>>
>>>> Yet the problem is that, when fs parses its mount parameters, it will
>>>> dereferences the param->string, without checking whether it is a
>>>> null pointer, which may trigger a null-ptr-deref bug.
>>>>
>>>> So this patchset reviews all functions for fs to parse parameters,
>>>> by using `git grep -n "\.parse_param" fs/*`, and adds sanity check
>>>> on param->string if its function will dereference param->string
>>>> without check.
>>> How about reverting the commit in question instead? Or dropping it
>>> from patch series, depending upon the way akpm handles the pile
>>> these days...
>> I think both are OK.
>>
>> On one hand, commit "vfs: parse: deal with zero length string value"
>> seems just want to make output more informattive, which probably is not
>> the one which must be applied immediately to fix the
>> panic.
>>
>> On the other hand, commit "vfs: parse: deal with zero length string value"
>> affects so many file systems, so there are probably some deeper
>> null-ptr-deref bugs I ignore, which may take time to review.
>
> Yeah, it would be good to make the file system handling consistent
> but I think there's been a bit too much breakage and it appears not
> everyone thinks the approach is the right way to do it.
>
> I'm thinking of abandoning this and restricting it to the "source"
> parameter only to solve the user space mount table parser problem but
> still doing it in the mount context code to keep it general (at least
> for this case).
No progress on this problem, and syzbot is reporting one after the other...
I think that reverting is the better choice.
Powered by blists - more mailing lists