Re: KASAN: use-after-free Read in path_lookupat

2019-04-10 Thread Linus Torvalds
On Wed, Apr 10, 2019 at 8:11 AM Al Viro wrote: > > Both are in vfs.git#fixes. Which way should that go - directly or > via linux-security.git? Just do it directly. I doubt you can trigger them for securityfs and apparmourfs, since normal users have no way to remove any files from them, so the

Re: KASAN: use-after-free Read in path_lookupat

2019-04-10 Thread Al Viro
On Tue, Mar 26, 2019 at 01:45:52AM +, Al Viro wrote: > On Mon, Mar 25, 2019 at 11:37:32PM +, Al Viro wrote: > > > For debugfs it's clearly "use default ->evict_inode(), have explicit > > ->destroy_inode() using free_inode_nonrcu()" - there we have nothing > > else done in ->evict_inode()

Re: KASAN: use-after-free Read in path_lookupat

2019-03-28 Thread Jan Kara
On Wed 27-03-19 18:59:48, Al Viro wrote: > On Wed, Mar 27, 2019 at 05:58:31PM +0100, Jan Kara wrote: > > On Tue 26-03-19 04:15:10, Al Viro wrote: > > > On Mon, Mar 25, 2019 at 08:18:25PM -0700, Mark Fasheh wrote: > > > > > > > Hey Al, > > > > > > > > It's been a while since I've looked at that

Re: KASAN: use-after-free Read in path_lookupat

2019-03-27 Thread Al Viro
On Wed, Mar 27, 2019 at 05:58:31PM +0100, Jan Kara wrote: > On Tue 26-03-19 04:15:10, Al Viro wrote: > > On Mon, Mar 25, 2019 at 08:18:25PM -0700, Mark Fasheh wrote: > > > > > Hey Al, > > > > > > It's been a while since I've looked at that bit of code but it looks like > > > Ocfs2 is syncing the

Re: KASAN: use-after-free Read in path_lookupat

