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@xxxxxxxxxxxxxxxxxx> 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.
