linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: shankarapailoor <shankarapailoor@gmail.com>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	viro@zeniv.linux.org.uk
Cc: Cong Wang <xiyou.wangcong@gmail.com>,
	David Miller <davem@davemloft.net>,
	LKML <linux-kernel@vger.kernel.org>,
	syzkaller <syzkaller@googlegroups.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: general protection fault in sockfs_setattr
Date: Wed, 6 Jun 2018 06:30:05 -0700	[thread overview]
Message-ID: <CAB+yDaZYZ5a5zusTE3V+AN_7uCOxGejDH=h7yEJjw0RB_gfOWQ@mail.gmail.com> (raw)
In-Reply-To: <23d3edb6-a9cd-0295-5433-1b2a13ecbf21@I-love.SAKURA.ne.jp>

++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
<penguin-kernel@i-love.sakura.ne.jp> 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 <xiyou.wangcong@gmail.com> wrote:
>>> On Mon, Jun 4, 2018 at 9:53 PM, shankarapailoor
>>> <shankarapailoor@gmail.com> 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)={<r0=>0xffffffffffffffff, <r1=>0xffffffffffffffff})
>>>> getresuid(&(0x7f0000000080)=<r2=>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

       reply	other threads:[~2018-06-06 13:30 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAB+yDabFuBpT5UU1Hy0s4kY5UKJzA84=6fNieNcdTjjZNq5SHQ@mail.gmail.com>
     [not found] ` <CAM_iQpUaK1xaZAKmPY0JnyAmebBJQ5GPJsiG8HgvXLrxNqphZg@mail.gmail.com>
     [not found]   ` <CAB+yDaahHYeWE1kS=4GEAC3GKyrAenaSat9Wi_iKQbt1yTp_zg@mail.gmail.com>
     [not found]     ` <23d3edb6-a9cd-0295-5433-1b2a13ecbf21@I-love.SAKURA.ne.jp>
2018-06-06 13:30       ` shankarapailoor [this message]
2018-06-06 18:21     ` Cong Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAB+yDaZYZ5a5zusTE3V+AN_7uCOxGejDH=h7yEJjw0RB_gfOWQ@mail.gmail.com' \
    --to=shankarapailoor@gmail.com \
    --cc=davem@davemloft.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=syzkaller@googlegroups.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xiyou.wangcong@gmail.com \
    --subject='Re: general protection fault in sockfs_setattr' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).