2019-03-27 Thread Jan Kara
On Mon 25-03-19 23:02:11, Al Viro wrote: > On Tue, Mar 26, 2019 at 09:48:23AM +1100, Dave Chinner wrote: > > > And when it comes to VFS inode reclaim, XFS does not implement > > ->evict_inode because there is nothing at the VFS level to do. > > And ->destroy_inode ends up doing cleanup work (e.g.

Re: KASAN: use-after-free Read in path_lookupat

2019-03-27 Thread Jan Kara
On Tue 26-03-19 04:15:10, Al Viro wrote: > On Mon, Mar 25, 2019 at 08:18:25PM -0700, Mark Fasheh wrote: > > > Hey Al, > > > > It's been a while since I've looked at that bit of code but it looks like > > Ocfs2 is syncing the inode to disk and disposing of it's memory > > representation (which

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Al Viro
On Mon, Mar 25, 2019 at 08:18:25PM -0700, Mark Fasheh wrote: > Hey Al, > > It's been a while since I've looked at that bit of code but it looks like > Ocfs2 is syncing the inode to disk and disposing of it's memory > representation (which would include the cluster locks held) so that other >

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Al Viro
On Mon, Mar 25, 2019 at 11:37:32PM +, Al Viro wrote: > For debugfs it's clearly "use default ->evict_inode(), have explicit > ->destroy_inode() using free_inode_nonrcu()" - there we have nothing > else done in ->evict_inode() and kfree is obviously safe in softirq. > I'll post that (or push

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Al Viro
On Mon, Mar 25, 2019 at 04:44:40PM -0700, Alexei Starovoitov wrote: > On Mon, Mar 25, 2019 at 11:37:32PM +, Al Viro wrote: > > > > For debugfs it's clearly "use default ->evict_inode(), have explicit > > ->destroy_inode() using free_inode_nonrcu()" - there we have nothing > > else done in

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Alexei Starovoitov
On Mon, Mar 25, 2019 at 11:37:32PM +, Al Viro wrote: > > For debugfs it's clearly "use default ->evict_inode(), have explicit > ->destroy_inode() using free_inode_nonrcu()" - there we have nothing > else done in ->evict_inode() and kfree is obviously safe in softirq. > I'll post that (or push

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Al Viro
On Mon, Mar 25, 2019 at 02:45:00PM -0700, Linus Torvalds wrote: > On Mon, Mar 25, 2019 at 2:14 PM Al Viro wrote: > > > > Maybe, but we really need to come up with sane documentation on the > > entire drop_inode/evict_inode/destroy_inode/rcu_destroy_inode > > group ;-/ > > Yeah. > > I actually

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Al Viro
On Tue, Mar 26, 2019 at 09:48:23AM +1100, Dave Chinner wrote: > And when it comes to VFS inode reclaim, XFS does not implement > ->evict_inode because there is nothing at the VFS level to do. > And ->destroy_inode ends up doing cleanup work (e.g. freeing on-disk > inodes) which is non-trivial,

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Al Viro
On Mon, Mar 25, 2019 at 11:04:53PM +0100, Daniel Borkmann wrote: > +static void bpf_destroy_inode_deferred(struct rcu_head *head) > +{ > + struct inode *inode = container_of(head, struct inode, i_rcu); > + enum bpf_type type; > + > + if (S_ISLNK(inode->i_mode)) > +

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Dave Chinner
On Mon, Mar 25, 2019 at 07:43:32PM +, Al Viro wrote: > On Mon, Mar 25, 2019 at 11:36:01AM -0700, Linus Torvalds wrote: > > Right. Not just move the existing destroy_inode() - because as you > > say, people may not be able to to do that in RCU contect, but split it > > up, and add a

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Daniel Borkmann
On 03/25/2019 11:13 PM, Linus Torvalds wrote: > On Mon, Mar 25, 2019 at 3:04 PM Daniel Borkmann wrote: >> >> I'm fine either way, I think the rcu_destroy_inode would indeed simplify >> it nicely. In any case fwiw, here's what I'd have ready for standby on bpf >> side and tested as well. Decided

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Linus Torvalds
On Mon, Mar 25, 2019 at 3:04 PM Daniel Borkmann wrote: > > I'm fine either way, I think the rcu_destroy_inode would indeed simplify > it nicely. In any case fwiw, here's what I'd have ready for standby on bpf > side and tested as well. Decided to get rid of bpf_evict_inode() entirely > since the

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Daniel Borkmann
On 03/25/2019 10:45 PM, Linus Torvalds wrote: > On Mon, Mar 25, 2019 at 2:14 PM Al Viro wrote: >> >> Maybe, but we really need to come up with sane documentation on the >> entire drop_inode/evict_inode/destroy_inode/rcu_destroy_inode >> group ;-/ > > Yeah. > > I actually think the

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Linus Torvalds
On Mon, Mar 25, 2019 at 2:14 PM Al Viro wrote: > > Maybe, but we really need to come up with sane documentation on the > entire drop_inode/evict_inode/destroy_inode/rcu_destroy_inode > group ;-/ Yeah. I actually think the "destroy_inode/rcu_destroy_inode" part is the simplest one to understand:

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Al Viro
On Mon, Mar 25, 2019 at 12:18:02PM -0700, Linus Torvalds wrote: > COMPLETELY UNTESTED. And no filesystems converted to actually use the > new rcu_destroy_inode() thing. > > Hmm? Maybe, but we really need to come up with sane documentation on the entire

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Al Viro
On Mon, Mar 25, 2019 at 11:36:01AM -0700, Linus Torvalds wrote: > Right. Not just move the existing destroy_inode() - because as you > say, people may not be able to to do that in RCU contect, but split it > up, and add a "final_free_inode()" callback or something for the RCU > phase. > > In

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Linus Torvalds
On Mon, Mar 25, 2019 at 11:36 AM Linus Torvalds wrote: > > > > You mean, split ->destroy_inode() into immediate and RCU-delayed parts? > > There are filesystems where both parts are non-empty - we can't just > > switch all ->destroy_inode() work to call_rcu(). > > Right. Not just move the

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Linus Torvalds
On Sun, Mar 24, 2019 at 9:57 PM Al Viro wrote: > > So we have 5 broken cases, all with the same kind of fix: move freeing > into the RCU-delayed part of ->destroy_inode(); for debugfs and bpf > that requires adding ->alloc_inode()/->destroy_inode(), rather than > relying upon the defaults from

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Daniel Borkmann
On 03/25/2019 12:17 PM, Al Viro wrote: > On Mon, Mar 25, 2019 at 11:11:23AM +, Al Viro wrote: >> On Mon, Mar 25, 2019 at 10:15:40AM +0100, Daniel Borkmann wrote: So we have 5 broken cases, all with the same kind of fix: move freeing into the RCU-delayed part of ->destroy_inode(); for

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Al Viro
On Mon, Mar 25, 2019 at 11:11:23AM +, Al Viro wrote: > On Mon, Mar 25, 2019 at 10:15:40AM +0100, Daniel Borkmann wrote: > > > So we have 5 broken cases, all with the same kind of fix: move freeing > > > into the RCU-delayed part of ->destroy_inode(); for debugfs and bpf > > > that requires

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Al Viro
On Mon, Mar 25, 2019 at 10:15:40AM +0100, Daniel Borkmann wrote: > > So we have 5 broken cases, all with the same kind of fix: move freeing > > into the RCU-delayed part of ->destroy_inode(); for debugfs and bpf > > that requires adding ->alloc_inode()/->destroy_inode(), rather than > > relying

Re: KASAN: use-after-free Read in path_lookupat

2019-03-25 Thread Daniel Borkmann
Hi Al, hi Linus, On 03/25/2019 05:57 AM, Al Viro wrote: > On Sun, Mar 24, 2019 at 06:23:24PM -0700, Linus Torvalds wrote: > >> Al, comments? At the very least, if we don't make >> simple_symlink_inode_operations() do that, we should have a *big* >> comment that if it's not part of the inode

Re: KASAN: use-after-free Read in path_lookupat

2019-03-24 Thread Al Viro
On Sun, Mar 24, 2019 at 06:23:24PM -0700, Linus Torvalds wrote: > Al, comments? At the very least, if we don't make > simple_symlink_inode_operations() do that, we should have a *big* > comment that if it's not part of the inode data, it needs to be > RCU-delayed.

Re: KASAN: use-after-free Read in path_lookupat

2019-03-24 Thread Linus Torvalds
On Sun, Mar 24, 2019 at 5:44 PM syzbot wrote: > > syzbot has bisected this bug to: > > commit a435393acafbf0ecff4deb3e3cb554b34f0d0664 That's wrong. It might change some timing or something to make the race hit, but it looks entirely unrelated to the bpf path_lookupat problem.

Re: KASAN: use-after-free Read in path_lookupat

2019-03-24 Thread Linus Torvalds
Hmm. Al, this one seems real and also seems pretty nasty from a vfs interface standpoint. On Wed, Nov 28, 2018 at 9:40 AM syzbot wrote: > > BUG: KASAN: use-after-free in lookup_last fs/namei.c:2269 [inline] > BUG: KASAN: use-after-free in path_lookupat.isra.43+0x9f8/0xc00 > fs/namei.c:2318 >

Re: KASAN: use-after-free Read in path_lookupat

2019-03-24 Thread syzbot
syzbot has bisected this bug to: commit a435393acafbf0ecff4deb3e3cb554b34f0d0664 Author: Sagi Grimberg Date: Thu Jul 13 08:09:40 2017 + mlx5: move affinity hints assignments to generic code bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=12df1ba320 start commit:

KASAN: use-after-free Read in path_lookupat

2018-11-28 Thread syzbot
Hello, syzbot found the following crash on: HEAD commit:442b8cea2477 Add linux-next specific files for 20181109 git tree: linux-next console output: https://syzkaller.appspot.com/x/log.txt?x=129574db40 kernel config: https://syzkaller.appspot.com/x/.config?x=2f72bdb11df9fbe8

KASAN: use-after-free Read in path_lookupat

2018-11-28 Thread syzbot
Hello, syzbot found the following crash on: HEAD commit:442b8cea2477 Add linux-next specific files for 20181109 git tree: linux-next console output: https://syzkaller.appspot.com/x/log.txt?x=129574db40 kernel config: https://syzkaller.appspot.com/x/.config?x=2f72bdb11df9fbe8