From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f66.google.com ([209.85.215.66]:36971 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751907AbeFFNaH (ORCPT ); Wed, 6 Jun 2018 09:30:07 -0400 MIME-Version: 1.0 In-Reply-To: <23d3edb6-a9cd-0295-5433-1b2a13ecbf21@I-love.SAKURA.ne.jp> References: <23d3edb6-a9cd-0295-5433-1b2a13ecbf21@I-love.SAKURA.ne.jp> From: shankarapailoor Date: Wed, 6 Jun 2018 06:30:05 -0700 Message-ID: Subject: Re: general protection fault in sockfs_setattr To: Tetsuo Handa , viro@zeniv.linux.org.uk Cc: Cong Wang , David Miller , LKML , syzkaller , linux-fsdevel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: ++Al Viro >Pastebin says that it was 4.17.0-rc4+ rather than 4.17-rc7. Oops. Apologies Tetsuo. I confirmed the bug still happens with linux.git. Here are the Syzkaller logs around the crash along with my kernel configs. Syzkaller Logs: https://pastebin.com/qqQyX0Ms Config: https://pastebin.com/aEDARPDJ Regards, Shankara On Wed, Jun 6, 2018 at 3:17 AM, Tetsuo Handa wrote: > Pastebin says that it was 4.17.0-rc4+ rather than 4.17-rc7. > I suggest reporting to Al Viro and linux-fsdevel ML after > confirming that this bug still happens with linux.git , in > case this is a dentry related bug (e.g. someone is by error > calling dput() without getting a refcount). > > Also, please don't eliminate kernel messages prior to the > crash. Sometimes previous kernel messages (e.g. memory > allocation fault injection) as-is indicate the cause. > > On 2018/06/06 11:19, shankarapailoor wrote: >> Hi Cong, >> >> I added that check and it seems to stop the crash. Like you said, I >> don't see where the reference count for the file is increased. The >> inode lock also seems to be held during this call. >> >> Regards, >> Shankara >> >> >> >> On Tue, Jun 5, 2018 at 12:14 PM, Cong Wang wrote: >>> On Mon, Jun 4, 2018 at 9:53 PM, shankarapailoor >>> wrote: >>>> Hi, >>>> >>>> I have been fuzzing Linux 4.17-rc7 with Syzkaller and found the >>>> following crash: https://pastebin.com/ixX3RB9j >>>> >>>> Syzkaller isolated the cause of the bug to the following program: >>>> >>>> socketpair$unix(0x1, 0x1, 0x0, >>>> &(0x7f0000000000)={0xffffffffffffffff, 0xffffffffffffffff}) >>>> getresuid(&(0x7f0000000080)=0x0, &(0x7f00000000c0), >>>> &(0x7f0000000700))r3 = getegid() >>>> fchownat(r0, &(0x7f0000000040)='\x00', r2, r3, 0x1000) >>>> dup3(r1, r0, 0x80000) >>>> >>>> >>>> The problematic area appears to be here: >>>> >>>> static int sockfs_setattr(struct dentry *dentry, struct iattr *iattr) >>>> { >>>> int err = simple_setattr(dentry, iattr); >>>> >>>> if (!err && (iattr->ia_valid & ATTR_UID)) { >>>> struct socket *sock = SOCKET_I(d_inode(dentry)); >>>> >>>> sock->sk->sk_uid = iattr->ia_uid; //KASAN GPF >>>> } >>>> return err; >>>> } >>>> >>>> If dup3 is called concurrently with fchownat then can sock->sk be NULL? >>> >>> Although dup3() implies a close(), fd is refcnt'ted, if dup3() runs >>> concurrently with fchownat() it should not be closed until whoever >>> the last closes it. >>> >>> Or maybe fchownat() doesn't even hold refcnt of fd, since it aims >>> to change the file backed. >>> >>> >>> Not sure if the following is sufficient, inode might need to be protected >>> with some lock... >>> >>> diff --git a/net/socket.c b/net/socket.c >>> index f10f1d947c78..6294b4b3132e 100644 >>> --- a/net/socket.c >>> +++ b/net/socket.c >>> @@ -537,7 +537,10 @@ static int sockfs_setattr(struct dentry *dentry, >>> struct iattr *iattr) >>> if (!err && (iattr->ia_valid & ATTR_UID)) { >>> struct socket *sock = SOCKET_I(d_inode(dentry)); >>> >>> - sock->sk->sk_uid = iattr->ia_uid; >>> + if (sock->sk) >>> + sock->sk->sk_uid = iattr->ia_uid; >>> + else >>> + err = -ENOENT; >>> } >>> >>> return err; >> >> >> > -- Regards, Shankara Pailoor