linux-kernel - Re: [PATCH v1] sysfs: fix sysfs_kf_seq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YqjXRMg2HZyk7+Cl@google.com>
Date:   Tue, 14 Jun 2022 18:45:24 +0000
From:   William McVicker <willmcvicker@...gle.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>, Tejun Heo <tj@...nel.org>,
        kernel-team@...roid.com, Christoph Hellwig <hch@....de>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] sysfs: fix sysfs_kf_seq_show null pointer dereference

On 06/14/2022, Greg Kroah-Hartman wrote:
> On Tue, Jun 14, 2022 at 05:24:01PM +0000, Will McVicker wrote:
> > When the kobj->ktype is null,
> 
> How can that happen?  What in-tree code does that?

This kernel panic happens randomly for me. The call trace shows that this
happens when the read syscall is invoked by Android. GDB gave me this line when
disassembling __arm64_sys_read+0x20/0x30:

fs/read_write.c:628
SYSCALL_DEFINE3(read, unsigned int, fd, char __user *, buf, size_t, count)

> 
> > sysfs_file_ops() returns a NULL pointer
> > for the sysfs_ops. When this happens, we hit a kernel panic in
> > sysfs_kf_seq_show() by dereferencing ops to check if ->show is NULL.
> > Based on commit 820879ee1865 ("sysfs: simplify sysfs_kf_seq_show"), it
> > sounds like we won't hit this often, but I have randomly hit this on my
> > Pixel 6 with 5.19-rc1. Refer to the crash stack below:
> > 
> >    Unable to handle kernel paging request at virtual address ...
> >    Internal error: Oops: 96000004 [#1] PREEMPT SMP
> >    Hardware name: Oriole EVT 1.0 (DT)
> >    pc : sysfs_kf_seq_show+0x3c/0x160
> >    lr : kernfs_seq_show+0x54/0xa0
> >    Call trace:
> >     sysfs_kf_seq_show+0x3c/0x160
> >     kernfs_seq_show+0x54/0xa0
> >     seq_read_iter+0x17c/0x638
> >     kernfs_fop_read_iter+0x70/0x1f4
> >     vfs_read+0x240/0x36c
> >     ksys_read+0x7c/0xf0
> >     __arm64_sys_read+0x20/0x30
> >     invoke_syscall+0x60/0x150
> >     el0_svc_common+0xb8/0x100
> >     do_el0_svc+0x30/0xd4
> >     el0_svc+0x30/0xc0
> >     el0t_64_sync_handler+0x88/0xf8
> >     el0t_64_sync+0x1a0/0x1a4
> > 
> > Fixes: 820879ee1865 ("sysfs: simplify sysfs_kf_seq_show")
> > Signed-off-by: Will McVicker <willmcvicker@...gle.com>
> > ---
> >  fs/sysfs/file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> > index a12ac0356c69..f09f86f10410 100644
> > --- a/fs/sysfs/file.c
> > +++ b/fs/sysfs/file.c
> > @@ -45,7 +45,7 @@ static int sysfs_kf_seq_show(struct seq_file *sf, void *v)
> >  	ssize_t count;
> >  	char *buf;
> >  
> > -	if (WARN_ON_ONCE(!ops->show))
> > +	if (WARN_ON_ONCE(!ops || !ops->show))
> >  		return -EINVAL;
> 
> Seems reasonable, but I want to track down how ops can be NULL here
> under normal operation.

Let me try to follow the code path through the call trace to see if I can track
down how ops could be NULL. It appears we could have hit this before commit
820879ee1865 ("sysfs: simplify sysfs_kf_seq_show") as well.

> 
> thanks,
> 
> greg k-h