linux-kernel - RE: [PATCHv1 7/7] vfio/mdev: Fix race conditions with mdev device life cycle APIs

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: <AM4PR0501MB22602E3F3480FAC5DC297D4DD13E0@AM4PR0501MB2260.eurprd05.prod.outlook.com>
Date:   Fri, 26 Apr 2019 19:02:40 +0000
From:   Parav Pandit <parav@...lanox.com>
To:     Alex Williamson <alex.williamson@...hat.com>
CC:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kwankhede@...dia.com" <kwankhede@...dia.com>,
        "cjia@...dia.com" <cjia@...dia.com>
Subject: RE: [PATCHv1 7/7] vfio/mdev: Fix race conditions with mdev device
 life cycle APIs

Hi Alex,


> -----Original Message-----
> From: Alex Williamson <alex.williamson@...hat.com>
> Sent: Friday, April 26, 2019 11:09 AM
> To: Parav Pandit <parav@...lanox.com>
> Cc: kvm@...r.kernel.org; linux-kernel@...r.kernel.org;
> kwankhede@...dia.com; cjia@...dia.com
> Subject: Re: [PATCHv1 7/7] vfio/mdev: Fix race conditions with mdev device
> life cycle APIs

[..]

> > > >
> > > > > Patch 6 looks ok, except I'd rather see the sanitizing loop stay
> > > > > until we can otherwise fix the race above.
> > > > I can put back the sanitizing look, once it looks valid. Will wait
> > > > for your response.
> > >
> > > Yep, I think patch 6 is good w/o the removal of the sanitizing loop.
> > > Will you repost it?
> > >
> > Just the patch-6 or 1 to 6?
> 
> Your choice, please roll in reviews/acks if you repost the rest.
> 
> > > > > Patch 7 needed more work, iirc.  Thanks,
> > > > For a moment if we assume sanitizing loop exists, than patch-7
> > > > looks good?
> > >
> > > Patch 7 is a bit less trivial, so I think as we're close to the
> > > merge window for v5.2, I'd rather push it out to be included with
> > > the later re-works.  Thanks,
> > >
> > I agree it little less trivial, I tried to place as much comment as possible,
> but it is an important fix.
> > Let me repost 6-7 and decide if it can be included or not?
> 
> Sounds good.  Thanks,
> 
I am dropping patch-7 for today and reworking the patch-6 for now.

Even after keeping that that crazy loop, I am easily able to create this below [1] call trace on adding file when mdev_remove() fails with the thread sequence we discussed above.

I think this is high time, we fix the sequence to match the linux bus sequence.
I have some cycles this week.
Post these 6 patches,
I like to get total 3 patches done.
1. fix the bus sequence
2. race with parent device removal
3. do not try to add sysfs file on remove() failure

Is there any possibility above 3 patches can make to 5.2, given that merge window closes in June?
If yes, I will get them in 2-3 days. I will test with sample drivers and mlx5 driver.
Can we get some tests also done from Kirti also done on their hw?

kernel: WARNING: CPU: 2 PID: 9348 at fs/sysfs/file.c:327 sysfs_create_file_ns+0x7f/0x90
kernel: CPU: 2 PID: 9348 Comm: bash Kdump: loaded Not tainted 5.1.0-rc6-vdevbus+ #6
kernel: Hardware name: Supermicro SYS-6028U-TR4+/X10DRU-i+, BIOS 2.0b 08/09/2016
kernel: RIP: 0010:sysfs_create_file_ns+0x7f/0x90
kernel: Call Trace:
kernel: remove_store+0xdc/0x100 [mdev]
kernel: kernfs_fop_write+0x113/0x1a0
kernel: vfs_write+0xad/0x1b0
kernel: ksys_write+0x5a/0xe0
kernel: do_syscall_64+0x5a/0x210
kernel: entry_SYSCALL_64_after_hwframe+0x49/0xbe